| Message ID | tencent_CC8C262F5097A6EA27D52AA3531D1C57CB06@qq.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,01/10] avformat/hls: fix repeated requests for media init section | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On Tue, Apr 12, 2022 at 4:15 PM Zhao Zhili <quinkblack@foxmail.com> wrote: > > --- > libavformat/hls.c | 59 ++++++++++++++++++++++++++++++++--------------- > 1 file changed, 40 insertions(+), 19 deletions(-) > > diff --git a/libavformat/hls.c b/libavformat/hls.c > index 83ff4cc607..67c9650e0b 100644 > --- a/libavformat/hls.c > +++ b/libavformat/hls.c > @@ -407,48 +407,65 @@ struct init_section_info { > char byterange[32]; > }; > > -static struct segment *new_init_section(struct playlist *pls, > +/* > + * Return a new init section if info is new, otherwise return an entry > + * from pls->init_sections. > + */ > +static struct segment *get_init_section(struct playlist *pls, > struct init_section_info *info, > - const char *url_base) > + const char *url_base, > + int *new_init) > { > - struct segment *sec; > + struct segment *sec_ptr = NULL; > + struct segment sec = {}; > char tmp_str[MAX_URL_SIZE], *ptr = tmp_str; > > + *new_init = 1; > if (!info->uri[0]) > return NULL; > > - sec = av_mallocz(sizeof(*sec)); > - if (!sec) > - return NULL; > - > if (!av_strncasecmp(info->uri, "data:", 5)) { > ptr = info->uri; > } else { > ff_make_absolute_url(tmp_str, sizeof(tmp_str), url_base, info->uri); > if (!tmp_str[0]) { > - av_free(sec); > return NULL; > } > } > - sec->url = av_strdup(ptr); > - if (!sec->url) { > - av_free(sec); > - return NULL; > - } > + // Don't dup until make sure it's a new initialization section > + sec.url = ptr; > > if (info->byterange[0]) { > - sec->size = strtoll(info->byterange, NULL, 10); > + sec.size = strtoll(info->byterange, NULL, 10); > ptr = strchr(info->byterange, '@'); > if (ptr) > - sec->url_offset = strtoll(ptr+1, NULL, 10); > + sec.url_offset = strtoll(ptr+1, NULL, 10); > } else { > /* the entire file is the init section */ > - sec->size = -1; > + sec.size = -1; > } > > - dynarray_add(&pls->init_sections, &pls->n_init_sections, sec); > + for (int i = 0; i < pls->n_init_sections; i++) { > + sec_ptr = pls->init_sections[i]; > + if (!strcmp(sec_ptr->url, sec.url) && > + sec_ptr->size == sec.size && > + sec_ptr->url_offset == sec.url_offset) { > + *new_init = 0; > + return sec_ptr; > + } > + } > + > + sec_ptr = av_malloc(sizeof(*sec_ptr)); > + if (!sec_ptr) return NULL; > + *sec_ptr = sec; > + sec_ptr->url = av_strdup(ptr); > + if (!sec_ptr->url) { > + av_free(sec_ptr); > + return NULL; > + } > + dynarray_add(&pls->init_sections, &pls->n_init_sections, sec_ptr); > > - return sec; > + return sec_ptr; > } > > static void handle_init_section_args(struct init_section_info *info, const char *key, > @@ -851,13 +868,17 @@ static int parse_playlist(HLSContext *c, const char *url, > else if (!strcmp(ptr, "VOD")) > pls->type = PLS_TYPE_VOD; > } else if (av_strstart(line, "#EXT-X-MAP:", &ptr)) { > + int new_init = 1; > struct init_section_info info = {{0}}; > ret = ensure_playlist(c, &pls, url); > if (ret < 0) > goto fail; > ff_parse_key_value(ptr, (ff_parse_key_val_cb) handle_init_section_args, > &info); > - cur_init_section = new_init_section(pls, &info, url); > + cur_init_section = get_init_section(pls, &info, url, &new_init); > + /* Skip if it's the same init section */ > + if (!new_init) > + continue; > if (!cur_init_section) { > ret = AVERROR(ENOMEM); > goto fail; > -- > 2.31.1 Can these patches handle multiple instances of the media init sections?
> On Apr 12, 2022, at 7:45 PM, mypopy@gmail.com wrote: > > On Tue, Apr 12, 2022 at 4:15 PM Zhao Zhili <quinkblack@foxmail.com> wrote: >> >> --- >> libavformat/hls.c | 59 ++++++++++++++++++++++++++++++++--------------- >> 1 file changed, 40 insertions(+), 19 deletions(-) >> > Can these patches handle multiple instances of the media init sections? Before and after the patch, struct playlist do take multiple media initialization sections into consideration. However, I think it doesn’t work for HLS as a whole. To support multiple media init sections, it needs to handle EXT-X-DISCONTINUITY and recreate mp4 demuxer. So even if new media init section is show up, it will be downloaded and then dropped by mp4 muxer: ``` if (c->found_moov) { av_log(c->fc, AV_LOG_WARNING, "Found duplicated MOOV Atom. Skipped it\n"); avio_skip(pb, atom.size); return 0; } ``` What the patch does is to avoid downloading the same init sections again and again, and then dropped by mp4 muxer. It’s common to have init section repeat in live mode. Just for reference, hls.js had the same issue, and have been fixed by https://github.com/video-dev/hls.js/pull/4452 > _______________________________________________ > 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".
> On Apr 12, 2022, at 9:26 PM, zhilizhao(赵志立) <quinkblack@foxmail.com> wrote: > > > >> On Apr 12, 2022, at 7:45 PM, mypopy@gmail.com wrote: >> >> On Tue, Apr 12, 2022 at 4:15 PM Zhao Zhili <quinkblack@foxmail.com> wrote: >>> >>> --- >>> libavformat/hls.c | 59 ++++++++++++++++++++++++++++++++--------------- >>> 1 file changed, 40 insertions(+), 19 deletions(-) >>> >> Can these patches handle multiple instances of the media init sections? > > Before and after the patch, struct playlist do take multiple media > initialization sections into consideration. However, I think it doesn’t > work for HLS as a whole. To support multiple media init sections, it > needs to handle EXT-X-DISCONTINUITY and recreate mp4 demuxer. > > So even if new media init section is show up, it will be downloaded and > then dropped by mp4 muxer: I mean mp4 demuxer. > ``` > if (c->found_moov) { > av_log(c->fc, AV_LOG_WARNING, "Found duplicated MOOV Atom. Skipped it\n"); > avio_skip(pb, atom.size); > return 0; > } > ``` > > What the patch does is to avoid downloading the same init sections again > and again, and then dropped by mp4 muxer. It’s common to have init section > repeat in live mode. Ditto. > > Just for reference, hls.js had the same issue, and have been fixed by > > https://github.com/video-dev/hls.js/pull/4452 > >> _______________________________________________ >> 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". > > _______________________________________________ > 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".
diff --git a/libavformat/hls.c b/libavformat/hls.c index 83ff4cc607..67c9650e0b 100644 --- a/libavformat/hls.c +++ b/libavformat/hls.c @@ -407,48 +407,65 @@ struct init_section_info { char byterange[32]; }; -static struct segment *new_init_section(struct playlist *pls, +/* + * Return a new init section if info is new, otherwise return an entry + * from pls->init_sections. + */ +static struct segment *get_init_section(struct playlist *pls, struct init_section_info *info, - const char *url_base) + const char *url_base, + int *new_init) { - struct segment *sec; + struct segment *sec_ptr = NULL; + struct segment sec = {}; char tmp_str[MAX_URL_SIZE], *ptr = tmp_str; + *new_init = 1; if (!info->uri[0]) return NULL; - sec = av_mallocz(sizeof(*sec)); - if (!sec) - return NULL; - if (!av_strncasecmp(info->uri, "data:", 5)) { ptr = info->uri; } else { ff_make_absolute_url(tmp_str, sizeof(tmp_str), url_base, info->uri); if (!tmp_str[0]) { - av_free(sec); return NULL; } } - sec->url = av_strdup(ptr); - if (!sec->url) { - av_free(sec); - return NULL; - } + // Don't dup until make sure it's a new initialization section + sec.url = ptr; if (info->byterange[0]) { - sec->size = strtoll(info->byterange, NULL, 10); + sec.size = strtoll(info->byterange, NULL, 10); ptr = strchr(info->byterange, '@'); if (ptr) - sec->url_offset = strtoll(ptr+1, NULL, 10); + sec.url_offset = strtoll(ptr+1, NULL, 10); } else { /* the entire file is the init section */ - sec->size = -1; + sec.size = -1; } - dynarray_add(&pls->init_sections, &pls->n_init_sections, sec); + for (int i = 0; i < pls->n_init_sections; i++) { + sec_ptr = pls->init_sections[i]; + if (!strcmp(sec_ptr->url, sec.url) && + sec_ptr->size == sec.size && + sec_ptr->url_offset == sec.url_offset) { + *new_init = 0; + return sec_ptr; + } + } + + sec_ptr = av_malloc(sizeof(*sec_ptr)); + if (!sec_ptr) return NULL; + *sec_ptr = sec; + sec_ptr->url = av_strdup(ptr); + if (!sec_ptr->url) { + av_free(sec_ptr); + return NULL; + } + dynarray_add(&pls->init_sections, &pls->n_init_sections, sec_ptr); - return sec; + return sec_ptr; } static void handle_init_section_args(struct init_section_info *info, const char *key, @@ -851,13 +868,17 @@ static int parse_playlist(HLSContext *c, const char *url, else if (!strcmp(ptr, "VOD")) pls->type = PLS_TYPE_VOD; } else if (av_strstart(line, "#EXT-X-MAP:", &ptr)) { + int new_init = 1; struct init_section_info info = {{0}}; ret = ensure_playlist(c, &pls, url); if (ret < 0) goto fail; ff_parse_key_value(ptr, (ff_parse_key_val_cb) handle_init_section_args, &info); - cur_init_section = new_init_section(pls, &info, url); + cur_init_section = get_init_section(pls, &info, url, &new_init); + /* Skip if it's the same init section */ + if (!new_init) + continue; if (!cur_init_section) { ret = AVERROR(ENOMEM); goto fail;