diff mbox

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

Message ID CACS3z+YmRL9JXq4rMgOakXxP0xeQ11VmsSFC2sB_C_DJcRQaGQ@mail.gmail.com
State Superseded
Headers show

Commit Message

Philippe Symons Nov. 14, 2018, 11:29 p.m. UTC 
Hello everyone,

Here is the new version of the patch in which the comments on the curly
braces have been resolved as well.

I hope it's okay now.

Best regards,

Philippe Symons

Op do 15 nov. 2018 om 00:13 schreef Philippe Symons <
philippe.symons@gmail.com>:

> Hold on, I just noticed the feedback on the curly braces. I'll create a
> new version of the patch to resolve this as well.
>
> Op wo 14 nov. 2018 om 23:49 schreef Philippe Symons <
> philippe.symons@gmail.com>:
>
>> Hello,
>>
>> I've created a new version of the patch in which the second empty line
>> was removed. I couldn't find any other feedback inline in your response, so
>> that's all I changed.
>>
>> Best regards,
>>
>> Philippe Symons
>>
>>
>> Op di 13 nov. 2018 om 03:29 schreef Liu Steven <lq@chinaffmpeg.org>:
>>
>>>
>>>
>>> > 在 2018年11月13日，上午6:36，Philippe Symons <philippe.symons@gmail.com> 写道：
>>> >
>>> > Hello everyone,
>>> >
>>> > This is my first patch to FFMpeg or any open-source project for that
>>> > matter. In this patch, I've added the LANGUAGE attribute to the
>>> > #EXT-X-MEDIA tag for for audio only variant streams for HLS streams.
>>> >
>>> > This attribute will be added to the output in
>>> > avformats' ff_hls_write_audio_rendition() function, if the language
>>> code
>>> > can be found in the metadata of the variant stream.
>>> >
>>> > The reason why I implemented this, was because I was working with a
>>> player
>>> > which needs this attribute to determine the language of the audio
>>> stream.
>>> > After asking the question on StackOverflow (
>>> >
>>> https://stackoverflow.com/questions/53205283/ffmpeg-hls-multiple-audio-languages-with-var-stream-map-ext-x-mediatype-doesn/53206492#53206492
>>> ),
>>> > I noticed that it wasn't implemented.
>>> >
>>> > I've tested these changes with the following command:
>>> >
>>> > ffmpeg -re -i /home/philippe/Videos/example.ts -map 0:v -c:v copy -map
>>> 0:a
>>> > -c:a copy -f hls -hls_time 6 -hls_list_size 10 -hls_flags
>>> > delete_segments+program_date_time -hls_segment_filename
>>> > "/var/www/html/live/stream_%v_%d.ts" -var_stream_map "v:0,agroup:lang
>>> > a:0,agroup:lang a:1,agroup:lang a:2,agroup:lang a:
>>> > 3,agroup:lang" -master_pl_name master.m3u8
>>> /var/www/html/live/stream-%v.m3u8
>>> >
>>> > Looking forward to your feedback. I hope it's okay.
>>> >
>>>
>>> Signed-off-by: Philippe Symons <philippe.symons@gmail.com>
>>> ---
>>>  libavformat/dashenc.c     |  2 +-
>>>  libavformat/hlsenc.c      | 24 ++++++++++++++++++++++--
>>>  libavformat/hlsplaylist.c |  6 +++++-
>>>  libavformat/hlsplaylist.h |  2 +-
>>>  4 files changed, 29 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
>>> index f8b3d106d5..ca5ef01e3f 100644
>>> --- a/libavformat/dashenc.c
>>> +++ b/libavformat/dashenc.c
>>> @@ -897,7 +897,7 @@ static int write_manifest(AVFormatContext *s, int
>>> final)
>>>              if (st->codecpar->codec_type != AVMEDIA_TYPE_AUDIO)
>>>                  continue;
>>>              get_hls_playlist_name(playlist_file, sizeof(playlist_file),
>>> NULL, i);
>>> -            ff_hls_write_audio_rendition(c->m3u8_out, (char
>>> *)audio_group,
>>> +            ff_hls_write_audio_rendition(c->m3u8_out, (char
>>> *)audio_group, NULL,
>>>                                           playlist_file, i, is_default);
>>>              max_audio_bitrate = FFMAX(st->codecpar->bit_rate +
>>>                                        os->muxer_overhead,
>>> max_audio_bitrate);
>>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
>>> index e0048aa9d8..b31daa108e 100644
>>> --- a/libavformat/hlsenc.c
>>> +++ b/libavformat/hlsenc.c
>>> @@ -1205,7 +1205,6 @@ static int create_master_playlist(AVFormatContext
>>> *s,
>>>      }
>>>
>>>      set_http_options(s, &options, hls);
>>> -
>>>      ret = hlsenc_io_open(s, &hls->m3u8_out, hls->master_m3u8_url,
>>> &options);
>>>      av_dict_free(&options);
>>>      if (ret < 0) {
>>> @@ -1228,12 +1227,33 @@ static int
>>> create_master_playlist(AVFormatContext *s,
>>>
>>>      /* For audio only variant streams add #EXT-X-MEDIA tag with
>>> attributes*/
>>>      for (i = 0; i < hls->nb_varstreams; i++) {
>>> +        AVFormatContext* var_context;
>>> +        char* language = NULL;
>>> +
>>>          vs = &(hls->var_streams[i]);
>>> +        var_context = vs->avf;
>>>
>>>          if (vs->has_video || vs->has_subtitle || !vs->agroup)
>>>              continue;
>>>
>>>          m3u8_name_size = strlen(vs->m3u8_name) + 1;
>>> +
>>> +        /*
>>> +         * Try to obtain the language code of the audio stream.
>>> +         * -if available- it will be used to write the LANGUAGE
>>> +         * attribute in the #EXT-X-MEDIA tag
>>> +         */
>>> +        if(var_context && var_context->nb_streams > 0)
>>> +        {
>>> if (var_context && var_context->nb_streams > 0) {
>>> +            AVDictionary* meta_dict = vs->streams[0]->metadata;
>>> +            AVDictionaryEntry *langEntry = av_dict_get(meta_dict,
>>> "language", NULL, 0);
>>> +            if(langEntry)
>>> +            {
>>>  if (langEntry) {
>>> +                language = langEntry->value;
>>> +            }
>>> +        }
>>> +
>>> +
>>>
>>> two empty line?
>>>          m3u8_rel_name = av_malloc(m3u8_name_size);
>>>          if (!m3u8_rel_name) {
>>>              ret = AVERROR(ENOMEM);
>>> @@ -1247,7 +1267,7 @@ static int create_master_playlist(AVFormatContext
>>> *s,
>>>              goto fail;
>>>          }
>>>
>>> -        ff_hls_write_audio_rendition(hls->m3u8_out, vs->agroup,
>>> m3u8_rel_name, 0, 1);
>>> +        ff_hls_write_audio_rendition(hls->m3u8_out, vs->agroup,
>>> language, m3u8_rel_name, 0, 1);
>>>
>>>          av_freep(&m3u8_rel_name);
>>>      }
>>> diff --git a/libavformat/hlsplaylist.c b/libavformat/hlsplaylist.c
>>> index efcbff0009..42c7cf5727 100644
>>> --- a/libavformat/hlsplaylist.c
>>> +++ b/libavformat/hlsplaylist.c
>>> @@ -35,12 +35,16 @@ void ff_hls_write_playlist_version(AVIOContext *out,
>>> int version) {
>>>      avio_printf(out, "#EXT-X-VERSION:%d\n", version);
>>>  }
>>>
>>> -void ff_hls_write_audio_rendition(AVIOContext *out, char *agroup,
>>> +void ff_hls_write_audio_rendition(AVIOContext *out, char *agroup, char*
>>> language,
>>>                                    char *filename, int name_id, int
>>> is_default) {
>>>      if (!out || !agroup || !filename)
>>>          return;
>>>
>>>      avio_printf(out, "#EXT-X-MEDIA:TYPE=AUDIO,GROUP-ID=\"group_%s\"",
>>> agroup);
>>> +
>>> +    if(language)
>>> +        avio_printf(out, ",LANGUAGE=\"%s\"", language);
>>> +
>>>      avio_printf(out, ",NAME=\"audio_%d\",DEFAULT=%s,URI=\"%s\"\n",
>>> name_id,
>>>                       is_default ? "YES" : "NO", filename);
>>>  }
>>> diff --git a/libavformat/hlsplaylist.h b/libavformat/hlsplaylist.h
>>> index 5054b01c8f..faf1ccdd02 100644
>>> --- a/libavformat/hlsplaylist.h
>>> +++ b/libavformat/hlsplaylist.h
>>> @@ -37,7 +37,7 @@ typedef enum {
>>>  } PlaylistType;
>>>
>>>  void ff_hls_write_playlist_version(AVIOContext *out, int version);
>>> -void ff_hls_write_audio_rendition(AVIOContext *out, char *agroup,
>>> +void ff_hls_write_audio_rendition(AVIOContext *out, char *agroup, char*
>>> language,
>>>                                    char *filename, int name_id, int
>>> is_default);
>>>  void ff_hls_write_stream_info(AVStream *st, AVIOContext *out,
>>>                                int bandwidth, char *filename, char
>>> *agroup,
>>> --
>>> 2.17.1
>>>
>>>
>>>
>>>
>>>
>>>
>>> > Best regards,
>>> >
>>> > Philippe Symons
>>> >
>>> <0001-HLS-Add-LANGUAGE-attribute-to-EXT-X-MEDIA-tag-for-au.patch>_______________________________________________
>>> > ffmpeg-devel mailing list
>>> > ffmpeg-devel@ffmpeg.org
>>> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>
>>>
>>>
>>> _______________________________________________
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel@ffmpeg.org
>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>
>>

Comments

Moritz Barsnick Nov. 17, 2018, 10:19 a.m. UTC | #1 
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
diff mbox

Patch

From 2f2d4a829d30a8dfc6084b347bb597ac994d6dda Mon Sep 17 00:00:00 2001
From: Philippe Symons <philippe.symons@gmail.com>
Date: Thu, 15 Nov 2018 00:20:17 +0100
Subject: [PATCH] [HLS] Add LANGUAGE attribute to #EXT-X-MEDIA tag for 
 audio-only variant streams.

Signed-off-by: Philippe Symons <philippe.symons@gmail.com>
---
 libavformat/dashenc.c     |  2 +-
 libavformat/hlsenc.c      | 21 +++++++++++++++++++--
 libavformat/hlsplaylist.c |  6 +++++-
 libavformat/hlsplaylist.h |  2 +-
 4 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
index d151921175..17465c1917 100644
--- a/libavformat/dashenc.c
+++ b/libavformat/dashenc.c
@@ -898,7 +898,7 @@  static int write_manifest(AVFormatContext *s, int final)
             if (st->codecpar->codec_type != AVMEDIA_TYPE_AUDIO)
                 continue;
             get_hls_playlist_name(playlist_file, sizeof(playlist_file), NULL, i);
-            ff_hls_write_audio_rendition(c->m3u8_out, (char *)audio_group,
+            ff_hls_write_audio_rendition(c->m3u8_out, (char *)audio_group, NULL, 
                                          playlist_file, i, is_default);
             max_audio_bitrate = FFMAX(st->codecpar->bit_rate +
                                       os->muxer_overhead, max_audio_bitrate);
diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index e0048aa9d8..fb77aa9ba8 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -1205,7 +1205,6 @@  static int create_master_playlist(AVFormatContext *s,
     }
 
     set_http_options(s, &options, hls);
-
     ret = hlsenc_io_open(s, &hls->m3u8_out, hls->master_m3u8_url, &options);
     av_dict_free(&options);
     if (ret < 0) {
@@ -1228,12 +1227,30 @@  static int create_master_playlist(AVFormatContext *s,
 
     /* For audio only variant streams add #EXT-X-MEDIA tag with attributes*/
     for (i = 0; i < hls->nb_varstreams; i++) {
+        AVFormatContext* var_context;
+        char* language = NULL;
+
         vs = &(hls->var_streams[i]);
+        var_context = vs->avf;
 
         if (vs->has_video || vs->has_subtitle || !vs->agroup)
             continue;
 
         m3u8_name_size = strlen(vs->m3u8_name) + 1;
+        
+        /*
+         * Try to obtain the language code of the audio stream.
+         * -if available- it will be used to write the LANGUAGE
+         * attribute in the #EXT-X-MEDIA tag
+         */
+        if(var_context && var_context->nb_streams > 0) {
+            AVDictionary* meta_dict = vs->streams[0]->metadata;
+            AVDictionaryEntry *langEntry = av_dict_get(meta_dict, "language", NULL, 0);
+            if(langEntry) {
+                language = langEntry->value;
+            }
+        }
+
         m3u8_rel_name = av_malloc(m3u8_name_size);
         if (!m3u8_rel_name) {
             ret = AVERROR(ENOMEM);
@@ -1247,7 +1264,7 @@  static int create_master_playlist(AVFormatContext *s,
             goto fail;
         }
 
-        ff_hls_write_audio_rendition(hls->m3u8_out, vs->agroup, m3u8_rel_name, 0, 1);
+        ff_hls_write_audio_rendition(hls->m3u8_out, vs->agroup, language, m3u8_rel_name, 0, 1);
 
         av_freep(&m3u8_rel_name);
     }
diff --git a/libavformat/hlsplaylist.c b/libavformat/hlsplaylist.c
index efcbff0009..42c7cf5727 100644
--- a/libavformat/hlsplaylist.c
+++ b/libavformat/hlsplaylist.c
@@ -35,12 +35,16 @@  void ff_hls_write_playlist_version(AVIOContext *out, int version) {
     avio_printf(out, "#EXT-X-VERSION:%d\n", version);
 }
 
-void ff_hls_write_audio_rendition(AVIOContext *out, char *agroup,
+void ff_hls_write_audio_rendition(AVIOContext *out, char *agroup, char* language, 
                                   char *filename, int name_id, int is_default) {
     if (!out || !agroup || !filename)
         return;
 
     avio_printf(out, "#EXT-X-MEDIA:TYPE=AUDIO,GROUP-ID=\"group_%s\"", agroup);
+
+    if(language)
+        avio_printf(out, ",LANGUAGE=\"%s\"", language);
+
     avio_printf(out, ",NAME=\"audio_%d\",DEFAULT=%s,URI=\"%s\"\n", name_id,
                      is_default ? "YES" : "NO", filename);
 }
diff --git a/libavformat/hlsplaylist.h b/libavformat/hlsplaylist.h
index 5054b01c8f..faf1ccdd02 100644
--- a/libavformat/hlsplaylist.h
+++ b/libavformat/hlsplaylist.h
@@ -37,7 +37,7 @@  typedef enum {
 } PlaylistType;
 
 void ff_hls_write_playlist_version(AVIOContext *out, int version);
-void ff_hls_write_audio_rendition(AVIOContext *out, char *agroup,
+void ff_hls_write_audio_rendition(AVIOContext *out, char *agroup, char* language, 
                                   char *filename, int name_id, int is_default);
 void ff_hls_write_stream_info(AVStream *st, AVIOContext *out,
                               int bandwidth, char *filename, char *agroup,
-- 
2.17.1