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@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
