Message ID | 20240520201420.92545-1-post@frankplowman.com |
---|---|
State | New |
Headers | show |
Series | [FFmpeg-devel] lavf/dash: Forward strict flag to component demuxers | expand |
Context | Check | Description |
---|---|---|
yinshiyou/make_loongarch64 | success | Make finished |
yinshiyou/make_fate_loongarch64 | success | Make fate finished |
Frank Plowman: > Before the patch, opening a DASH file containing streams which require > experimental decoders was problematic. No matter where the -strict -2 > was put on the command line, the option was not passed to the demuxer > for that component. This resulted in an error, prompting the user to > add the -strict -2 flag, which is already present. Decoding appeared to > continue correctly however. > > Patch removes the error message by creating an options object for the > demuxer created for the component, which inherits from the parent > demuxer. > > Signed-off-by: Frank Plowman <post@frankplowman.com> > --- > PS: Can anyone think of other options which should be propagated to the > component demuxers? > > libavformat/dashdec.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c > index 555e21bf69..40abb5ebba 100644 > --- a/libavformat/dashdec.c > +++ b/libavformat/dashdec.c > @@ -1911,13 +1911,18 @@ static int reopen_demux_for_component(AVFormatContext *s, struct representation > if (ret < 0) > goto fail; > if (pls->n_fragments) { > + AVDictionary *stream_info_opts = NULL; > + > #if FF_API_R_FRAME_RATE > if (pls->framerate.den) { > for (i = 0; i < pls->ctx->nb_streams; i++) > pls->ctx->streams[i]->r_frame_rate = pls->framerate; > } > #endif > - ret = avformat_find_stream_info(pls->ctx, NULL); > + > + av_dict_set_int(&stream_info_opts, "strict", s->strict_std_compliance, 0); > + > + ret = avformat_find_stream_info(pls->ctx, &stream_info_opts); > if (ret < 0) > goto fail; > } The loop over pls->ctx indicates that pls->ctx->nb_streams can be > 1 before avformat_find_stream_info(). But then using a single AVDictionary is wrong, as avformat_find_stream_info() expects an array of pls->ctx->nb_streams AVDictionary*. Furthermore, the mixing between AVFormatContext and AVCodecContext options here does not seem good (e.g. for ordinary demuxers setting strict_std_compliance does not affect the AVCodecContext's values at all). - Andreas
Thanks for your review Andreas. On 20/05/2024 21:41, Andreas Rheinhardt wrote: > Frank Plowman: >> Before the patch, opening a DASH file containing streams which require >> experimental decoders was problematic. No matter where the -strict -2 >> was put on the command line, the option was not passed to the demuxer >> for that component. This resulted in an error, prompting the user to >> add the -strict -2 flag, which is already present. Decoding appeared to >> continue correctly however. >> >> Patch removes the error message by creating an options object for the >> demuxer created for the component, which inherits from the parent >> demuxer. >> >> Signed-off-by: Frank Plowman <post@frankplowman.com> >> --- >> PS: Can anyone think of other options which should be propagated to the >> component demuxers? >> >> libavformat/dashdec.c | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c >> index 555e21bf69..40abb5ebba 100644 >> --- a/libavformat/dashdec.c >> +++ b/libavformat/dashdec.c >> @@ -1911,13 +1911,18 @@ static int reopen_demux_for_component(AVFormatContext *s, struct representation >> if (ret < 0) >> goto fail; >> if (pls->n_fragments) { >> + AVDictionary *stream_info_opts = NULL; >> + >> #if FF_API_R_FRAME_RATE >> if (pls->framerate.den) { >> for (i = 0; i < pls->ctx->nb_streams; i++) >> pls->ctx->streams[i]->r_frame_rate = pls->framerate; >> } >> #endif >> - ret = avformat_find_stream_info(pls->ctx, NULL); >> + >> + av_dict_set_int(&stream_info_opts, "strict", s->strict_std_compliance, 0); >> + >> + ret = avformat_find_stream_info(pls->ctx, &stream_info_opts); >> if (ret < 0) >> goto fail; >> } > > The loop over pls->ctx indicates that pls->ctx->nb_streams can be > 1 > before avformat_find_stream_info(). But then using a single AVDictionary > is wrong, as avformat_find_stream_info() expects an array of > pls->ctx->nb_streams AVDictionary*. Thanks, v2 sent which addresses this. > > Furthermore, the mixing between AVFormatContext and AVCodecContext > options here does not seem good (e.g. for ordinary demuxers setting > strict_std_compliance does not affect the AVCodecContext's values at all). Can you see an alternative? In the case of fftools, the -strict flag is always set for both the codec_opts and format_opts but yes I see the concern assuming this logic in libav*.
diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c index 555e21bf69..40abb5ebba 100644 --- a/libavformat/dashdec.c +++ b/libavformat/dashdec.c @@ -1911,13 +1911,18 @@ static int reopen_demux_for_component(AVFormatContext *s, struct representation if (ret < 0) goto fail; if (pls->n_fragments) { + AVDictionary *stream_info_opts = NULL; + #if FF_API_R_FRAME_RATE if (pls->framerate.den) { for (i = 0; i < pls->ctx->nb_streams; i++) pls->ctx->streams[i]->r_frame_rate = pls->framerate; } #endif - ret = avformat_find_stream_info(pls->ctx, NULL); + + av_dict_set_int(&stream_info_opts, "strict", s->strict_std_compliance, 0); + + ret = avformat_find_stream_info(pls->ctx, &stream_info_opts); if (ret < 0) goto fail; }
Before the patch, opening a DASH file containing streams which require experimental decoders was problematic. No matter where the -strict -2 was put on the command line, the option was not passed to the demuxer for that component. This resulted in an error, prompting the user to add the -strict -2 flag, which is already present. Decoding appeared to continue correctly however. Patch removes the error message by creating an options object for the demuxer created for the component, which inherits from the parent demuxer. Signed-off-by: Frank Plowman <post@frankplowman.com> --- PS: Can anyone think of other options which should be propagated to the component demuxers? libavformat/dashdec.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)