[FFmpeg-devel] [PATCH] avformat/utils: always preserve container dimensions for all streams

[Hendrik Leppkes]

On Wed, Jan 27, 2021 at 2:56 PM Anton Khirnov <anton at khirnov.net> wrote:
>
> Quoting James Almer (2021-01-26 00:17:51)
> > On 1/25/2021 7:46 PM, Michael Niedermayer wrote:
> > > On Sun, Jan 24, 2021 at 11:41:13AM -0300, James Almer wrote:
> > >> If a decoder is used for probing it may change the dimensions reported by the
> > >> demuxer, either by the lowres factor or because of assorted frames reporting
> > >> different dimensions, and in a codec copy scenario, the last dimensions
> > >> arbitrarily set by it could end up being propagated to the muxer.
> > >>
> > >> Signed-off-by: James Almer <jamrial at gmail.com>
> > >> ---
> > >>   libavformat/utils.c                     | 10 ++++++----
> > >>   tests/ref/fate/cbs-vp9-vp90-2-05-resize |  2 +-
> > >>   tests/ref/fate/redcode-demux            |  2 +-
> > >>   tests/ref/fate/wtv-demux                |  4 ++--
> > >>   4 files changed, 10 insertions(+), 8 deletions(-)
> > >>
> > >
> > > breaks:
> > >
> > > ./ffmpeg -i tickets/2892/MPEG_tbn_test.mov  -c:v copy -c:a copy -vtag mx3n -timecode 10:00:00:00 -vframes 3 file.mov
> > > ...
> > > [mov @ 0x558785108e00] D-10/IMX must use 720x608 or 720x512 video resolution
> >
> > So the source mov is faulty and reports a wrong resolution? And trying
> > to pass it through instead of letting a decoder change it makes it fail
> > to mux because the muxer refuses to create non compliant files.
> > That means if there's no decoder in the build, you'd get the same result
> > as without this patch.
> >
> > The mere existence of this code here means that letting decoders set the
> > resolution in a codec copy scenario is not ideal, as shown by the fact
> > one can tell it to make up an arbitrary resolution like it's the case of
> > lowres, or it can just pick up one from an arbitrary frame. Hence
> > prioritizing what the demuxer reads from the container feels like the
> > correct thing to do. But of course, broken files are a thing.
> >
> > I don't know, maybe a new AVFMT_FLAG_ flag to choose between giving
> > priority to what the container reports or what a decoder sets, if any is
> > present, could work around this?
>
> I'd prefer an ffmpeg.c flag if anything.
>
> It is not lavf's job to set policy on what information is more
> trustworthy.
> The demuxer should export what is written in the container, the
> decoder/parser should export what is written in the codec. The user then
> decides which gets used.
>

I concur with that approach. User code (including ffmpeg.c) should set
a preference if required, avformat should expose container information
if present, otherwise information is lost and not recoverable.

- Hendrik