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

[Moritz Barsnick]

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