diff mbox series

[FFmpeg-devel,v2] lavf/dash: Forward strict flag to component demuxers

Message ID 20240522090513.55806-1-post@frankplowman.com
State New
Headers show
Series [FFmpeg-devel,v2] lavf/dash: Forward strict flag to component demuxers | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished

Commit Message

Frank Plowman May 22, 2024, 9:02 a.m. UTC
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>
---
Changes since v1: Use nb_streams stream_info_opts, not just one.

 libavformat/dashdec.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

Comments

Andreas Rheinhardt May 22, 2024, 9:12 a.m. UTC | #1
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>
> ---
> Changes since v1: Use nb_streams stream_info_opts, not just one.
> 
>  libavformat/dashdec.c | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
> index 555e21bf69..9c5ef5801a 100644
> --- a/libavformat/dashdec.c
> +++ b/libavformat/dashdec.c
> @@ -1911,13 +1911,28 @@ static int reopen_demux_for_component(AVFormatContext *s, struct representation
>      if (ret < 0)
>          goto fail;
>      if (pls->n_fragments) {
> +        AVDictionary **stream_info_opts;
> +
> +        stream_info_opts = av_calloc(pls->ctx->nb_streams, sizeof(*stream_info_opts));
> +        if (!stream_info_opts)
> +            goto fail;

Presumably you should set ret here.

> +
>  #if FF_API_R_FRAME_RATE
>          if (pls->framerate.den) {
> -            for (i = 0; i < pls->ctx->nb_streams; i++)
> +            for (i = 0; i < pls->ctx->nb_streams; i++) {
>                  pls->ctx->streams[i]->r_frame_rate = pls->framerate;
> +                av_dict_set_int(&stream_info_opts[i], "strict", s->strict_std_compliance, 0);

You are setting this in a loop intended to be removed; and one that is
furthermore only executed under certain conditions. This is surely not
what you intended.

> +            }
> +
>          }
>  #endif
> -        ret = avformat_find_stream_info(pls->ctx, NULL);
> +
> +        ret = avformat_find_stream_info(pls->ctx, stream_info_opts);
> +
> +        for (i = 0; i < pls->ctx->nb_streams; i++)

We prefer loop-based iterators nowadays.

> +            av_dict_free(&stream_info_opts[i]);
> +        av_dict_free(stream_info_opts);

This is the wrong deallocator; stream_info_opts will leak.

> +
>          if (ret < 0)
>              goto fail;
>      }

Apart from that my comment about using
AVFormatContext.stridt_std_compliance still applies.

- Andreas
diff mbox series

Patch

diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
index 555e21bf69..9c5ef5801a 100644
--- a/libavformat/dashdec.c
+++ b/libavformat/dashdec.c
@@ -1911,13 +1911,28 @@  static int reopen_demux_for_component(AVFormatContext *s, struct representation
     if (ret < 0)
         goto fail;
     if (pls->n_fragments) {
+        AVDictionary **stream_info_opts;
+
+        stream_info_opts = av_calloc(pls->ctx->nb_streams, sizeof(*stream_info_opts));
+        if (!stream_info_opts)
+            goto fail;
+
 #if FF_API_R_FRAME_RATE
         if (pls->framerate.den) {
-            for (i = 0; i < pls->ctx->nb_streams; i++)
+            for (i = 0; i < pls->ctx->nb_streams; i++) {
                 pls->ctx->streams[i]->r_frame_rate = pls->framerate;
+                av_dict_set_int(&stream_info_opts[i], "strict", s->strict_std_compliance, 0);
+            }
+
         }
 #endif
-        ret = avformat_find_stream_info(pls->ctx, NULL);
+
+        ret = avformat_find_stream_info(pls->ctx, stream_info_opts);
+
+        for (i = 0; i < pls->ctx->nb_streams; i++)
+            av_dict_free(&stream_info_opts[i]);
+        av_dict_free(stream_info_opts);
+
         if (ret < 0)
             goto fail;
     }