diff mbox series

[FFmpeg-devel] avformat/hls: Fixed incorrect behaviour of default setting, , added autoselect and forced

Message ID 691930bc-b3e6-8185-2a7e-cf90462d4376@gmail.com
State New
Headers show
Series [FFmpeg-devel] avformat/hls: Fixed incorrect behaviour of default setting, , added autoselect and forced | expand

Checks

Context Check Description
andriy/configure_x86 warning Failed to apply patch

Commit Message

CoderVenkat July 5, 2024, 1:27 p.m. UTC 
In absence of default in var_stream_map, it was setting default=yes on 
every stream,
but according to RFC8216 4.3.4.1 only one stream in a default group may 
have that.
Additionally added support for autoselect=yes/no, whose presence 
combined with default
means that it MUST be YES. Similarly forced=yes/no for subtitle stream.

Showed sample output of incorrectness in bug #11088

Signed-off-by: vckt <codervenkat@gmail.com>
---
  libavformat/dashenc.c     |  3 ++-
  libavformat/hlsenc.c      | 41 +++++++++++++++++++++++++++++++++++++--
  libavformat/hlsplaylist.c | 25 ++++++++++++++++++++----
  libavformat/hlsplaylist.h |  6 ++++--
  4 files changed, 66 insertions(+), 9 deletions(-)

  void ff_hls_write_stream_info(AVStream *st, AVIOContext *out, int 
bandwidth,
                                int avg_bandwidth,
                                const char *filename, const char *agroup,

Comments

Steven Liu July 7, 2024, 10:18 a.m. UTC | #1 
CoderVenkat <codervenkat@gmail.com> 于2024年7月5日周五 21:27写道：
>
> In absence of default in var_stream_map, it was setting default=yes on
> every stream,
> but according to RFC8216 4.3.4.1 only one stream in a default group may
> have that.
> Additionally added support for autoselect=yes/no, whose presence
> combined with default
> means that it MUST be YES. Similarly forced=yes/no for subtitle stream.
>
> Showed sample output of incorrectness in bug #11088
>
> Signed-off-by: vckt <codervenkat@gmail.com>
> ---
>   libavformat/dashenc.c     |  3 ++-
>   libavformat/hlsenc.c      | 41 +++++++++++++++++++++++++++++++++++++--
>   libavformat/hlsplaylist.c | 25 ++++++++++++++++++++----
>   libavformat/hlsplaylist.h |  6 ++++--
>   4 files changed, 66 insertions(+), 9 deletions(-)
>
> diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
> index d4a6fe0304..c84b44f20d 100644
> --- a/libavformat/dashenc.c
> +++ b/libavformat/dashenc.c
> @@ -1273,6 +1273,7 @@ static int write_manifest(AVFormatContext *s, int
> final)
>               const char *audio_group = "A1";
>               char audio_codec_str[128] = "\0";
>               int is_default = 1;
> +            int autoselect = 0;
>               int max_audio_bitrate = 0;
>
>               for (i = 0; i < s->nb_streams; i++) {
> @@ -1285,7 +1286,7 @@ static int write_manifest(AVFormatContext *s, int
> final)
>                       continue;
>                   get_hls_playlist_name(playlist_file,
> sizeof(playlist_file), NULL, i);
>                   ff_hls_write_audio_rendition(c->m3u8_out, audio_group,
> -                                             playlist_file, NULL, i,
> is_default,
> +                                             playlist_file, NULL, i,
> is_default, autoselect
> s->streams[i]->codecpar->ch_layout.nb_channels);
>                   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 274de00f9a..af4501af54 100644
> --- a/libavformat/hlsenc.c
> +++ b/libavformat/hlsenc.c
> @@ -184,6 +184,8 @@ typedef struct VariantStream {
>       unsigned int nb_streams;
>       int m3u8_created; /* status of media play-list creation */
>       int is_default; /* default status of audio group */
> +    int autoselect; /* autoselect by system language */
> +    int forced; /* forced status of subtitle stream */
>       const char *language; /* audio language name */
>       const char *agroup;   /* audio group name */
>       const char *sgroup;   /* subtitle group name */
> @@ -1434,6 +1436,31 @@ static int create_master_playlist(AVFormatContext *s,
>           avio_printf(hls->m3u8_out, ",INSTREAM-ID=\"%s\"\n",
> ccs->instreamid);
>       }
>
> +    /* Check only one default is present in a group */
> +    for (i = 0; i < hls->nb_varstreams; i++) {
> +        vs = &(hls->var_streams[i]);
> +        for (j = 0; j < hls->nb_varstreams; j++) {
> +            if (i != j) {
> +                temp_vs = &(hls->var_streams[j]);
> +                if (vs->agroup && temp_vs->agroup &&
> +                    !vs->has_video && !temp_vs->has_video) {
> +                    if (!av_strcasecmp(vs->agroup, temp_vs->agroup) &&
> +                        vs->is_default && temp_vs->is_default) {
> +                        av_log(s, AV_LOG_ERROR, "Two streams in an
> agroup can not be default\n");
> +                        goto fail;
> +                    }
> +                } else if (vs->sgroup && temp_vs->sgroup &&
> +                            !vs->has_video && !temp_vs->has_video) {
> +                    if (!av_strcasecmp(vs->sgroup, temp_vs->sgroup) &&
> +                        vs->is_default && temp_vs->is_default) {
> +                        av_log(s, AV_LOG_ERROR, "Two streams in an
> sgroup can not be default\n");
> +                        goto fail;
> +                    }
> +                }
> +            }
> +        }
> +    }
> +
>       /* For audio only variant streams add #EXT-X-MEDIA tag with
> attributes*/
>       for (i = 0; i < hls->nb_varstreams; i++) {
>           vs = &(hls->var_streams[i]);
> @@ -1452,7 +1479,7 @@ static int create_master_playlist(AVFormatContext *s,
>                   if (vs->streams[j]->codecpar->ch_layout.nb_channels >
> nb_channels)
>                       nb_channels =
> vs->streams[j]->codecpar->ch_layout.nb_channels;
>
> -        ff_hls_write_audio_rendition(hls->m3u8_out, vs->agroup,
> m3u8_rel_name, vs->language, i, hls->has_default_key ? vs->is_default :
> 1, nb_channels);
> +        ff_hls_write_audio_rendition(hls->m3u8_out, vs->agroup,
> m3u8_rel_name, vs->language, i, hls->has_default_key ? vs->is_default :
> 0, vs->autoselect, nb_channels);
>       }
>
>       /* For variant streams with video add #EXT-X-STREAM-INF tag with
> attributes*/
> @@ -1533,7 +1560,7 @@ static int create_master_playlist(AVFormatContext *s,
>                   break;
>               }
>
> -            ff_hls_write_subtitle_rendition(hls->m3u8_out, sgroup,
> vtt_m3u8_rel_name, vs->language, i, hls->has_default_key ?
> vs->is_default : 1);
> +            ff_hls_write_subtitle_rendition(hls->m3u8_out, sgroup,
> vtt_m3u8_rel_name, vs->language, i, hls->has_default_key ?
> vs->is_default : 0, vs->autoselect, vs->forced);
>           }
>
>           if (!hls->has_default_key || !hls->has_video_m3u8) {
> @@ -2104,6 +2131,16 @@ static int
> parse_variant_stream_mapstring(AVFormatContext *s)
>                                     (!av_strncasecmp(val, "1",
> strlen("1"))));
>                   hls->has_default_key = 1;
>                   continue;
> +            } else if (av_strstart(keyval, "autoselect:", &val)) {
> +                vs->autoselect = (!av_strncasecmp(val, "YES",
> strlen("YES")) ||
> +                                  (!av_strncasecmp(val, "1",
> strlen("1")))) ||
What about move the strlen const to start of this function, then it
not need more strlen functions call.
> +                                  (hls->has_default_key && vs->is_default);
> +                /* autoselect must = 1 if default = 1 */
> +                continue;
> +            } else if (av_strstart(keyval, "forced:", &val)) {
> +                vs->forced     = (!av_strncasecmp(val, "YES",
> strlen("YES")) ||
> +                                  (!av_strncasecmp(val, "1",
> strlen("1"))));
> +                continue;
>               } else if (av_strstart(keyval, "name:", &val)) {
>                   vs->varname  = val;
>                   continue;
> diff --git a/libavformat/hlsplaylist.c b/libavformat/hlsplaylist.c
> index f8a6977702..40a8a36849 100644
> --- a/libavformat/hlsplaylist.c
> +++ b/libavformat/hlsplaylist.c
> @@ -39,13 +39,20 @@ void ff_hls_write_playlist_version(AVIOContext *out,
> int version)
>
>   void ff_hls_write_audio_rendition(AVIOContext *out, const char *agroup,
>                                     const char *filename, const char
> *language,
> -                                  int name_id, int is_default, int
> nb_channels)
> +                                  int name_id, int is_default,
> +                                  int autoselect, int nb_channels)
>   {
>       if (!out || !agroup || !filename)
>           return;
>
>       avio_printf(out, "#EXT-X-MEDIA:TYPE=AUDIO,GROUP-ID=\"group_%s\"",
> agroup);
> -    avio_printf(out, ",NAME=\"audio_%d\",DEFAULT=%s,", name_id,
> is_default ? "YES" : "NO");
> +    avio_printf(out, ",NAME=\"audio_%d\",", name_id);
> +    if (is_default) {
> +        avio_printf(out, "DEFAULT=YES,");
> +    }
Unnecessary braces,

> +    if (autoselect) {
> +        avio_printf(out, "AUTOSELECT=YES,");
> +    }
Unnecessary braces,

>       if (language) {
>           avio_printf(out, "LANGUAGE=\"%s\",", language);
>       }
Unnecessary braces,

> @@ -57,13 +64,23 @@ void ff_hls_write_audio_rendition(AVIOContext *out,
> const char *agroup,
>
>   void ff_hls_write_subtitle_rendition(AVIOContext *out, const char *sgroup,
>                                        const char *filename, const char
> *language,
> -                                     int name_id, int is_default)
> +                                     int name_id, int is_default,
> +                                     int autoselect, int forced)
>   {
>       if (!out || !filename)
>           return;
>
>       avio_printf(out, "#EXT-X-MEDIA:TYPE=SUBTITLES,GROUP-ID=\"%s\"",
> sgroup);
> -    avio_printf(out, ",NAME=\"subtitle_%d\",DEFAULT=%s,", name_id,
> is_default ? "YES" : "NO");
> +    avio_printf(out, ",NAME=\"subtitle_%d\",", name_id);
> +    if (is_default) {
> +        avio_printf(out, "DEFAULT=YES,");
> +    }
Unnecessary braces,

> +    if (autoselect) {
> +        avio_printf(out, "AUTOSELECT=YES,");
> +    }
Unnecessary braces,

> +    if (forced) {
> +        avio_printf(out, "FORCED=YES,");
> +    }
Unnecessary braces,

>       if (language) {
>           avio_printf(out, "LANGUAGE=\"%s\",", language);
>       }

> diff --git a/libavformat/hlsplaylist.h b/libavformat/hlsplaylist.h
> index d7aa44d8dc..41e640a09a 100644
> --- a/libavformat/hlsplaylist.h
> +++ b/libavformat/hlsplaylist.h
> @@ -38,10 +38,12 @@ typedef enum {
>   void ff_hls_write_playlist_version(AVIOContext *out, int version);
>   void ff_hls_write_audio_rendition(AVIOContext *out, const char *agroup,
>                                     const char *filename, const char
> *language,
> -                                  int name_id, int is_default, int
> nb_channels);
> +                                  int name_id, int is_default,
> +                                  int autoselect, int nb_channels);
>   void ff_hls_write_subtitle_rendition(AVIOContext *out, const char *sgroup,
>                                        const char *filename, const char
> *language,
> -                                     int name_id, int is_default);
> +                                     int name_id, int is_default,
> +                                     int autoselect, int forced);
>   void ff_hls_write_stream_info(AVStream *st, AVIOContext *out, int
> bandwidth,
>                                 int avg_bandwidth,
>                                 const char *filename, const char *agroup,
> --
Thans
Steven
diff mbox series

Patch

diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
index d4a6fe0304..c84b44f20d 100644
--- a/libavformat/dashenc.c
+++ b/libavformat/dashenc.c
@@ -1273,6 +1273,7 @@  static int write_manifest(AVFormatContext *s, int 
final)
              const char *audio_group = "A1";
              char audio_codec_str[128] = "\0";
              int is_default = 1;
+            int autoselect = 0;
              int max_audio_bitrate = 0;

              for (i = 0; i < s->nb_streams; i++) {
@@ -1285,7 +1286,7 @@  static int write_manifest(AVFormatContext *s, int 
final)
                      continue;
                  get_hls_playlist_name(playlist_file, 
sizeof(playlist_file), NULL, i);
                  ff_hls_write_audio_rendition(c->m3u8_out, audio_group,
-                                             playlist_file, NULL, i, 
is_default,
+                                             playlist_file, NULL, i, 
is_default, autoselect
s->streams[i]->codecpar->ch_layout.nb_channels);
                  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 274de00f9a..af4501af54 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -184,6 +184,8 @@  typedef struct VariantStream {
      unsigned int nb_streams;
      int m3u8_created; /* status of media play-list creation */
      int is_default; /* default status of audio group */
+    int autoselect; /* autoselect by system language */
+    int forced; /* forced status of subtitle stream */
      const char *language; /* audio language name */
      const char *agroup;   /* audio group name */
      const char *sgroup;   /* subtitle group name */
@@ -1434,6 +1436,31 @@  static int create_master_playlist(AVFormatContext *s,
          avio_printf(hls->m3u8_out, ",INSTREAM-ID=\"%s\"\n", 
ccs->instreamid);
      }

+    /* Check only one default is present in a group */
+    for (i = 0; i < hls->nb_varstreams; i++) {
+        vs = &(hls->var_streams[i]);
+        for (j = 0; j < hls->nb_varstreams; j++) {
+            if (i != j) {
+                temp_vs = &(hls->var_streams[j]);
+                if (vs->agroup && temp_vs->agroup &&
+                    !vs->has_video && !temp_vs->has_video) {
+                    if (!av_strcasecmp(vs->agroup, temp_vs->agroup) &&
+                        vs->is_default && temp_vs->is_default) {
+                        av_log(s, AV_LOG_ERROR, "Two streams in an 
agroup can not be default\n");
+                        goto fail;
+                    }
+                } else if (vs->sgroup && temp_vs->sgroup &&
+                            !vs->has_video && !temp_vs->has_video) {
+                    if (!av_strcasecmp(vs->sgroup, temp_vs->sgroup) &&
+                        vs->is_default && temp_vs->is_default) {
+                        av_log(s, AV_LOG_ERROR, "Two streams in an 
sgroup can not be default\n");
+                        goto fail;
+                    }
+                }
+            }
+        }
+    }
+
      /* For audio only variant streams add #EXT-X-MEDIA tag with 
attributes*/
      for (i = 0; i < hls->nb_varstreams; i++) {
          vs = &(hls->var_streams[i]);
@@ -1452,7 +1479,7 @@  static int create_master_playlist(AVFormatContext *s,
                  if (vs->streams[j]->codecpar->ch_layout.nb_channels > 
nb_channels)
                      nb_channels = 
vs->streams[j]->codecpar->ch_layout.nb_channels;

-        ff_hls_write_audio_rendition(hls->m3u8_out, vs->agroup, 
m3u8_rel_name, vs->language, i, hls->has_default_key ? vs->is_default : 
1, nb_channels);
+        ff_hls_write_audio_rendition(hls->m3u8_out, vs->agroup, 
m3u8_rel_name, vs->language, i, hls->has_default_key ? vs->is_default : 
0, vs->autoselect, nb_channels);
      }

      /* For variant streams with video add #EXT-X-STREAM-INF tag with 
attributes*/
@@ -1533,7 +1560,7 @@  static int create_master_playlist(AVFormatContext *s,
                  break;
              }

-            ff_hls_write_subtitle_rendition(hls->m3u8_out, sgroup, 
vtt_m3u8_rel_name, vs->language, i, hls->has_default_key ? 
vs->is_default : 1);
+            ff_hls_write_subtitle_rendition(hls->m3u8_out, sgroup, 
vtt_m3u8_rel_name, vs->language, i, hls->has_default_key ? 
vs->is_default : 0, vs->autoselect, vs->forced);
          }

          if (!hls->has_default_key || !hls->has_video_m3u8) {
@@ -2104,6 +2131,16 @@  static int 
parse_variant_stream_mapstring(AVFormatContext *s)
                                    (!av_strncasecmp(val, "1", 
strlen("1"))));
                  hls->has_default_key = 1;
                  continue;
+            } else if (av_strstart(keyval, "autoselect:", &val)) {
+                vs->autoselect = (!av_strncasecmp(val, "YES", 
strlen("YES")) ||
+                                  (!av_strncasecmp(val, "1", 
strlen("1")))) ||
+                                  (hls->has_default_key && vs->is_default);
+                /* autoselect must = 1 if default = 1 */
+                continue;
+            } else if (av_strstart(keyval, "forced:", &val)) {
+                vs->forced     = (!av_strncasecmp(val, "YES", 
strlen("YES")) ||
+                                  (!av_strncasecmp(val, "1", 
strlen("1"))));
+                continue;
              } else if (av_strstart(keyval, "name:", &val)) {
                  vs->varname  = val;
                  continue;
diff --git a/libavformat/hlsplaylist.c b/libavformat/hlsplaylist.c
index f8a6977702..40a8a36849 100644
--- a/libavformat/hlsplaylist.c
+++ b/libavformat/hlsplaylist.c
@@ -39,13 +39,20 @@  void ff_hls_write_playlist_version(AVIOContext *out, 
int version)

  void ff_hls_write_audio_rendition(AVIOContext *out, const char *agroup,
                                    const char *filename, const char 
*language,
-                                  int name_id, int is_default, int 
nb_channels)
+                                  int name_id, int is_default,
+                                  int autoselect, int nb_channels)
  {
      if (!out || !agroup || !filename)
          return;

      avio_printf(out, "#EXT-X-MEDIA:TYPE=AUDIO,GROUP-ID=\"group_%s\"", 
agroup);
-    avio_printf(out, ",NAME=\"audio_%d\",DEFAULT=%s,", name_id, 
is_default ? "YES" : "NO");
+    avio_printf(out, ",NAME=\"audio_%d\",", name_id);
+    if (is_default) {
+        avio_printf(out, "DEFAULT=YES,");
+    }
+    if (autoselect) {
+        avio_printf(out, "AUTOSELECT=YES,");
+    }
      if (language) {
          avio_printf(out, "LANGUAGE=\"%s\",", language);
      }
@@ -57,13 +64,23 @@  void ff_hls_write_audio_rendition(AVIOContext *out, 
const char *agroup,

  void ff_hls_write_subtitle_rendition(AVIOContext *out, const char *sgroup,
                                       const char *filename, const char 
*language,
-                                     int name_id, int is_default)
+                                     int name_id, int is_default,
+                                     int autoselect, int forced)
  {
      if (!out || !filename)
          return;

      avio_printf(out, "#EXT-X-MEDIA:TYPE=SUBTITLES,GROUP-ID=\"%s\"", 
sgroup);
-    avio_printf(out, ",NAME=\"subtitle_%d\",DEFAULT=%s,", name_id, 
is_default ? "YES" : "NO");
+    avio_printf(out, ",NAME=\"subtitle_%d\",", name_id);
+    if (is_default) {
+        avio_printf(out, "DEFAULT=YES,");
+    }
+    if (autoselect) {
+        avio_printf(out, "AUTOSELECT=YES,");
+    }
+    if (forced) {
+        avio_printf(out, "FORCED=YES,");
+    }
      if (language) {
          avio_printf(out, "LANGUAGE=\"%s\",", language);
      }
diff --git a/libavformat/hlsplaylist.h b/libavformat/hlsplaylist.h
index d7aa44d8dc..41e640a09a 100644
--- a/libavformat/hlsplaylist.h
+++ b/libavformat/hlsplaylist.h
@@ -38,10 +38,12 @@  typedef enum {
  void ff_hls_write_playlist_version(AVIOContext *out, int version);
  void ff_hls_write_audio_rendition(AVIOContext *out, const char *agroup,
                                    const char *filename, const char 
*language,
-                                  int name_id, int is_default, int 
nb_channels);
+                                  int name_id, int is_default,
+                                  int autoselect, int nb_channels);
  void ff_hls_write_subtitle_rendition(AVIOContext *out, const char *sgroup,
                                       const char *filename, const char 
*language,
-                                     int name_id, int is_default);
+                                     int name_id, int is_default,
+                                     int autoselect, int forced);