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

[Philippe Symons]

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: text/x-patch
Size: 4523 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20181119/bbfa9fda/attachment.bin>