[vlc-devel] [PATCH] demux: mkv: parse language codes with embedded country codes

[Filip Roséen]

Hello Shaleen,

On 2017-04-14 10:32, Shaleen Jain wrote:

> fixes bug: https://trac.videolan.org/vlc/ticket/11813
> mkv spec: https://www.matroska.org/technical/specs/index.html#languages
> ---
>  modules/demux/mkv/matroska_segment_parse.cpp | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/modules/demux/mkv/matroska_segment_parse.cpp b/modules/demux/mkv/matroska_segment_parse.cpp
> index f4d73613c9..705a5baab0 100644
> --- a/modules/demux/mkv/matroska_segment_parse.cpp
> +++ b/modules/demux/mkv/matroska_segment_parse.cpp
> @@ -379,9 +379,14 @@ void matroska_segment_c::ParseTrackEntry( KaxTrackEntry *m )
>          }
>          E_CASE( KaxTrackLanguage, lang )
>          {
> -            free( vars.tk->fmt.psz_language );
> -            vars.tk->fmt.psz_language = strdup( std::string( lang ).c_str() );
> -            debug( vars, "Track Language=`%s'", vars.tk->fmt.psz_language );
> +            free(vars.tk->fmt.psz_language);
> +            char* psz_lang = strdup(std::string(lang).c_str());
> +            if (psz_lang != NULL && strchr(psz_lang, '-') != NULL) {
> +                vars.tk->fmt.psz_language = strtok(psz_lang, "-");
> +            } else {
> +                vars.tk->fmt.psz_language = psz_lang;
> +            }
> +            debug(vars, "Track Language=`%s'", vars.tk->fmt.psz_language );

As implied by my earlier message, please use the C++ utilities
available for making this work, or take Jean-Baptiste's [advice][1],
and use a helper available in `<vlc_iso_long.h>`.

Given that you did not take neither of the adviced paths, see the
below for an example of a functionally equivalent implementation to
yours.

    std::string const& slang ( lang );

    free( vars.tk->fmt.psz_language );
    vars.tk->fmt.psz_language = strndup( slang.c_str (),
        slang.find_first_of( '-' ) );

[1]: https://mailman.videolan.org/pipermail/vlc-devel/2017-April/112839.html

As I expected you to move to C++ utilities I did not bother to point
out the problem in terms of thread-safety with `strtok`; but it is
important for you to note that *Rémi* is of course correct with his
remark, and that your reasoning in [the reply to his message][1] is
wrong.

[1]: https://mailman.videolan.org/pipermail/vlc-devel/2017-April/112856.html

Best Regards,\
Filip

>          }
>          E_CASE( KaxCodecID, codecid )
>          {
> -- 
> 2.12.2
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20170414/99132249/attachment.html>