From: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> To: ffmpeg-devel@ffmpeg.org Subject: Re: [FFmpeg-devel] [PATCH 1/2] avformat/hls demuxer: Add WebVTT subtitle support Date: Fri, 21 Feb 2025 10:18:02 +0100 Message-ID: <AS8P250MB0744730C91BF3EEA971F36268FC72@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM> (raw) In-Reply-To: <c2e1404a4277e42b06b390b0550c71a30fcd2db6.1739974227.git.ffmpegagent@gmail.com> softworkz: > From: softworkz <softworkz@hotmail.com> > > This add support for WebVTT subtitles in HLS streams. > Just like for separate audio streams, it supports all available > WebVTT streams in all renditions. > No new options are added, it just works and provides subtitles streams > like any other demuxer. > The code prevents downloading subtitle segments which are farther > in the future than the main segments, to avoid loading hundreds > of subtitle segments in advance. > > Signed-off-by: softworkz <softworkz@hotmail.com> > --- > libavformat/hls.c | 218 ++++++++++++++++++++++++++++++++++++++-------- > 1 file changed, 180 insertions(+), 38 deletions(-) > > diff --git a/libavformat/hls.c b/libavformat/hls.c > index 3bdc1bc848..51cf013a66 100644 > --- a/libavformat/hls.c > +++ b/libavformat/hls.c > @@ -56,6 +56,8 @@ > #define MPEG_TIME_BASE 90000 > #define MPEG_TIME_BASE_Q (AVRational){1, MPEG_TIME_BASE} > > +static char *vtt_sample = "WEBVTT\n"; Missing const. And actually it is simpler to avoid this pointer altogether and just use "WEBVTT\n" below. > + > /* > * An apple http stream consists of a playlist with media segment files, > * played sequentially. There may be several playlists with the same > @@ -173,6 +175,7 @@ struct playlist { > * playlist, if any. */ > int n_init_sections; > struct segment **init_sections; > + int is_subtitle; /* Indicates if it's a subtitle playlist */ > }; > > /* > @@ -330,6 +333,7 @@ static struct playlist *new_playlist(HLSContext *c, const char *url, > return NULL; > } > pls->seek_timestamp = AV_NOPTS_VALUE; > + pls->is_subtitle = 0; Is pls not zero-allocated? > > pls->is_id3_timestamped = -1; > pls->id3_mpegts_timestamp = AV_NOPTS_VALUE; > @@ -515,13 +519,6 @@ static struct rendition *new_rendition(HLSContext *c, struct rendition_info *inf > return NULL; > } > > - /* TODO: handle subtitles (each segment has to parsed separately) */ > - if (c->ctx->strict_std_compliance > FF_COMPLIANCE_EXPERIMENTAL) > - if (type == AVMEDIA_TYPE_SUBTITLE) { > - av_log(c->ctx, AV_LOG_WARNING, "Can't support the subtitle(uri: %s)\n", info->uri); > - return NULL; > - } > - > rend = av_mallocz(sizeof(struct rendition)); > if (!rend) > return NULL; > @@ -536,9 +533,14 @@ static struct rendition *new_rendition(HLSContext *c, struct rendition_info *inf > /* add the playlist if this is an external rendition */ > if (info->uri[0]) { > rend->playlist = new_playlist(c, info->uri, url_base); > - if (rend->playlist) > + if (rend->playlist) { > dynarray_add(&rend->playlist->renditions, > - &rend->playlist->n_renditions, rend); > + &rend->playlist->n_renditions, rend); Don't change indentation in this patch. > + if (type == AVMEDIA_TYPE_SUBTITLE) { > + rend->playlist->is_subtitle = 1; > + rend->playlist->is_id3_timestamped = 0; > + } Nit: Put this block before dynarray_add(). > + } > } > > if (info->assoc_language[0]) { > @@ -761,6 +763,9 @@ static int test_segment(AVFormatContext *s, const AVInputFormat *in_fmt, struct > } else if (!strcmp(in_fmt->name, "mpegts")) { > matchF = av_match_ext( seg->url, "ts,m2t,m2ts,mts,mpg,m4s,mpeg,mpegts") > + 2*(ff_match_url_ext(seg->url, "ts,m2t,m2ts,mts,mpg,m4s,mpeg,mpegts") > 0); > + } else if (!strcmp(in_fmt->name, "webvtt")) { > + matchF = av_match_ext( seg->url, "vtt,webvtt") > + + 2*(ff_match_url_ext(seg->url, "vtt,webvtt") > 0); > } > > if (!(matchA & matchF)) { > @@ -969,7 +974,7 @@ static int parse_playlist(HLSContext *c, const char *url, > if (ptr) > seg_offset = strtoll(ptr+1, NULL, 10); > } else if (av_strstart(line, "#", NULL)) { > - av_log(c->ctx, AV_LOG_INFO, "Skip ('%s')\n", line); > + av_log(c->ctx, AV_LOG_VERBOSE, "Skip ('%s')\n", line); > continue; > } else if (line[0]) { > if (is_variant) { > @@ -1484,7 +1489,7 @@ static int playlist_needed(struct playlist *pls) > int first_st; > > /* If there is no context or streams yet, the playlist is needed */ > - if (!pls->ctx || !pls->n_main_streams) > + if ((!pls->ctx || !pls->n_main_streams) && !pls->is_subtitle) > return 1; > > /* check if any of the streams in the playlist are needed */ > @@ -1522,17 +1527,13 @@ static int playlist_needed(struct playlist *pls) > return 0; > } > > -static int read_data(void *opaque, uint8_t *buf, int buf_size) > +static int reload_playlist(struct playlist *v, HLSContext *c) > { > - struct playlist *v = opaque; > - HLSContext *c = v->parent->priv_data; > - int ret; > - int just_opened = 0; > + int ret = 0; > int reload_count = 0; > - int segment_retries = 0; > - struct segment *seg; > > -restart: > + v->needed = playlist_needed(v); > + > if (!v->needed) > return AVERROR_EOF; > > @@ -1588,7 +1589,7 @@ reload: > av_log(v->parent, AV_LOG_WARNING, "The m3u8 list sequence may have been wrapped.\n"); > } > if (v->cur_seq_no >= v->start_seq_no + v->n_segments) { > - if (v->finished) > + if (v->finished || v->is_subtitle) > return AVERROR_EOF; > while (av_gettime_relative() - v->last_load_time < reload_interval) { > if (ff_check_interrupt(c->interrupt_callback)) > @@ -1599,9 +1600,35 @@ reload: > goto reload; > } > > - v->input_read_done = 0; > - seg = current_segment(v); > + } > + return ret; > +} > + > +static int read_data_continuous(void *opaque, uint8_t *buf, int buf_size) > +{ > + struct playlist *v = opaque; > + HLSContext *c = v->parent->priv_data; > + int ret; > + int just_opened = 0; > + int segment_retries = 0; > + struct segment *seg; > + > + if (c->http_persistent && v->input_read_done) { > + ret = reload_playlist(v, c); > + if (ret < 0) > + return ret; > + } > + > + v->input_read_done = 0; > + > +restart: > + ret = reload_playlist(v, c); > + if (ret < 0) > + return ret; > > + seg = current_segment(v); > + > + if (!v->input || (c->http_persistent && v->input_read_done)) { > /* load/update Media Initialization Section, if any */ > ret = update_init_section(v, seg); > if (ret) > @@ -1630,7 +1657,7 @@ reload: > } else { > segment_retries++; > } > - goto reload; > + goto restart; > } > segment_retries = 0; > just_opened = 1; > @@ -1692,6 +1719,110 @@ reload: > goto restart; > } > > +static int read_data_subtitle_segment(void *opaque, uint8_t *buf, int buf_size) > +{ > + struct playlist *v = opaque; > + HLSContext *c = v->parent->priv_data; > + int ret; > + struct segment *seg; > + > + if (!v->needed || v->cur_seq_no - v->start_seq_no >= v->n_segments) { > + return AVERROR_EOF; > + } else { > + seg = current_segment(v); > + } > + > + if (!v->input) { > + ret = open_input(c, v, seg, &v->input); > + if (ret < 0) { > + if (ff_check_interrupt(c->interrupt_callback)) > + return AVERROR_EXIT; > + av_log(v->parent, AV_LOG_WARNING, "Failed to open segment of playlist %d\n", > + v->index); > + return ret; > + } > + } > + > + return read_from_url(v, seg, buf, buf_size); > +} > + > +static int nested_io_open(AVFormatContext *s, AVIOContext **pb, const char *url, > + int flags, AVDictionary **opts) > +{ > + av_log(s, AV_LOG_ERROR, > + "A HLS playlist item '%s' referred to an external file '%s'. " > + "Opening this file was forbidden for security reasons\n", > + s->url, url); > + return AVERROR(EPERM); > +} > + > +static int init_subtitle_context(struct playlist *pls) > +{ > + HLSContext *c = pls->parent->priv_data; > + const AVInputFormat *in_fmt; > + AVDictionary *opts = NULL; > + int ret; > + > + if (!(pls->ctx = avformat_alloc_context())) > + return AVERROR(ENOMEM); > + > + pls->read_buffer = av_malloc(INITIAL_BUFFER_SIZE); > + if (!pls->read_buffer) { > + avformat_free_context(pls->ctx); > + pls->ctx = NULL; > + return AVERROR(ENOMEM); > + } > + > + ffio_init_context(&pls->pb, pls->read_buffer, INITIAL_BUFFER_SIZE, 0, pls, > + read_data_subtitle_segment, NULL, NULL); > + pls->pb.pub.seekable = 0; > + pls->ctx->pb = &pls->pb.pub; > + pls->ctx->io_open = nested_io_open; > + > + ret = ff_copy_whiteblacklists(pls->ctx, pls->parent); > + if (ret < 0) > + return ret; > + > + in_fmt = av_find_input_format("webvtt"); > + av_dict_copy(&opts, c->seg_format_opts, 0); > + ret = avformat_open_input(&pls->ctx, current_segment(pls)->url, in_fmt, &opts); > + av_dict_free(&opts); > + > + return ret; > +} > + > +static int read_subtitle_packet(struct playlist *v, AVPacket *pkt) > +{ > + HLSContext *c = v->parent->priv_data; > + int ret; > + > +restart: > + ret = reload_playlist(v, c); > + if (ret < 0) > + return ret; > + > + if (v->input && !v->ctx) > + ff_format_io_close(v->parent, &v->input); > + > + if (!v->input && !v->ctx) { > + ret = init_subtitle_context(v); > + if (ret < 0) > + return ret; > + } > + > + ret = av_read_frame(v->ctx, v->pkt); > + if (!ret) { > + return ret; > + } > + ff_format_io_close(v->parent, &v->input); > + v->cur_seq_no++; > + c->cur_seq_no = v->cur_seq_no; > + > + avformat_close_input(&v->ctx); > + > + goto restart; > +} > + > static void add_renditions_to_variant(HLSContext *c, struct variant *var, > enum AVMediaType type, const char *group_id) > { > @@ -1853,16 +1984,6 @@ static int64_t select_cur_seq_no(HLSContext *c, struct playlist *pls) > return pls->start_seq_no; > } > > -static int nested_io_open(AVFormatContext *s, AVIOContext **pb, const char *url, > - int flags, AVDictionary **opts) > -{ > - av_log(s, AV_LOG_ERROR, > - "A HLS playlist item '%s' referred to an external file '%s'. " > - "Opening this file was forbidden for security reasons\n", > - s->url, url); > - return AVERROR(EPERM); > -} > - > static void add_stream_to_programs(AVFormatContext *s, struct playlist *pls, AVStream *stream) > { > HLSContext *c = s->priv_data; > @@ -2070,6 +2191,8 @@ static int hls_read_header(AVFormatContext *s) > highest_cur_seq_no = FFMAX(highest_cur_seq_no, pls->cur_seq_no); > } > > + av_dict_set(&c->seg_format_opts, "prefer_hls_mpegts_pts", "1", 0); > + > /* Open the demuxer for each playlist */ > for (i = 0; i < c->n_playlists; i++) { > struct playlist *pls = c->playlists[i]; > @@ -2107,8 +2230,12 @@ static int hls_read_header(AVFormatContext *s) > return AVERROR(ENOMEM); > } > > - ffio_init_context(&pls->pb, pls->read_buffer, INITIAL_BUFFER_SIZE, 0, pls, > - read_data, NULL, NULL); > + if (pls->is_subtitle) > + ffio_init_context(&pls->pb, (unsigned char*)av_strdup(vtt_sample), (int)strlen(vtt_sample), 0, pls, > + NULL, NULL, NULL); > + else > + ffio_init_context(&pls->pb, pls->read_buffer, INITIAL_BUFFER_SIZE, 0, pls, > + read_data_continuous, NULL, NULL); 1. Unchecked av_strdup(). 2. Is duplicating the string even needed? Can't we simply set the AVIOContext to NULL before closing the AVFormatContext? > > /* > * If encryption scheme is SAMPLE-AES, try to read ID3 tags of > @@ -2254,6 +2381,13 @@ static int hls_read_header(AVFormatContext *s) > if (pls->n_main_streams) > av_dict_copy(&pls->main_streams[0]->metadata, pls->ctx->metadata, 0); > > + if (pls->is_subtitle) { > + avformat_free_context(pls->ctx); Doesn't the copy of vtt_sample leak here? > + pls->ctx = NULL; > + pls->needed = 0; > + pls->main_streams[0]->discard = AVDISCARD_ALL; > + } > + > add_metadata_from_renditions(s, pls, AVMEDIA_TYPE_AUDIO); > add_metadata_from_renditions(s, pls, AVMEDIA_TYPE_VIDEO); > add_metadata_from_renditions(s, pls, AVMEDIA_TYPE_SUBTITLE); > @@ -2296,6 +2430,8 @@ static int recheck_discard_flags(AVFormatContext *s, int first) > pls->input_read_done = 0; > ff_format_io_close(pls->parent, &pls->input_next); > pls->input_next_requested = 0; > + if (pls->is_subtitle) > + avformat_close_input(&pls->ctx); > pls->needed = 0; > changed = 1; > av_log(s, AV_LOG_INFO, "No longer receiving playlist %d\n", i); > @@ -2363,7 +2499,10 @@ static int hls_read_packet(AVFormatContext *s, AVPacket *pkt) > int64_t ts_diff; > AVRational tb; > struct segment *seg = NULL; > - ret = av_read_frame(pls->ctx, pls->pkt); > + if (pls->is_subtitle) > + ret = read_subtitle_packet(pls, pls->pkt); > + else > + ret = av_read_frame(pls->ctx, pls->pkt); > if (ret < 0) { > if (!avio_feof(&pls->pb.pub) && ret != AVERROR_EOF) > return ret; > @@ -2559,7 +2698,10 @@ static int hls_read_seek(AVFormatContext *s, int stream_index, > /* Reset the pos, to let the mpegts/mov demuxer know we've seeked. */ > pb->pos = 0; > /* Flush the packet queue of the subdemuxer. */ > - ff_read_frame_flush(pls->ctx); > + if (pls->ctx) > + ff_read_frame_flush(pls->ctx); > + if (pls->is_subtitle) > + avformat_close_input(&pls->ctx); > > /* Reset the init segment so it's re-fetched and served appropiately */ > pls->cur_init_section = NULL; > @@ -2628,7 +2770,7 @@ static const AVOption hls_options[] = { > OFFSET(prefer_x_start), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, FLAGS}, > {"allowed_extensions", "List of file extensions that hls is allowed to access", > OFFSET(allowed_extensions), AV_OPT_TYPE_STRING, > - {.str = "3gp,aac,avi,ac3,eac3,flac,mkv,m3u8,m4a,m4s,m4v,mpg,mov,mp2,mp3,mp4,mpeg,mpegts,ogg,ogv,oga,ts,vob,wav"}, > + {.str = "3gp,aac,avi,ac3,eac3,flac,mkv,m3u8,m4a,m4s,m4v,mpg,mov,mp2,mp3,mp4,mpeg,mpegts,ogg,ogv,oga,ts,vob,vtt,wav,webvtt"}, > INT_MIN, INT_MAX, FLAGS}, > {"extension_picky", "Be picky with all extensions matching", > OFFSET(extension_picky), AV_OPT_TYPE_BOOL, {.i64 = 1}, 0, 1, FLAGS}, _______________________________________________ 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".
next prev parent reply other threads:[~2025-02-21 9:18 UTC|newest] Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top 2025-02-19 14:10 [FFmpeg-devel] [PATCH 0/2] " ffmpegagent 2025-02-19 14:10 ` [FFmpeg-devel] [PATCH 1/2] " softworkz 2025-02-21 9:18 ` Andreas Rheinhardt [this message] 2025-02-21 9:23 ` Soft Works 2025-02-21 11:56 ` Soft Works 2025-02-19 14:10 ` [FFmpeg-devel] [PATCH 2/2] avformat/webvttdec: Add webvtt extension and MIME type softworkz 2025-02-21 15:13 ` [FFmpeg-devel] [PATCH v2 0/2] avformat/hls demuxer: Add WebVTT subtitle support ffmpegagent 2025-02-21 15:13 ` [FFmpeg-devel] [PATCH v2 1/2] " softworkz 2025-02-21 15:13 ` [FFmpeg-devel] [PATCH v2 2/2] avformat/webvttdec: Add webvtt extension and MIME type softworkz
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=AS8P250MB0744730C91BF3EEA971F36268FC72@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM \ --to=andreas.rheinhardt@outlook.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