[FFmpeg-devel,v3] avformat/hls: Fixed incorrect behaviour of default

Hi,
I separated both into different for loops, and changed the order of some checks so they aren't unnecessarily done. Attached patch to this mail.
Thanks,
vckt
From 1ccc029801699c21db2fad8e20ddfab226068c37 Mon Sep 17 00:00:00 2001
From: vckt <codervenkat@gmail.com>
Date: Fri, 5 Jul 2024 18:51:32 +0530
Subject: [PATCH v3] avformat/hls: Fixed incorrect behaviour of default
 setting, added autoselect and forced

In absence of defualt 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      | 47 +++++++++++++++++++++++++++++++++++----
 libavformat/hlsplaylist.c | 26 +++++++++++++++-------
 libavformat/hlsplaylist.h |  6 +++--
 4 files changed, 67 insertions(+), 15 deletions(-)

Apologies
Correct file attached in this mail.
From 631bdac055935be6f2dfb7c6e227098dae62eb6e Mon Sep 17 00:00:00 2001
From: vckt <codervenkat@gmail.com>
Date: Fri, 5 Jul 2024 18:51:32 +0530
Subject: [PATCH v3] avformat/hls: Fixed incorrect behaviour of default
 setting, added autoselect and forced

In absence of defualt 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      | 60 ++++++++++++++++++++++++++++++++++++---
 libavformat/hlsplaylist.c | 26 +++++++++++------
 libavformat/hlsplaylist.h |  6 ++--
 4 files changed, 80 insertions(+), 15 deletions(-)

diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
index d4a6fe0304..898a227cbe 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..ac4f48d36c 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,44 @@ static int create_master_playlist(AVFormatContext *s,
         avio_printf(hls->m3u8_out, ",INSTREAM-ID=\"%s\"\n", ccs->instreamid);
     }
 
+    /* Check only one default audio stream is present in a group */
+    for (i = 0; i < hls->nb_varstreams; i++) {
+        vs = &(hls->var_streams[i]);
+        if (vs->agroup && !vs->has_video) {
+            for (j = 0; j < hls->nb_varstreams; j++) {
+                if (i != j) {
+                    temp_vs = &(hls->var_streams[j]);
+                    if (temp_vs->agroup && !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;
+                        }
+                    }
+                }
+            }
+        }
+    }
+
+    /* Check only one default subtitle stream present in a group */
+    for (i = 0; i < hls->nb_varstreams; i++) {
+        vs = &(hls->var_streams[i]);
+        if (vs->sgroup && !vs->has_video) {
+            for (j = 0; j < hls->nb_varstreams; j++) {
+                if (i != j) {
+                    temp_vs = &(hls->var_streams[j]);
+                    if (temp_vs->sgroup && !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 +1492,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 +1573,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) {
@@ -2032,6 +2072,8 @@ static int parse_variant_stream_mapstring(AVFormatContext *s)
     int nb_varstreams = 0, nb_streams;
     char *p, *q, *saveptr1, *saveptr2, *varstr, *keyval;
     const char *val;
+    const size_t strlen_yes = strlen("YES");
+    const size_t strlen_1 = strlen("1"); /* used in multiple arguments */
 
     /**
      * Expected format for var_stream_map string is as below:
@@ -2100,10 +2142,20 @@ static int parse_variant_stream_mapstring(AVFormatContext *s)
                 vs->language = val;
                 continue;
             } else if (av_strstart(keyval, "default:", &val)) {
-                vs->is_default = (!av_strncasecmp(val, "YES", strlen("YES")) ||
-                                  (!av_strncasecmp(val, "1", strlen("1"))));
+                vs->is_default = (!av_strncasecmp(val, "YES", strlen_yes) ||
+                                  (!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..094bb56689 100644
--- a/libavformat/hlsplaylist.c
+++ b/libavformat/hlsplaylist.c
@@ -39,16 +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");
-    if (language) {
+    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);
-    }
     if (nb_channels) {
         avio_printf(out, "CHANNELS=\"%d\",", nb_channels);
     }
@@ -57,16 +61,22 @@ 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");
-    if (language) {
+    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);
-    }
     avio_printf(out, "URI=\"%s\"\n", filename);
 }
 
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,

CoderVenkat <codervenkat@gmail.com> 于2024年7月15日周一 00:07写道：
>
> Apologies
> Correct file attached in this mail._______________________________________________
I need more time look at the deep for and if logic.


Thanks
Steven

Steven Liu <lingjiujianke@gmail.com> 于2024年7月15日周一 21:47写道：
>
> CoderVenkat <codervenkat@gmail.com> 于2024年7月15日周一 00:07写道：
> >
> > Apologies
> > Correct file attached in this mail._______________________________________________
> I need more time look at the deep for and if logic.
Hi CoderVenkat,

+    /* Check only one default audio stream is present in a group */
+    for (i = 0; i < hls->nb_varstreams; i++) {
+        vs = &(hls->var_streams[i]);
+        if (vs->agroup && !vs->has_video) {
+            for (j = 0; j < hls->nb_varstreams; j++) {
+                if (i != j) {
+                    temp_vs = &(hls->var_streams[j]);
+                    if (temp_vs->agroup && !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;
+                        }
+                    }
+                }
+            }
+        }
+    }

Can this logic modify as bellow?

int has_default = 0;
for (i = 0; i < hls->nb_varstreams; i++) {
    vs = &(hls->var_streams[i]);
    if (vs->is_default == 1)
        has_default++;
    if (vs->agroup && !vs->has_video && has_default > 1) {
        av_log(s, AV_LOG_ERROR, "Two streams in an agroup can not be
default\n");
        has_default = 0;
        goto fail;
   }
}
has_default = 0;

Thanks
Steven