[vlc-devel] [PATCH 1/2] VLSub: support for subtitles that "SubFormat" attribute is different that subtitle extension Some subtitles in opensubtitles has attribute "SubFormat" different than the filename so they are not found in the downloaded archive. This patch checks for SubFileName first, so if proper subtitle is found, the "SubFormat" attribute is ignored. (srt=>sub, sub=>txt, mpl=>txt etc.)

[Filip Roséen]

Hi Adrian,

I am not sure what the status of this patch set is as I have been away
from the mailing-list for quite some time, so if it has already been
superseded (I looked but couldn't find anything) I apologize for the
useless reply.

It is also worth noting that your commit messages are formatted weird.
On the first line you should be the "subject" line, which should be
short, you can later go on and explain the changes in more detail on
lines that follow. Do not put everything on the same line.

On 2018-08-30 16:37, Adrian Borkiet wrote:

> ---
>  share/lua/extensions/VLSub.lua | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/share/lua/extensions/VLSub.lua b/share/lua/extensions/VLSub.lua
> index ee9e5f84a9..720dd08cdc 100644
> --- a/share/lua/extensions/VLSub.lua
> +++ b/share/lua/extensions/VLSub.lua
> @@ -329,8 +329,8 @@ local select_conf = {} -- Drop down widget / option table association
>  
>  function descriptor()
>    return {
> -    title = "VLsub 0.11.0",
> -    version = "0.11.0",
> +    title = "VLsub 0.11.1",
> +    version = "0.11.1",
>      author = "exebetche",
>      url = 'http://www.opensubtitles.org/',
>      shortdesc = "VLsub";
> @@ -1010,7 +1010,7 @@ openSub = {
>      url = "http://api.opensubtitles.org/xml-rpc",
>      path = nil,
>      userAgentHTTP = "VLSub",
> -    useragent = "VLSub 0.11.0",
> +    useragent = "VLSub 0.11.1",
>      translations_avail = {},
>      downloadBehaviours = nil,
>      languages = languages
> @@ -1437,12 +1437,17 @@ function get_first_sel(list)
>    return 0
>  end
>  
> -function find_subtitle_in_archive(archivePath, subfileExt)
> +function find_subtitle_in_archive(archivePath, subfileExt, subFileName)
>    local archive = vlc.directory_stream(vlc.strings.make_uri(archivePath))
>    local items = archive:readdir()
>    if not items then
>      return nil
>    end
> +  for _, item in pairs(items) do
> +    if string.sub(item:uri(), -string.len(subFileName)) == subFileName then

It might be unlikely, but the above can result in false-positives if
the subtitle filename is a suffix of some other file with a longer
name. If you have `demo-awesome_subtitle.srt` and
`awesome_subtitle.srt`, the former could be matched when looking for
`awesome_subtitle.srt`.

In order to fix this issue you must take directory separators into
account.

> +      return item:uri()
> +    end
> +  end
>    subfileExt = "." .. subfileExt
>    for _, item in pairs(items) do
>      if string.sub(item:uri(), -string.len(subfileExt)) == subfileExt then

The above has the same problem, though even less likely to cause
issues in practice.

> @@ -1503,7 +1508,7 @@ function download_subtitles()
>    end
>    vlc.msg.dbg("[VLsub] tmpFileName: "..tmpFileName)
>  
> -  local subtitleMrl = find_subtitle_in_archive(tmpFileName, item.SubFormat)
> +  local subtitleMrl = find_subtitle_in_archive(tmpFileName, item.SubFormat, item.SubFileName)
>  
>    if not subtitleMrl then
>      setMessage( lang['mess_not_load'] )

Best Regards,\
Filip
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20180920/c20e3f2b/attachment.html>