| Message ID | 20171126104620.2241-1-fernetmenta@online.de |
|---|---|
| State | Superseded |
| Headers | show |
2017-11-26 18:46 GMT+08:00 Rainer Hochecker <fernetmenta@online.de>: > fixed mem leak poined out by Steven Hi Rainer, I'm not sure that is memleak, but looks like memleak when reading the code, i see the code always in hls.c before this patch, but no people report it memleak. If that is memleak, maybe use goto method is better way, because the workflow of bellow have alloc resource faild check, i will point out base on your patch. > > --- > doc/demuxers.texi | 5 + > libavformat/hls.c | 304 ++++++++++++++++++++++++++++++++++++------------------ > 2 files changed, 209 insertions(+), 100 deletions(-) > > diff --git a/doc/demuxers.texi b/doc/demuxers.texi > index 73dc0feec1..634b122e10 100644 > --- a/doc/demuxers.texi > +++ b/doc/demuxers.texi > @@ -316,6 +316,11 @@ segment index to start live streams at (negative values are from the end). > @item max_reload > Maximum number of times a insufficient list is attempted to be reloaded. > Default value is 1000. > + > +@item load_all_variants > +If 0, only the first variant/playlist is loaded on open. All other variants > +get disabled and can be enabled by setting discard option in program. > +Default value is 1. > @end table > > @section image2 > diff --git a/libavformat/hls.c b/libavformat/hls.c > index 786934af03..c42e0b0f95 100644 > --- a/libavformat/hls.c > +++ b/libavformat/hls.c > @@ -112,6 +112,7 @@ struct playlist { > int n_segments; > struct segment **segments; > int needed, cur_needed; > + int parsed; > int cur_seq_no; > int64_t cur_seg_offset; > int64_t last_load_time; > @@ -206,6 +207,7 @@ typedef struct HLSContext { > int strict_std_compliance; > char *allowed_extensions; > int max_reload; > + int load_all_variants; > } HLSContext; > > static int read_chomp_line(AVIOContext *s, char *buf, int maxlen) > @@ -314,6 +316,9 @@ static struct playlist *new_playlist(HLSContext *c, const char *url, > pls->is_id3_timestamped = -1; > pls->id3_mpegts_timestamp = AV_NOPTS_VALUE; > > + pls->index = c->n_playlists; > + pls->parsed = 0; > + pls->needed = 0; > dynarray_add(&c->playlists, &c->n_playlists, pls); > return pls; > } > @@ -721,6 +726,7 @@ static int parse_playlist(HLSContext *c, const char *url, > free_segment_list(pls); > pls->finished = 0; > pls->type = PLS_TYPE_UNSPECIFIED; > + pls->parsed = 1; > } > while (!avio_feof(in)) { > read_chomp_line(in, line, sizeof(line)); > @@ -1377,23 +1383,41 @@ reload: > static void add_renditions_to_variant(HLSContext *c, struct variant *var, > enum AVMediaType type, const char *group_id) > { > - int i; > + int i, j; > + int found; > > for (i = 0; i < c->n_renditions; i++) { > struct rendition *rend = c->renditions[i]; > > if (rend->type == type && !strcmp(rend->group_id, group_id)) { > > - if (rend->playlist) > + if (rend->playlist) { > /* rendition is an external playlist > * => add the playlist to the variant */ > - dynarray_add(&var->playlists, &var->n_playlists, rend->playlist); > - else > + found = 0; > + for (j = 0; j < var->n_playlists; j++) { > + if (var->playlists[j] == rend->playlist) { > + found = 1; > + break; > + } > + } > + if (!found) > + dynarray_add(&var->playlists, &var->n_playlists, rend->playlist); > + } else { > /* rendition is part of the variant main Media Playlist > * => add the rendition to the main Media Playlist */ > - dynarray_add(&var->playlists[0]->renditions, > - &var->playlists[0]->n_renditions, > - rend); > + found = 0; > + for (j = 0; j < var->playlists[0]->n_renditions; j++) { > + if (var->playlists[0]->renditions[j] == rend) { > + found = 1; > + break; > + } > + } > + if (!found) > + dynarray_add(&var->playlists[0]->renditions, > + &var->playlists[0]->n_renditions, > + rend); > + } > } > } > } > @@ -1631,6 +1655,124 @@ static int hls_close(AVFormatContext *s) > return 0; > } > > +static int init_playlist(HLSContext *c, struct playlist *pls) > +{ > + AVInputFormat *in_fmt = NULL; > + int highest_cur_seq_no = 0; > + int ret; > + int i; > + > + if (!(pls->ctx = avformat_alloc_context())) { > + return AVERROR(ENOMEM); > + } > + > + if (pls->n_segments == 0) > + return 0; > + > + pls->needed = 1; > + pls->parent = c->ctx; > + > + /* > + * If this is a live stream and this playlist looks like it is one segment > + * behind, try to sync it up so that every substream starts at the same > + * time position (so e.g. avformat_find_stream_info() will see packets from > + * all active streams within the first few seconds). This is not very generic, > + * though, as the sequence numbers are technically independent. > + */ > + highest_cur_seq_no = 0; > + for (i = 0; i < c->n_playlists; i++) { > + struct playlist *pls = c->playlists[i]; > + if (!pls->parsed) > + continue; > + if (pls->cur_seq_no > highest_cur_seq_no) > + highest_cur_seq_no = pls->cur_seq_no; > + } > + if (!pls->finished && pls->cur_seq_no == highest_cur_seq_no - 1 && > + highest_cur_seq_no < pls->start_seq_no + pls->n_segments) { > + pls->cur_seq_no = highest_cur_seq_no; > + } > + > + pls->read_buffer = av_malloc(INITIAL_BUFFER_SIZE); > + if (!pls->read_buffer){ > + ret = AVERROR(ENOMEM); > + avformat_free_context(pls->ctx); > + pls->ctx = NULL; > + return ret; > + } > + ffio_init_context(&pls->pb, pls->read_buffer, INITIAL_BUFFER_SIZE, 0, pls, > + read_data, NULL, NULL); > + pls->pb.seekable = 0; > + ret = av_probe_input_buffer(&pls->pb, &in_fmt, pls->segments[0]->url, > + NULL, 0, 0); > + if (ret < 0) { > + /* Free the ctx - it isn't initialized properly at this point, > + * so avformat_close_input shouldn't be called. If > + * avformat_open_input fails below, it frees and zeros the > + * context, so it doesn't need any special treatment like this. */ > + av_log(c->ctx, AV_LOG_ERROR, "Error when loading first segment '%s'\n", pls->segments[0]->url); > + av_free(pls->read_buffer); > + pls->read_buffer = NULL; > + avformat_free_context(pls->ctx); > + pls->ctx = NULL; > + return ret; > + } > + pls->ctx->pb = &pls->pb; > + pls->ctx->io_open = nested_io_open; > + pls->ctx->flags |= c->ctx->flags & ~AVFMT_FLAG_CUSTOM_IO; > + > + if ((ret = ff_copy_whiteblacklists(pls->ctx, c->ctx)) < 0) > + return ret; > + > + ret = avformat_open_input(&pls->ctx, pls->segments[0]->url, in_fmt, NULL); > + if (ret < 0) { > + av_log(c->ctx, AV_LOG_ERROR, "Error opening playlist %s", pls->segments[0]->url); first one. > + avformat_free_context(pls->ctx); > + pls->ctx = NULL; > + return ret; > + } > + > + if (pls->id3_deferred_extra && pls->ctx->nb_streams == 1) { > + ff_id3v2_parse_apic(pls->ctx, &pls->id3_deferred_extra); > + avformat_queue_attached_pictures(pls->ctx); > + ff_id3v2_free_extra_meta(&pls->id3_deferred_extra); > + pls->id3_deferred_extra = NULL; > + } > + > + if (pls->is_id3_timestamped == -1) > + av_log(c->ctx, AV_LOG_WARNING, "No expected HTTP requests have been made\n"); > + > + /* > + * For ID3 timestamped raw audio streams we need to detect the packet > + * durations to calculate timestamps in fill_timing_for_id3_timestamped_stream(), > + * but for other streams we can rely on our user calling avformat_find_stream_info() > + * on us if they want to. > + */ > + if (pls->is_id3_timestamped) { > + ret = avformat_find_stream_info(pls->ctx, NULL); > + if (ret < 0) { second one. > + avformat_free_context(pls->ctx); > + pls->ctx = NULL; > + return ret; > + } > + } > + > + pls->has_noheader_flag = !!(pls->ctx->ctx_flags & AVFMTCTX_NOHEADER); > + > + /* Create new AVStreams for each stream in this playlist */ > + ret = update_streams_from_subdemuxer(c->ctx, pls); > + if (ret < 0) { Third one. > + avformat_free_context(pls->ctx); > + pls->ctx = NULL; > + return ret; > + } > + > + add_metadata_from_renditions(c->ctx, pls, AVMEDIA_TYPE_AUDIO); > + add_metadata_from_renditions(c->ctx, pls, AVMEDIA_TYPE_VIDEO); > + add_metadata_from_renditions(c->ctx, pls, AVMEDIA_TYPE_SUBTITLE); > + > + return 0; > +} > + > static int hls_read_header(AVFormatContext *s) > { > void *u = (s->flags & AVFMT_FLAG_CUSTOM_IO) ? NULL : s->pb; > @@ -1663,6 +1805,9 @@ static int hls_read_header(AVFormatContext *s) > if ((ret = parse_playlist(c, s->filename, NULL, s->pb)) < 0) > goto fail; > > + /* first playlist was created, set it to parsed */ > + c->variants[0]->playlists[0]->parsed = 1; > + > if ((ret = save_avio_options(s)) < 0) > goto fail; > > @@ -1675,8 +1820,15 @@ static int hls_read_header(AVFormatContext *s) > goto fail; > } > /* If the playlist only contained playlists (Master Playlist), > - * parse each individual playlist. */ > - if (c->n_playlists > 1 || c->playlists[0]->n_segments == 0) { > + * parse all individual playlists. > + * If option load_all_variants is false, load only first variant */ > + if (!c->load_all_variants && c->n_variants > 1) { > + for (i = 0; i < c->variants[0]->n_playlists; i++) { > + struct playlist *pls = c->variants[0]->playlists[i]; > + if ((ret = parse_playlist(c, pls->url, pls, NULL)) < 0) > + goto fail; > + } > + } else if (c->n_playlists > 1 || c->playlists[0]->n_segments == 0) { > for (i = 0; i < c->n_playlists; i++) { > struct playlist *pls = c->playlists[i]; > if ((ret = parse_playlist(c, pls->url, pls, NULL)) < 0) > @@ -1720,13 +1872,17 @@ static int hls_read_header(AVFormatContext *s) > if (!program) > goto fail; > av_dict_set_int(&program->metadata, "variant_bitrate", v->bandwidth, 0); > + > + /* start with the first variant and disable all others */ > + if (i > 0 && !c->load_all_variants) > + program->discard = AVDISCARD_ALL; > } > > /* Select the starting segments */ > for (i = 0; i < c->n_playlists; i++) { > struct playlist *pls = c->playlists[i]; > > - if (pls->n_segments == 0) > + if (pls->n_segments == 0 && !pls->parsed) > continue; > > pls->cur_seq_no = select_cur_seq_no(c, pls); > @@ -1736,97 +1892,9 @@ static int hls_read_header(AVFormatContext *s) > /* Open the demuxer for each playlist */ > for (i = 0; i < c->n_playlists; i++) { > struct playlist *pls = c->playlists[i]; > - AVInputFormat *in_fmt = NULL; > - > - if (!(pls->ctx = avformat_alloc_context())) { > - ret = AVERROR(ENOMEM); > - goto fail; > - } > - > - if (pls->n_segments == 0) > - continue; > > - pls->index = i; > - pls->needed = 1; > - pls->parent = s; > - > - /* > - * If this is a live stream and this playlist looks like it is one segment > - * behind, try to sync it up so that every substream starts at the same > - * time position (so e.g. avformat_find_stream_info() will see packets from > - * all active streams within the first few seconds). This is not very generic, > - * though, as the sequence numbers are technically independent. > - */ > - if (!pls->finished && pls->cur_seq_no == highest_cur_seq_no - 1 && > - highest_cur_seq_no < pls->start_seq_no + pls->n_segments) { > - pls->cur_seq_no = highest_cur_seq_no; > - } > - > - pls->read_buffer = av_malloc(INITIAL_BUFFER_SIZE); > - if (!pls->read_buffer){ > - ret = AVERROR(ENOMEM); > - avformat_free_context(pls->ctx); > - pls->ctx = NULL; > - goto fail; > - } > - ffio_init_context(&pls->pb, pls->read_buffer, INITIAL_BUFFER_SIZE, 0, pls, > - read_data, NULL, NULL); > - pls->pb.seekable = 0; > - ret = av_probe_input_buffer(&pls->pb, &in_fmt, pls->segments[0]->url, > - NULL, 0, 0); > - if (ret < 0) { > - /* Free the ctx - it isn't initialized properly at this point, > - * so avformat_close_input shouldn't be called. If > - * avformat_open_input fails below, it frees and zeros the > - * context, so it doesn't need any special treatment like this. */ > - av_log(s, AV_LOG_ERROR, "Error when loading first segment '%s'\n", pls->segments[0]->url); > - avformat_free_context(pls->ctx); > - pls->ctx = NULL; > - goto fail; > - } > - pls->ctx->pb = &pls->pb; > - pls->ctx->io_open = nested_io_open; > - pls->ctx->flags |= s->flags & ~AVFMT_FLAG_CUSTOM_IO; > - > - if ((ret = ff_copy_whiteblacklists(pls->ctx, s)) < 0) > - goto fail; > - > - ret = avformat_open_input(&pls->ctx, pls->segments[0]->url, in_fmt, NULL); > - if (ret < 0) > - goto fail; > - > - if (pls->id3_deferred_extra && pls->ctx->nb_streams == 1) { > - ff_id3v2_parse_apic(pls->ctx, &pls->id3_deferred_extra); > - avformat_queue_attached_pictures(pls->ctx); > - ff_id3v2_free_extra_meta(&pls->id3_deferred_extra); > - pls->id3_deferred_extra = NULL; > - } > - > - if (pls->is_id3_timestamped == -1) > - av_log(s, AV_LOG_WARNING, "No expected HTTP requests have been made\n"); > - > - /* > - * For ID3 timestamped raw audio streams we need to detect the packet > - * durations to calculate timestamps in fill_timing_for_id3_timestamped_stream(), > - * but for other streams we can rely on our user calling avformat_find_stream_info() > - * on us if they want to. > - */ > - if (pls->is_id3_timestamped) { > - ret = avformat_find_stream_info(pls->ctx, NULL); > - if (ret < 0) > - goto fail; > - } > - > - pls->has_noheader_flag = !!(pls->ctx->ctx_flags & AVFMTCTX_NOHEADER); > - > - /* Create new AVStreams for each stream in this playlist */ > - ret = update_streams_from_subdemuxer(s, pls); > - if (ret < 0) > - goto fail; > - > - 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); > + if (pls->parsed) > + init_playlist(c, pls); > } > > update_noheader_flag(s); > @@ -1877,6 +1945,36 @@ static int recheck_discard_flags(AVFormatContext *s, int first) > return changed; > } > > +static void recheck_discard_programs(AVFormatContext *s) > +{ > + HLSContext *c = s->priv_data; > + int i, j; > + > + for (i = 0; i < c->n_variants; i++) { > + struct variant *var = c->variants[i]; > + AVProgram *program = s->programs[i]; > + > + if (program->discard >= AVDISCARD_ALL) > + continue; > + > + for (j = 0; j < c->variants[i]->n_playlists; j++) { > + struct playlist *pls = c->variants[i]->playlists[j]; > + > + if (!pls->parsed) { > + if (parse_playlist(c, pls->url, pls, NULL) < 0) > + continue; > + if (var->audio_group[0]) > + add_renditions_to_variant(c, var, AVMEDIA_TYPE_AUDIO, var->audio_group); > + if (var->video_group[0]) > + add_renditions_to_variant(c, var, AVMEDIA_TYPE_VIDEO, var->video_group); > + if (var->subtitles_group[0]) > + add_renditions_to_variant(c, var, AVMEDIA_TYPE_SUBTITLE, var->subtitles_group); > + init_playlist(c, pls); > + } > + } > + } > +} > + > static void fill_timing_for_id3_timestamped_stream(struct playlist *pls) > { > if (pls->id3_offset >= 0) { > @@ -1924,6 +2022,8 @@ static int hls_read_packet(AVFormatContext *s, AVPacket *pkt) > HLSContext *c = s->priv_data; > int ret, i, minplaylist = -1; > > + recheck_discard_programs(s); > + > recheck_discard_flags(s, c->first_packet); > c->first_packet = 0; > > @@ -2101,6 +2201,8 @@ static int hls_read_seek(AVFormatContext *s, int stream_index, > for (i = 0; i < c->n_playlists; i++) { > /* Reset reading */ > struct playlist *pls = c->playlists[i]; > + if (!pls->parsed) > + continue; > if (pls->input) > ff_format_io_close(pls->parent, &pls->input); > av_packet_unref(&pls->pkt); > @@ -2157,6 +2259,8 @@ static const AVOption hls_options[] = { > INT_MIN, INT_MAX, FLAGS}, > {"max_reload", "Maximum number of times a insufficient list is attempted to be reloaded", > OFFSET(max_reload), AV_OPT_TYPE_INT, {.i64 = 1000}, 0, INT_MAX, FLAGS}, > + {"load_all_variants", "if > 0 all playlists of all variants are opened at startup", > + OFFSET(load_all_variants), AV_OPT_TYPE_BOOL, {.i64 = 1}, 0, 1, FLAGS}, > {NULL} > }; > > -- > 2.14.1 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
2017-11-26 12:04 GMT+01:00 Steven Liu <lingjiujianke@gmail.com>: > 2017-11-26 18:46 GMT+08:00 Rainer Hochecker <fernetmenta@online.de>: >> fixed mem leak poined out by Steven > Hi Rainer, > > I'm not sure that is memleak, but looks like memleak when reading > the code, i see the code always in hls.c before this patch, but no > people report it memleak. > If that is memleak, maybe use goto method is better way, because > the workflow of bellow have alloc resource faild check, i will point > out base on your patch. >> Hi Steven, As soon as you associate the probe buffer with the context, the context cares about allocated resources. That's most likely the reason why pb was not cleared here. It is assigned to the context right after this block. Rainer
Hi, Rainer Hochecker kirjoitti 2017-11-26 12:46: > fixed mem leak poined out by Steven > > --- > doc/demuxers.texi | 5 + > libavformat/hls.c | 304 > ++++++++++++++++++++++++++++++++++++------------------ > 2 files changed, 209 insertions(+), 100 deletions(-) > [...] > + > +@item load_all_variants > +If 0, only the first variant/playlist is loaded on open. All other > variants > +get disabled and can be enabled by setting discard option in program. > +Default value is 1. > @end table If playlist download+parsing is indeed taking long enough time that this is warranted, I wonder if we should maybe just never read any variant playlists in read_header(), and instead set AVFMTCTX_NOHEADER. Then the user may discard AVPrograms right after open, before unneeded playlists are loaded+parsed. This would avoid having a separate option/mode for this. Any thoughts? > @section image2 > diff --git a/libavformat/hls.c b/libavformat/hls.c > index 786934af03..c42e0b0f95 100644 > --- a/libavformat/hls.c > +++ b/libavformat/hls.c > @@ -112,6 +112,7 @@ struct playlist { > int n_segments; > struct segment **segments; > int needed, cur_needed; > + int parsed; > int cur_seq_no; > int64_t cur_seg_offset; > int64_t last_load_time; > @@ -206,6 +207,7 @@ typedef struct HLSContext { > int strict_std_compliance; > char *allowed_extensions; > int max_reload; > + int load_all_variants; > } HLSContext; [...] > @@ -721,6 +726,7 @@ static int parse_playlist(HLSContext *c, const char > *url, > free_segment_list(pls); > pls->finished = 0; > pls->type = PLS_TYPE_UNSPECIFIED; > + pls->parsed = 1; > } That "pls->parsed = 1" is in the "reinit old playlist for re-parse" block, is that intentional? I'd think it should rather be at the end of the function alongside setting pls->last_load_time, so it is set to 1 whenever parsing has been done. > while (!avio_feof(in)) { > read_chomp_line(in, line, sizeof(line)); [...] > @@ -1631,6 +1655,124 @@ static int hls_close(AVFormatContext *s) > return 0; > } > > +static int init_playlist(HLSContext *c, struct playlist *pls) > +{ This init_playlist() seems to be mostly code moved from below, could you split the patch so that the first patch moves the code around but does not change behavior, and the second patch makes the actual changes (i.e. adds the option)? That would ease e.g. future bisecting. [...] > + return 0; > +} > + > static int hls_read_header(AVFormatContext *s) > { > void *u = (s->flags & AVFMT_FLAG_CUSTOM_IO) ? NULL : s->pb; > @@ -1663,6 +1805,9 @@ static int hls_read_header(AVFormatContext *s) > if ((ret = parse_playlist(c, s->filename, NULL, s->pb)) < 0) > goto fail; > > + /* first playlist was created, set it to parsed */ > + c->variants[0]->playlists[0]->parsed = 1; > + I think parse_playlist() should set this when it parses something. > if ((ret = save_avio_options(s)) < 0) > goto fail; > > @@ -1675,8 +1820,15 @@ static int hls_read_header(AVFormatContext *s) > goto fail; [...] > > /* Select the starting segments */ > for (i = 0; i < c->n_playlists; i++) { > struct playlist *pls = c->playlists[i]; > > - if (pls->n_segments == 0) > + if (pls->n_segments == 0 && !pls->parsed) > continue; Why? > pls->cur_seq_no = select_cur_seq_no(c, pls); > @@ -1736,97 +1892,9 @@ static int hls_read_header(AVFormatContext *s) > /* Open the demuxer for each playlist */ > for (i = 0; i < c->n_playlists; i++) { > struct playlist *pls = c->playlists[i]; > - AVInputFormat *in_fmt = NULL; > - [...]
2017-11-27 22:53 GMT+01:00 Anssi Hannula <anssi.hannula@iki.fi>: > Hi, > > Rainer Hochecker kirjoitti 2017-11-26 12:46: >> >> fixed mem leak poined out by Steven >> >> --- >> doc/demuxers.texi | 5 + >> libavformat/hls.c | 304 >> ++++++++++++++++++++++++++++++++++++------------------ >> 2 files changed, 209 insertions(+), 100 deletions(-) >> > [...] >> >> + >> +@item load_all_variants >> +If 0, only the first variant/playlist is loaded on open. All other >> variants >> +get disabled and can be enabled by setting discard option in program. >> +Default value is 1. >> @end table > > > If playlist download+parsing is indeed taking long enough time that this is > warranted, I wonder if we should maybe just never read any variant playlists > in read_header(), and instead set AVFMTCTX_NOHEADER. > Then the user may discard AVPrograms right after open, before unneeded > playlists are loaded+parsed. > > This would avoid having a separate option/mode for this. > > Any thoughts? I actually tried it like this but somwhow did not like it because this is different to all other demuxers/formats. In general you open an input, call find_stream_info, and select a program. I did not like selecting the program between open and find_stream_info. > > >> @section image2 >> diff --git a/libavformat/hls.c b/libavformat/hls.c >> index 786934af03..c42e0b0f95 100644 >> --- a/libavformat/hls.c >> +++ b/libavformat/hls.c >> @@ -112,6 +112,7 @@ struct playlist { >> int n_segments; >> struct segment **segments; >> int needed, cur_needed; >> + int parsed; >> int cur_seq_no; >> int64_t cur_seg_offset; >> int64_t last_load_time; >> @@ -206,6 +207,7 @@ typedef struct HLSContext { >> int strict_std_compliance; >> char *allowed_extensions; >> int max_reload; >> + int load_all_variants; >> } HLSContext; > > [...] >> >> @@ -721,6 +726,7 @@ static int parse_playlist(HLSContext *c, const char >> *url, >> free_segment_list(pls); >> pls->finished = 0; >> pls->type = PLS_TYPE_UNSPECIFIED; >> + pls->parsed = 1; >> } > > > That "pls->parsed = 1" is in the "reinit old playlist for re-parse" block, > is that intentional? > > I'd think it should rather be at the end of the function alongside setting > pls->last_load_time, so it is set to 1 whenever parsing has been done. > I put it at the beginning because I wanted to avoid that it gets tried again on error. >> while (!avio_feof(in)) { >> read_chomp_line(in, line, sizeof(line)); > > [...] >> >> @@ -1631,6 +1655,124 @@ static int hls_close(AVFormatContext *s) >> return 0; >> } >> >> +static int init_playlist(HLSContext *c, struct playlist *pls) >> +{ > > > This init_playlist() seems to be mostly code moved from below, could you > split the patch so that the first patch moves the code around but does not > change behavior, and the second patch makes the actual changes (i.e. adds > the option)? > > That would ease e.g. future bisecting. Sure. At least I can try. > > [...] >> >> + return 0; >> +} >> + >> static int hls_read_header(AVFormatContext *s) >> { >> void *u = (s->flags & AVFMT_FLAG_CUSTOM_IO) ? NULL : s->pb; >> @@ -1663,6 +1805,9 @@ static int hls_read_header(AVFormatContext *s) >> if ((ret = parse_playlist(c, s->filename, NULL, s->pb)) < 0) >> goto fail; >> >> + /* first playlist was created, set it to parsed */ >> + c->variants[0]->playlists[0]->parsed = 1; >> + > > > I think parse_playlist() should set this when it parses something. That was pitfall for my v1. The first playlist gets parsed with no playlist given as argument. fate failed because non-master playlists were not set to parsed. > >> if ((ret = save_avio_options(s)) < 0) >> goto fail; >> >> @@ -1675,8 +1820,15 @@ static int hls_read_header(AVFormatContext *s) >> goto fail; > > [...] >> >> >> /* Select the starting segments */ >> for (i = 0; i < c->n_playlists; i++) { >> struct playlist *pls = c->playlists[i]; >> >> - if (pls->n_segments == 0) >> + if (pls->n_segments == 0 && !pls->parsed) >> continue; > > > Why? playlists not parsed are invalid, right? maybe n_segments is 0 for not parsed playlists but this was no obvious to me. > >> pls->cur_seq_no = select_cur_seq_no(c, pls); >> @@ -1736,97 +1892,9 @@ static int hls_read_header(AVFormatContext *s) >> /* Open the demuxer for each playlist */ >> for (i = 0; i < c->n_playlists; i++) { >> struct playlist *pls = c->playlists[i]; >> - AVInputFormat *in_fmt = NULL; >> - > > [...] > > -- > Anssi Hannula >
Hi, Sorry about the delay. Rainer Hochecker kirjoitti 2017-11-28 00:23: > 2017-11-27 22:53 GMT+01:00 Anssi Hannula <anssi.hannula@iki.fi>: >> Hi, >> >> Rainer Hochecker kirjoitti 2017-11-26 12:46: >>> >>> fixed mem leak poined out by Steven >>> >>> --- >>> doc/demuxers.texi | 5 + >>> libavformat/hls.c | 304 >>> ++++++++++++++++++++++++++++++++++++------------------ >>> 2 files changed, 209 insertions(+), 100 deletions(-) >>> >> [...] >>> >>> + >>> +@item load_all_variants >>> +If 0, only the first variant/playlist is loaded on open. All other >>> variants >>> +get disabled and can be enabled by setting discard option in >>> program. >>> +Default value is 1. >>> @end table >> >> >> If playlist download+parsing is indeed taking long enough time that >> this is >> warranted, I wonder if we should maybe just never read any variant >> playlists >> in read_header(), and instead set AVFMTCTX_NOHEADER. >> Then the user may discard AVPrograms right after open, before unneeded >> playlists are loaded+parsed. >> >> This would avoid having a separate option/mode for this. >> >> Any thoughts? > > I actually tried it like this but somwhow did not like it because this > is different > to all other demuxers/formats. In general you open an input, call > find_stream_info, > and select a program. I did not like selecting the program between open > and > find_stream_info. OK I guess. Though arguably hls is already quite different from mpegts which is the only other demuxer that creates multiple AVPrograms. In the long run, I think I'd prefer the HLS demuxer to have a mode where only a single program/variant is given to the user at a time, with the demuxer handling the switching between the variants. But that would be a lot more work and I'm not even sure it would be feasible. So I guess your solution is OK, at least for now. Is there a reason the first variant/playlist is still parsed, i.e. why not simply not parse any media playlists (i.e. only master pls) when the new mode is selected? If the player is selecting the variant/program based on bitrate (like Kodi does), then the first playlist would likely have been downloaded+parsed unnecessarily. Also, even with your current patch you will need to set AVFMTCTX_NOHEADER as you defer avformat_new_stream() calls to read_packet(). I think you can update update_noheader_flag() to set flag_needed=1 if any pls is !pls->is_parsed. >> >> >>> @section image2 >>> diff --git a/libavformat/hls.c b/libavformat/hls.c >>> index 786934af03..c42e0b0f95 100644 >>> --- a/libavformat/hls.c >>> +++ b/libavformat/hls.c >>> @@ -112,6 +112,7 @@ struct playlist { >>> int n_segments; >>> struct segment **segments; >>> int needed, cur_needed; >>> + int parsed; >>> int cur_seq_no; >>> int64_t cur_seg_offset; >>> int64_t last_load_time; >>> @@ -206,6 +207,7 @@ typedef struct HLSContext { >>> int strict_std_compliance; >>> char *allowed_extensions; >>> int max_reload; >>> + int load_all_variants; >>> } HLSContext; >> >> [...] >>> >>> @@ -721,6 +726,7 @@ static int parse_playlist(HLSContext *c, const >>> char >>> *url, >>> free_segment_list(pls); >>> pls->finished = 0; >>> pls->type = PLS_TYPE_UNSPECIFIED; >>> + pls->parsed = 1; >>> } >> >> >> That "pls->parsed = 1" is in the "reinit old playlist for re-parse" >> block, >> is that intentional? >> >> I'd think it should rather be at the end of the function alongside >> setting >> pls->last_load_time, so it is set to 1 whenever parsing has been done. >> > > I put it at the beginning because I wanted to avoid that it gets tried > again > on error. OK. But now it is not set for master playlists, so maybe move pls->parsed = 1 below the "fail" label then? >>> while (!avio_feof(in)) { >>> read_chomp_line(in, line, sizeof(line)); >> >> [...] >>> >>> @@ -1631,6 +1655,124 @@ static int hls_close(AVFormatContext *s) >>> return 0; >>> } >>> >>> +static int init_playlist(HLSContext *c, struct playlist *pls) >>> +{ >> >> >> This init_playlist() seems to be mostly code moved from below, could >> you >> split the patch so that the first patch moves the code around but does >> not >> change behavior, and the second patch makes the actual changes (i.e. >> adds >> the option)? >> >> That would ease e.g. future bisecting. > > Sure. At least I can try. > >> >> [...] >>> >>> + return 0; >>> +} >>> + >>> static int hls_read_header(AVFormatContext *s) >>> { >>> void *u = (s->flags & AVFMT_FLAG_CUSTOM_IO) ? NULL : s->pb; >>> @@ -1663,6 +1805,9 @@ static int hls_read_header(AVFormatContext *s) >>> if ((ret = parse_playlist(c, s->filename, NULL, s->pb)) < 0) >>> goto fail; >>> >>> + /* first playlist was created, set it to parsed */ >>> + c->variants[0]->playlists[0]->parsed = 1; >>> + >> >> >> I think parse_playlist() should set this when it parses something. > > That was pitfall for my v1. The first playlist gets parsed with no > playlist given as argument. > fate failed because non-master playlists were not set to parsed. I don't think I follow. If parse_playlist() would set the playlist as parsed on every call (whether failed or not), then all playlists would be set to parsed and this line would be unnecessary. >> >>> if ((ret = save_avio_options(s)) < 0) >>> goto fail; >>> >>> @@ -1675,8 +1820,15 @@ static int hls_read_header(AVFormatContext *s) >>> goto fail; >> >> [...] >>> >>> >>> /* Select the starting segments */ >>> for (i = 0; i < c->n_playlists; i++) { >>> struct playlist *pls = c->playlists[i]; >>> >>> - if (pls->n_segments == 0) >>> + if (pls->n_segments == 0 && !pls->parsed) >>> continue; >> >> >> Why? > > playlists not parsed are invalid, right? maybe n_segments is 0 for > not parsed playlists but this was no obvious to me. Yes, but after your change any parsed zero-segment playlists will get select_cur_seq_no() called while they didn't before. [...] > return changed; > } > > +static void recheck_discard_programs(AVFormatContext *s) > +{ > + HLSContext *c = s->priv_data; > + int i, j; > + > + for (i = 0; i < c->n_variants; i++) { > + struct variant *var = c->variants[i]; > + AVProgram *program = s->programs[i]; > + > + if (program->discard >= AVDISCARD_ALL) > + continue; > + > + for (j = 0; j < c->variants[i]->n_playlists; j++) { > + struct playlist *pls = c->variants[i]->playlists[j]; > + > + if (!pls->parsed) { > + if (parse_playlist(c, pls->url, pls, NULL) < 0) > + continue; > + if (var->audio_group[0]) > + add_renditions_to_variant(c, var, > AVMEDIA_TYPE_AUDIO, var->audio_group); > + if (var->video_group[0]) > + add_renditions_to_variant(c, var, > AVMEDIA_TYPE_VIDEO, var->video_group); > + if (var->subtitles_group[0]) > + add_renditions_to_variant(c, var, > AVMEDIA_TYPE_SUBTITLE, var->subtitles_group); > + init_playlist(c, pls); > + } > + } > + } > +} > + I applied a fix few days ago for hls that refactored the discard flag handling. Instead of adding this new recheck_discard_programs(), could you instead update the playlist_needed() logic to return 0 on appropriate situations in the new mode, and then have recheck_discard_flags() call a function to do the needed initialization/parsing in the "if (cur_needed && !pls->needed)" case? Sorry about the timing. [...] > + {"load_all_variants", "if > 0 all playlists of all variants are > opened at startup", > + OFFSET(load_all_variants), AV_OPT_TYPE_BOOL, {.i64 = 1}, 0, 1, > FLAGS}, The help text is a bit confusing for the user as it is a boolean. Maybe just: "parse all playlists of all variants at startup"
I tried to implement what you requested
v4: fixed the memleak properly
(Re-added ffmpeg-devel@) Rainer Hochecker kirjoitti 2017-12-03 09:16: > 2017-12-02 16:09 GMT+01:00 Anssi Hannula <anssi.hannula@iki.fi>: >> Hi, >> >> Sorry about the delay. >> >> >> Rainer Hochecker kirjoitti 2017-11-28 00:23: >>> >>> 2017-11-27 22:53 GMT+01:00 Anssi Hannula <anssi.hannula@iki.fi>: >>>> >>>> Hi, >>>> >>>> Rainer Hochecker kirjoitti 2017-11-26 12:46: >>>>> >>>>> >>>>> fixed mem leak poined out by Steven >>>>> >>>>> --- >>>>> doc/demuxers.texi | 5 + >>>>> libavformat/hls.c | 304 >>>>> ++++++++++++++++++++++++++++++++++++------------------ >>>>> 2 files changed, 209 insertions(+), 100 deletions(-) >>>>> >>>> [...] >>>>> >>>>> >>>>> + >>>>> +@item load_all_variants >>>>> +If 0, only the first variant/playlist is loaded on open. All other >>>>> variants >>>>> +get disabled and can be enabled by setting discard option in >>>>> program. >>>>> +Default value is 1. >>>>> @end table >>>> >>>> >>>> >>>> If playlist download+parsing is indeed taking long enough time that >>>> this >>>> is >>>> warranted, I wonder if we should maybe just never read any variant >>>> playlists >>>> in read_header(), and instead set AVFMTCTX_NOHEADER. >>>> Then the user may discard AVPrograms right after open, before >>>> unneeded >>>> playlists are loaded+parsed. >>>> >>>> This would avoid having a separate option/mode for this. >>>> >>>> Any thoughts? >>> >>> >>> I actually tried it like this but somwhow did not like it because >>> this >>> is different >>> to all other demuxers/formats. In general you open an input, call >>> find_stream_info, >>> and select a program. I did not like selecting the program between >>> open >>> and >>> find_stream_info. >> >> >> OK I guess. Though arguably hls is already quite different from mpegts >> which >> is the only other demuxer that creates multiple AVPrograms. >> >> In the long run, I think I'd prefer the HLS demuxer to have a mode >> where >> only a single program/variant is given to the user at a time, with the >> demuxer handling the switching between the variants. But that would be >> a lot >> more work and I'm not even sure it would be feasible. So I guess your >> solution is OK, at least for now. >> >> >> Is there a reason the first variant/playlist is still parsed, i.e. why >> not >> simply not parse any media playlists (i.e. only master pls) when the >> new >> mode is selected? > > I thought this could become the new default after a while. Clients that > don't > select a program would still work. Selecting the first available > program, if > none was selected seems the most natural way. Selecting all or none > make > less sense. Hmm.. I wonder if selecting the highest-bandwidth-one might make more sense as the playlist/program order is arbitrary (IIRC) so currently an arbitrary playlist is parsed. >> If the player is selecting the variant/program based on bitrate (like >> Kodi >> does), then the first playlist would likely have been >> downloaded+parsed >> unnecessarily. >> > > Right, but this is acceptable. The majority of user don't have set > network > bandwidth anyway. If the user does not have bandwidth set, Kodi selects the highest-bandwidth program => so both the first playlist and the highest-bandwidth playlist get parsed. >> >> Also, even with your current patch you will need to set >> AVFMTCTX_NOHEADER as >> you defer avformat_new_stream() calls to read_packet(). I think you >> can >> update update_noheader_flag() to set flag_needed=1 if any pls is >> !pls->is_parsed. > > Actually I call open again after having set AVFMTCTX_NOHEADER. > Could you please be more verbose on how you would like to have it? With your patch, with load_all_variants=0, this happens, if I follow the code right: 1. hls_read_header() - A single playlist is opened - in this case the subdemuxer has no AVFTMCTX_NOHEADER so AVFMTCTX_NOHEADER is not set on the main context. - Other playlists are not parsed. ... 2. user undiscards a program. 3. hls_read_packet() - Discard flag checking notices a new playlist is needed and it is parsed. - update_streams_from_subdemuxer() gets called from init_playlist(), and it calls avformat_new_stream() The read_packet documentation says: > 'avformat_new_stream' can be called only if the flag > * AVFMTCTX_NOHEADER is used and only in the calling thread (not in > a > * background thread). And you have not set AVFMTCTX_NOHEADER. So probably update_noheader_flag() needs this change: - if (pls->has_noheader_flag) { + if (pls->has_noheader_flag || !pls->is_parsed) { So that AVFMTCTX_NOHEADER gets always set if there are unparsed playlists left. That might cause e.g. avformat_find_stream_info (if it is called - looks like Kodi always does for HLS - which might be another thing to check to improve launch performance) to probe longer as it tries to find the missing streams in vain, though, so better check the performance is still OK afterwards... >> >> >>>> >>>> >>>>> @section image2 >>>>> diff --git a/libavformat/hls.c b/libavformat/hls.c >>>>> index 786934af03..c42e0b0f95 100644 >>>>> --- a/libavformat/hls.c >>>>> +++ b/libavformat/hls.c >>>>> @@ -112,6 +112,7 @@ struct playlist { >>>>> int n_segments; >>>>> struct segment **segments; >>>>> int needed, cur_needed; >>>>> + int parsed; >>>>> int cur_seq_no; >>>>> int64_t cur_seg_offset; >>>>> int64_t last_load_time; >>>>> @@ -206,6 +207,7 @@ typedef struct HLSContext { >>>>> int strict_std_compliance; >>>>> char *allowed_extensions; >>>>> int max_reload; >>>>> + int load_all_variants; >>>>> } HLSContext; >>>> >>>> >>>> [...] >>>>> >>>>> >>>>> @@ -721,6 +726,7 @@ static int parse_playlist(HLSContext *c, const >>>>> char >>>>> *url, >>>>> free_segment_list(pls); >>>>> pls->finished = 0; >>>>> pls->type = PLS_TYPE_UNSPECIFIED; >>>>> + pls->parsed = 1; >>>>> } >>>> >>>> >>>> >>>> That "pls->parsed = 1" is in the "reinit old playlist for re-parse" >>>> block, >>>> is that intentional? >>>> >>>> I'd think it should rather be at the end of the function alongside >>>> setting >>>> pls->last_load_time, so it is set to 1 whenever parsing has been >>>> done. >>>> >>> >>> I put it at the beginning because I wanted to avoid that it gets >>> tried >>> again >>> on error. >> >> >> OK. But now it is not set for master playlists, so maybe move >> pls->parsed = >> 1 below the "fail" label then? > > Does this solve the issue with the master pls? For master playlist > parse_playlist > is called with NULL for argument playlist. I think if (pls) pls->is_parsed = 1; should work. A master playlist has no struct playlist, but if the URL we starts with is a media playlist then a pls is allocated during parse_playlist(). [...] >>>>> void *u = (s->flags & AVFMT_FLAG_CUSTOM_IO) ? NULL : s->pb; >>>>> @@ -1663,6 +1805,9 @@ static int hls_read_header(AVFormatContext >>>>> *s) >>>>> if ((ret = parse_playlist(c, s->filename, NULL, s->pb)) < 0) >>>>> goto fail; >>>>> >>>>> + /* first playlist was created, set it to parsed */ >>>>> + c->variants[0]->playlists[0]->parsed = 1; >>>>> + >>>> >>>> >>>> >>>> I think parse_playlist() should set this when it parses something. >>> >>> >>> That was pitfall for my v1. The first playlist gets parsed with no >>> playlist given as argument. >>> fate failed because non-master playlists were not set to parsed. >> >> >> I don't think I follow. If parse_playlist() would set the playlist as >> parsed >> on every call (whether failed or not), then all playlists would be set >> to >> parsed and this line would be unnecessary. > > if ((ret = parse_playlist(c, s->filename, NULL, s->pb)) < 0) > goto fail; > > How can parse_playlist set anything on pls if called with NULL for ps? If parse_playlist sees a media playlist, it allocates a pls, so it can set it to is_parsed as long as the set is at the end of the function. >> >>>> >>>>> if ((ret = save_avio_options(s)) < 0) >>>>> goto fail; >>>>> >>>>> @@ -1675,8 +1820,15 @@ static int hls_read_header(AVFormatContext >>>>> *s) >>>>> goto fail; >>>> >>>> >>>> [...] >>>>> >>>>> >>>>> >>>>> /* Select the starting segments */ >>>>> for (i = 0; i < c->n_playlists; i++) { >>>>> struct playlist *pls = c->playlists[i]; >>>>> >>>>> - if (pls->n_segments == 0) >>>>> + if (pls->n_segments == 0 && !pls->parsed) >>>>> continue; >>>> >>>> >>>> >>>> Why? >>> >>> >>> playlists not parsed are invalid, right? maybe n_segments is 0 for >>> not parsed playlists but this was no obvious to me. >> >> >> Yes, but after your change any parsed zero-segment playlists will get >> select_cur_seq_no() called while they didn't before. > > Just drop this? Should be OK to drop, I think. [...]
2017-12-09 20:17 GMT+01:00 Anssi Hannula <anssi.hannula@iki.fi>: >> Selecting the first available program, if none was selected >> seems the most natural way. Selecting all or none make >> less sense. > > Hmm.. I wonder if selecting the highest-bandwidth-one might > make more sense This is what FFmpeg tries in some other cases. Carl Eugen
diff --git a/doc/demuxers.texi b/doc/demuxers.texi index 73dc0feec1..634b122e10 100644 --- a/doc/demuxers.texi +++ b/doc/demuxers.texi @@ -316,6 +316,11 @@ segment index to start live streams at (negative values are from the end). @item max_reload Maximum number of times a insufficient list is attempted to be reloaded. Default value is 1000. + +@item load_all_variants +If 0, only the first variant/playlist is loaded on open. All other variants +get disabled and can be enabled by setting discard option in program. +Default value is 1. @end table @section image2 diff --git a/libavformat/hls.c b/libavformat/hls.c index 786934af03..c42e0b0f95 100644 --- a/libavformat/hls.c +++ b/libavformat/hls.c @@ -112,6 +112,7 @@ struct playlist { int n_segments; struct segment **segments; int needed, cur_needed; + int parsed; int cur_seq_no; int64_t cur_seg_offset; int64_t last_load_time; @@ -206,6 +207,7 @@ typedef struct HLSContext { int strict_std_compliance; char *allowed_extensions; int max_reload; + int load_all_variants; } HLSContext; static int read_chomp_line(AVIOContext *s, char *buf, int maxlen) @@ -314,6 +316,9 @@ static struct playlist *new_playlist(HLSContext *c, const char *url, pls->is_id3_timestamped = -1; pls->id3_mpegts_timestamp = AV_NOPTS_VALUE; + pls->index = c->n_playlists; + pls->parsed = 0; + pls->needed = 0; dynarray_add(&c->playlists, &c->n_playlists, pls); return pls; } @@ -721,6 +726,7 @@ static int parse_playlist(HLSContext *c, const char *url, free_segment_list(pls); pls->finished = 0; pls->type = PLS_TYPE_UNSPECIFIED; + pls->parsed = 1; } while (!avio_feof(in)) { read_chomp_line(in, line, sizeof(line)); @@ -1377,23 +1383,41 @@ reload: static void add_renditions_to_variant(HLSContext *c, struct variant *var, enum AVMediaType type, const char *group_id) { - int i; + int i, j; + int found; for (i = 0; i < c->n_renditions; i++) { struct rendition *rend = c->renditions[i]; if (rend->type == type && !strcmp(rend->group_id, group_id)) { - if (rend->playlist) + if (rend->playlist) { /* rendition is an external playlist * => add the playlist to the variant */ - dynarray_add(&var->playlists, &var->n_playlists, rend->playlist); - else + found = 0; + for (j = 0; j < var->n_playlists; j++) { + if (var->playlists[j] == rend->playlist) { + found = 1; + break; + } + } + if (!found) + dynarray_add(&var->playlists, &var->n_playlists, rend->playlist); + } else { /* rendition is part of the variant main Media Playlist * => add the rendition to the main Media Playlist */ - dynarray_add(&var->playlists[0]->renditions, - &var->playlists[0]->n_renditions, - rend); + found = 0; + for (j = 0; j < var->playlists[0]->n_renditions; j++) { + if (var->playlists[0]->renditions[j] == rend) { + found = 1; + break; + } + } + if (!found) + dynarray_add(&var->playlists[0]->renditions, + &var->playlists[0]->n_renditions, + rend); + } } } } @@ -1631,6 +1655,124 @@ static int hls_close(AVFormatContext *s) return 0; } +static int init_playlist(HLSContext *c, struct playlist *pls) +{ + AVInputFormat *in_fmt = NULL; + int highest_cur_seq_no = 0; + int ret; + int i; + + if (!(pls->ctx = avformat_alloc_context())) { + return AVERROR(ENOMEM); + } + + if (pls->n_segments == 0) + return 0; + + pls->needed = 1; + pls->parent = c->ctx; + + /* + * If this is a live stream and this playlist looks like it is one segment + * behind, try to sync it up so that every substream starts at the same + * time position (so e.g. avformat_find_stream_info() will see packets from + * all active streams within the first few seconds). This is not very generic, + * though, as the sequence numbers are technically independent. + */ + highest_cur_seq_no = 0; + for (i = 0; i < c->n_playlists; i++) { + struct playlist *pls = c->playlists[i]; + if (!pls->parsed) + continue; + if (pls->cur_seq_no > highest_cur_seq_no) + highest_cur_seq_no = pls->cur_seq_no; + } + if (!pls->finished && pls->cur_seq_no == highest_cur_seq_no - 1 && + highest_cur_seq_no < pls->start_seq_no + pls->n_segments) { + pls->cur_seq_no = highest_cur_seq_no; + } + + pls->read_buffer = av_malloc(INITIAL_BUFFER_SIZE); + if (!pls->read_buffer){ + ret = AVERROR(ENOMEM); + avformat_free_context(pls->ctx); + pls->ctx = NULL; + return ret; + } + ffio_init_context(&pls->pb, pls->read_buffer, INITIAL_BUFFER_SIZE, 0, pls, + read_data, NULL, NULL); + pls->pb.seekable = 0; + ret = av_probe_input_buffer(&pls->pb, &in_fmt, pls->segments[0]->url, + NULL, 0, 0); + if (ret < 0) { + /* Free the ctx - it isn't initialized properly at this point, + * so avformat_close_input shouldn't be called. If + * avformat_open_input fails below, it frees and zeros the + * context, so it doesn't need any special treatment like this. */ + av_log(c->ctx, AV_LOG_ERROR, "Error when loading first segment '%s'\n", pls->segments[0]->url); + av_free(pls->read_buffer); + pls->read_buffer = NULL; + avformat_free_context(pls->ctx); + pls->ctx = NULL; + return ret; + } + pls->ctx->pb = &pls->pb; + pls->ctx->io_open = nested_io_open; + pls->ctx->flags |= c->ctx->flags & ~AVFMT_FLAG_CUSTOM_IO; + + if ((ret = ff_copy_whiteblacklists(pls->ctx, c->ctx)) < 0) + return ret; + + ret = avformat_open_input(&pls->ctx, pls->segments[0]->url, in_fmt, NULL); + if (ret < 0) { + av_log(c->ctx, AV_LOG_ERROR, "Error opening playlist %s", pls->segments[0]->url); + avformat_free_context(pls->ctx); + pls->ctx = NULL; + return ret; + } + + if (pls->id3_deferred_extra && pls->ctx->nb_streams == 1) { + ff_id3v2_parse_apic(pls->ctx, &pls->id3_deferred_extra); + avformat_queue_attached_pictures(pls->ctx); + ff_id3v2_free_extra_meta(&pls->id3_deferred_extra); + pls->id3_deferred_extra = NULL; + } + + if (pls->is_id3_timestamped == -1) + av_log(c->ctx, AV_LOG_WARNING, "No expected HTTP requests have been made\n"); + + /* + * For ID3 timestamped raw audio streams we need to detect the packet + * durations to calculate timestamps in fill_timing_for_id3_timestamped_stream(), + * but for other streams we can rely on our user calling avformat_find_stream_info() + * on us if they want to. + */ + if (pls->is_id3_timestamped) { + ret = avformat_find_stream_info(pls->ctx, NULL); + if (ret < 0) { + avformat_free_context(pls->ctx); + pls->ctx = NULL; + return ret; + } + } + + pls->has_noheader_flag = !!(pls->ctx->ctx_flags & AVFMTCTX_NOHEADER); + + /* Create new AVStreams for each stream in this playlist */ + ret = update_streams_from_subdemuxer(c->ctx, pls); + if (ret < 0) { + avformat_free_context(pls->ctx); + pls->ctx = NULL; + return ret; + } + + add_metadata_from_renditions(c->ctx, pls, AVMEDIA_TYPE_AUDIO); + add_metadata_from_renditions(c->ctx, pls, AVMEDIA_TYPE_VIDEO); + add_metadata_from_renditions(c->ctx, pls, AVMEDIA_TYPE_SUBTITLE); + + return 0; +} + static int hls_read_header(AVFormatContext *s) { void *u = (s->flags & AVFMT_FLAG_CUSTOM_IO) ? NULL : s->pb; @@ -1663,6 +1805,9 @@ static int hls_read_header(AVFormatContext *s) if ((ret = parse_playlist(c, s->filename, NULL, s->pb)) < 0) goto fail; + /* first playlist was created, set it to parsed */ + c->variants[0]->playlists[0]->parsed = 1; + if ((ret = save_avio_options(s)) < 0) goto fail; @@ -1675,8 +1820,15 @@ static int hls_read_header(AVFormatContext *s) goto fail; } /* If the playlist only contained playlists (Master Playlist), - * parse each individual playlist. */ - if (c->n_playlists > 1 || c->playlists[0]->n_segments == 0) { + * parse all individual playlists. + * If option load_all_variants is false, load only first variant */ + if (!c->load_all_variants && c->n_variants > 1) { + for (i = 0; i < c->variants[0]->n_playlists; i++) { + struct playlist *pls = c->variants[0]->playlists[i]; + if ((ret = parse_playlist(c, pls->url, pls, NULL)) < 0) + goto fail; + } + } else if (c->n_playlists > 1 || c->playlists[0]->n_segments == 0) { for (i = 0; i < c->n_playlists; i++) { struct playlist *pls = c->playlists[i]; if ((ret = parse_playlist(c, pls->url, pls, NULL)) < 0) @@ -1720,13 +1872,17 @@ static int hls_read_header(AVFormatContext *s) if (!program) goto fail; av_dict_set_int(&program->metadata, "variant_bitrate", v->bandwidth, 0); + + /* start with the first variant and disable all others */ + if (i > 0 && !c->load_all_variants) + program->discard = AVDISCARD_ALL; } /* Select the starting segments */ for (i = 0; i < c->n_playlists; i++) { struct playlist *pls = c->playlists[i]; - if (pls->n_segments == 0) + if (pls->n_segments == 0 && !pls->parsed) continue; pls->cur_seq_no = select_cur_seq_no(c, pls); @@ -1736,97 +1892,9 @@ static int hls_read_header(AVFormatContext *s) /* Open the demuxer for each playlist */ for (i = 0; i < c->n_playlists; i++) { struct playlist *pls = c->playlists[i]; - AVInputFormat *in_fmt = NULL; - - if (!(pls->ctx = avformat_alloc_context())) { - ret = AVERROR(ENOMEM); - goto fail; - } - - if (pls->n_segments == 0) - continue; - pls->index = i; - pls->needed = 1; - pls->parent = s; - - /* - * If this is a live stream and this playlist looks like it is one segment - * behind, try to sync it up so that every substream starts at the same - * time position (so e.g. avformat_find_stream_info() will see packets from - * all active streams within the first few seconds). This is not very generic, - * though, as the sequence numbers are technically independent. - */ - if (!pls->finished && pls->cur_seq_no == highest_cur_seq_no - 1 && - highest_cur_seq_no < pls->start_seq_no + pls->n_segments) { - pls->cur_seq_no = highest_cur_seq_no; - } - - pls->read_buffer = av_malloc(INITIAL_BUFFER_SIZE); - if (!pls->read_buffer){ - ret = AVERROR(ENOMEM); - avformat_free_context(pls->ctx); - pls->ctx = NULL; - goto fail; - } - ffio_init_context(&pls->pb, pls->read_buffer, INITIAL_BUFFER_SIZE, 0, pls, - read_data, NULL, NULL); - pls->pb.seekable = 0; - ret = av_probe_input_buffer(&pls->pb, &in_fmt, pls->segments[0]->url, - NULL, 0, 0); - if (ret < 0) { - /* Free the ctx - it isn't initialized properly at this point, - * so avformat_close_input shouldn't be called. If - * avformat_open_input fails below, it frees and zeros the - * context, so it doesn't need any special treatment like this. */ - av_log(s, AV_LOG_ERROR, "Error when loading first segment '%s'\n", pls->segments[0]->url); - avformat_free_context(pls->ctx); - pls->ctx = NULL; - goto fail; - } - pls->ctx->pb = &pls->pb; - pls->ctx->io_open = nested_io_open; - pls->ctx->flags |= s->flags & ~AVFMT_FLAG_CUSTOM_IO; - - if ((ret = ff_copy_whiteblacklists(pls->ctx, s)) < 0) - goto fail; - - ret = avformat_open_input(&pls->ctx, pls->segments[0]->url, in_fmt, NULL); - if (ret < 0) - goto fail; - - if (pls->id3_deferred_extra && pls->ctx->nb_streams == 1) { - ff_id3v2_parse_apic(pls->ctx, &pls->id3_deferred_extra); - avformat_queue_attached_pictures(pls->ctx); - ff_id3v2_free_extra_meta(&pls->id3_deferred_extra); - pls->id3_deferred_extra = NULL; - } - - if (pls->is_id3_timestamped == -1) - av_log(s, AV_LOG_WARNING, "No expected HTTP requests have been made\n"); - - /* - * For ID3 timestamped raw audio streams we need to detect the packet - * durations to calculate timestamps in fill_timing_for_id3_timestamped_stream(), - * but for other streams we can rely on our user calling avformat_find_stream_info() - * on us if they want to. - */ - if (pls->is_id3_timestamped) { - ret = avformat_find_stream_info(pls->ctx, NULL); - if (ret < 0) - goto fail; - } - - pls->has_noheader_flag = !!(pls->ctx->ctx_flags & AVFMTCTX_NOHEADER); - - /* Create new AVStreams for each stream in this playlist */ - ret = update_streams_from_subdemuxer(s, pls); - if (ret < 0) - goto fail; - - 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); + if (pls->parsed) + init_playlist(c, pls); } update_noheader_flag(s); @@ -1877,6 +1945,36 @@ static int recheck_discard_flags(AVFormatContext *s, int first) return changed; } +static void recheck_discard_programs(AVFormatContext *s) +{ + HLSContext *c = s->priv_data; + int i, j; + + for (i = 0; i < c->n_variants; i++) { + struct variant *var = c->variants[i]; + AVProgram *program = s->programs[i]; + + if (program->discard >= AVDISCARD_ALL) + continue; + + for (j = 0; j < c->variants[i]->n_playlists; j++) { + struct playlist *pls = c->variants[i]->playlists[j]; + + if (!pls->parsed) { + if (parse_playlist(c, pls->url, pls, NULL) < 0) + continue; + if (var->audio_group[0]) + add_renditions_to_variant(c, var, AVMEDIA_TYPE_AUDIO, var->audio_group); + if (var->video_group[0]) + add_renditions_to_variant(c, var, AVMEDIA_TYPE_VIDEO, var->video_group); + if (var->subtitles_group[0]) + add_renditions_to_variant(c, var, AVMEDIA_TYPE_SUBTITLE, var->subtitles_group); + init_playlist(c, pls); + } + } + } +} + static void fill_timing_for_id3_timestamped_stream(struct playlist *pls) { if (pls->id3_offset >= 0) { @@ -1924,6 +2022,8 @@ static int hls_read_packet(AVFormatContext *s, AVPacket *pkt) HLSContext *c = s->priv_data; int ret, i, minplaylist = -1; + recheck_discard_programs(s); + recheck_discard_flags(s, c->first_packet); c->first_packet = 0; @@ -2101,6 +2201,8 @@ static int hls_read_seek(AVFormatContext *s, int stream_index, for (i = 0; i < c->n_playlists; i++) { /* Reset reading */ struct playlist *pls = c->playlists[i]; + if (!pls->parsed) + continue; if (pls->input) ff_format_io_close(pls->parent, &pls->input); av_packet_unref(&pls->pkt); @@ -2157,6 +2259,8 @@ static const AVOption hls_options[] = { INT_MIN, INT_MAX, FLAGS}, {"max_reload", "Maximum number of times a insufficient list is attempted to be reloaded", OFFSET(max_reload), AV_OPT_TYPE_INT, {.i64 = 1000}, 0, INT_MAX, FLAGS}, + {"load_all_variants", "if > 0 all playlists of all variants are opened at startup", + OFFSET(load_all_variants), AV_OPT_TYPE_BOOL, {.i64 = 1}, 0, 1, FLAGS}, {NULL} };