Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: Steven Liu <lingjiujianke@gmail.com>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH] avformat/hls: Fixed incorrect behaviour of default setting, , added autoselect and forced
Date: Sun, 7 Jul 2024 18:18:41 +0800
Message-ID: <CADxeRwmG-tbN8gyZPOEcykKYSWn2AjUTz5YG1-38sPtN-HGyXw@mail.gmail.com> (raw)
In-Reply-To: <691930bc-b3e6-8185-2a7e-cf90462d4376@gmail.com>

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
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

  reply	other threads:[~2024-07-07 10:19 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-05 13:27 CoderVenkat
2024-07-07 10:18 ` Steven Liu [this message]
2024-07-08 13:43   ` [FFmpeg-devel] [PATCH v2] avformat/hls: Fixed incorrect behaviour of default CoderVenkat
2024-07-14  8:21     ` CoderVenkat
2024-07-14 12:04       ` Steven Liu
2024-07-14 15:16         ` [FFmpeg-devel] [PATCH v3] " CoderVenkat
2024-07-14 16:06           ` CoderVenkat
2024-07-15 13:47             ` Steven Liu
2024-07-16 10:59               ` Steven Liu
2024-07-05 13:32 [FFmpeg-devel] [PATCH] avformat/hls: Fixed incorrect behaviour of default setting, , added autoselect and forced CoderVenkat
2024-07-05 13:36 CoderVenkat
2024-07-05 18:04 CoderVenkat

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CADxeRwmG-tbN8gyZPOEcykKYSWn2AjUTz5YG1-38sPtN-HGyXw@mail.gmail.com \
    --to=lingjiujianke@gmail.com \
    --cc=ffmpeg-devel@ffmpeg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git