[FFmpeg-devel] [PATCH v3] avformat/concatdec: add support for setting input options

[Jan Ekström]

On Wed, Feb 10, 2021 at 8:33 PM Nicolas George <george at nsup.org> wrote:
>
> Jan Ekström (12021-02-08):
> > This way protocol or format related options can be set for all
> > of the files opened during concatenation both globally as well
> > as per-file.
> > ---
> >
> > Changes from v2:
> >
> > 1. Added an example, although I had issues figuring out something useful
> >    that is not a hack for something. Ended up doing a disablement of
> >    advanced edit list functionality, since that can sometimes lead to
> >    unwanted behavior.
> >
>
> >    * First idea was to override the input format for a file without an
> >      extension. For that, we have no AVOption it seems.
>
> Indeed, lavf does not accept it as an option. It needs to be added
> separately. Or we can make it an option.
>
> >    * Then came the idea of utilizing the framerate option in the raw
> >      h264 demuxer. That didn't work because apparently if there is a header
> >      in there that probed/parsed frame rate field gets utilized.
>
> You could set the frame rate for a raw video stream.
>

Sure.

> >    * Third idea was either the one I picked, or analyzeduration/probesize
> >      for MPEG-TS. I opted for the mp4 case.
> >
> > 2. Quoted the : in documentation with @code{:}.
> > 3. Fixed a duplicate space in a log message.
> > ---
> >
> >  doc/demuxers.texi       | 24 ++++++++++++++
> >  libavformat/concatdec.c | 69 ++++++++++++++++++++++++++++++++++++++++-
> >  2 files changed, 92 insertions(+), 1 deletion(-)
> >
> > diff --git a/doc/demuxers.texi b/doc/demuxers.texi
> > index 3c15ab9eee..20601f9575 100644
> > --- a/doc/demuxers.texi
> > +++ b/doc/demuxers.texi
> > @@ -149,6 +149,14 @@ Metadata of the packets of the file. The specified metadata will be set for
> >  each file packet. You can specify this directive multiple times to add multiple
> >  metadata entries.
> >
> > + at item @code{input_options @var{key=value:key2=value2}}
>
> > +Input options passed on when reading a specific file, using a @code{:}-separated
> > +list of key=value pairs.
>
> No! Why would you do that? It adds one level of escaping, we already
> have way too much of them.

Because:
1. we have the functionality for it
2. it matches the AVOption.

Please do not take it as if I did not have any logic behind my actions
whatsoever, which is what the exclamation mark looks like.

>
> input option key=value
>
> singular, and can be set as many times as necessary.
>
> > Requires @code{safe} to be non-positive.
>
> Zero would be safer.
>
> > Global options
> > +for all files can be set with the @code{input_options} demuxer option. When using
> > +both options on the list of files as well as globally via the demuxer option,
> > +the global ones get applied first and the file-specific options are then applied
> > +on top of them.
>
> I am not sure it is the right order. Options added on the fly should
> take precedence over options written in a file, in general. But the
> global / specific distinction makes it less clear-cut.
>

You set the AVOption for option(s) to be applied for all files, and if
you want to only set some option for specific files you set them in
the list.

*You* requested the latter capability, I added it.

> Other opinions?
>
> > +
> >  @item @code{stream}
> >  Introduce a stream in the virtual file.
> >  All subsequent stream-related directives apply to the last introduced
> > @@ -204,6 +212,10 @@ expressed in microseconds. The duration metadata is only set if it is known
> >  based on the concat file.
> >  The default is 0.
> >
> > + at item input_options
> > +Input options to be passed on for all opened inputs using
>
> > a :-separated list of
>
> It should say "a dictionary", with a link to where the syntax of
> dictionaries is documented, but whoever added AV_OPT_TYPE_DICT neglected
> to add the corresponding documentation paragraph :(
>

Yes. This just matches the rest of the documentation strings etc for
such things.

Jan