Re: [FFmpeg-devel] [PATCH] lavf/dash: Forward strict flag to component demuxers

[Frank Plowman <post@frankplowman.com>]

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*.

-- 
Frank
_______________________________________________
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".