[FFmpeg-devel] [PATCH] [HLS] Add LANGUAGE attribute to #EXT-X-MEDIA tag for audio-only variant streams.

[Philippe Symons]

... I just noticed the langEntry var name. Here's an update. Sorry... Even
after checking the guidelines twice, apparently I still miss things.

I'll try to do better next time. I really expect this to be the last
update. (at the very least concerning coding style issues)

Sorry,

Philippe

Op ma 19 nov. 2018 om 21:21 schreef Philippe Symons <
philippe.symons at gmail.com>:

> Hello everyone,
>
> I've updated the patch based on the feedback from Moritz. Thanks, btw! I
> apologize if I wasted your time with this.
>
> I've updated the patch based on your feedback. I hope I got it right this
> time.
>
> Looking forward to your feedback,
>
> Best regards,
>
> Philippe Symons
>
> Op za 17 nov. 2018 om 11:20 schreef Moritz Barsnick <barsnick at gmx.net>:
>
>> On Thu, Nov 15, 2018 at 00:29:00 +0100, Philippe Symons wrote:
>> > Here is the new version of the patch in which the comments on the curly
>> > braces have been resolved as well.
>>
>> Style-wise, there are other issues. (I'm not judging technically here.)
>>
>> > Subject: [PATCH] [HLS] Add LANGUAGE attribute to #EXT-X-MEDIA tag for
>> >  audio-only variant streams.
>>
>> The project prefers:
>> avformat/hls,dash: add LANGUAGE attribute to #EXT-X-MEDIA tag for
>> audio-only variant streams
>>
>> (i.e. source hierarchy, no trailing dot, ...)
>>
>> >      set_http_options(s, &options, hls);
>> > -
>> >      ret = hlsenc_io_open(s, &hls->m3u8_out, hls->master_m3u8_url,
>> &options);
>>
>> Don't add extra lines, please.
>>
>> >      for (i = 0; i < hls->nb_varstreams; i++) {
>> > +        AVFormatContext* var_context;
>> > +        char* language = NULL;
>>
>> Please read
>> https://www.ffmpeg.org/developer.html#Coding-Rules-1
>>
>> abouts brackets and indentation, and look at other code sections..
>> Furthermore, the "pointer specifier" (or whatever that's called) sticks
>> to the variable, not the type, in ffmpeg declarations:
>>
>> char *language = NULL;
>>
>> Same in other places.
>>
>> > +        if(var_context && var_context->nb_streams > 0) {
>>
>> Bracket / whitspace style: "if (var_context [...]"
>>
>> > +            if(langEntry) {
>> Same here: if (langEntry)
>> And in other places.
>>
>> > +                language = langEntry->value;
>> > +            }
>>
>> Some developers prefer you to drop the curly brackets around one-liner
>> blocks.
>>
>> No review of the technical implications, sorry.
>>
>> Cheers,
>> Moritz
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-avformat-hls-dash-add-LANGUAGE-attribute-to-EXT-X-ME.patch
Type: application/x-patch
Size: 4527 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20181119/30ee2dd6/attachment.bin>