Re: [FFmpeg-devel] [PATCH 25/33] fftools/ffmpeg_filter: stop disregarding user-specified pixel format

[Anton Khirnov <anton@khirnov.net>]

Quoting Michael Niedermayer (2023-07-14 01:11:07)
> On Thu, Jul 13, 2023 at 12:55:45PM +0200, Anton Khirnov wrote:
> > When the user explicitly specifies a pixel format that is not supported
> > by the encoder, ffmpeg CLI will currently use some heuristics to pick
> > another supported format. This is wrong and the correct action here is
> > to fail.
> > 
> > Surprisingly, a number of FATE tests are affected by this and actually
> > use a different pixel format than is specified in the makefiles.
> > ---
> >  doc/ffmpeg.texi                               |  3 +-
> >  fftools/ffmpeg_filter.c                       | 35 +------------------
> >  tests/fate/fits.mak                           |  6 ++--
> >  tests/fate/lavf-video.mak                     |  2 +-
> >  tests/fate/vcodec.mak                         |  4 +--
> >  .../{fitsdec-gbrap16le => fitsdec-gbrap16be}  |  4 +--
> >  .../fate/{fitsdec-gbrp16 => fitsdec-gbrp16be} |  4 +--
> >  tests/ref/lavf/gif                            |  2 +-
> >  8 files changed, 13 insertions(+), 47 deletions(-)
> >  rename tests/ref/fate/{fitsdec-gbrap16le => fitsdec-gbrap16be} (79%)
> >  rename tests/ref/fate/{fitsdec-gbrp16 => fitsdec-gbrp16be} (79%)
> > 
> > diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> > index 6769f8d305..08b11097b7 100644
> > --- a/doc/ffmpeg.texi
> > +++ b/doc/ffmpeg.texi
> > @@ -1014,8 +1014,7 @@ Disable autoscale at your own risk.
> >  @item -pix_fmt[:@var{stream_specifier}] @var{format} (@emph{input/output,per-stream})
> >  Set pixel format. Use @code{-pix_fmts} to show all the supported
> >  pixel formats.
> > -If the selected pixel format can not be selected, ffmpeg will print a
> > -warning and select the best pixel format supported by the encoder.
> > +
> >  If @var{pix_fmt} is prefixed by a @code{+}, ffmpeg will exit with an error
> >  if the requested pixel format can not be selected, and automatic conversions
> >  inside filtergraphs are disabled.
> 
> The commit message makes this sound like a bugfix, while really this is
> removing a documented feature.

It is a bugfix in my eyes. When you explicitly tell a program to perform
a specific action, and the program just decides to do something else,
then that program is broken.

As far as I can tell, this "feature" was added by you in 89f86379797
with no explanation or documentation beyond 'fix regression with png'.
It was later documented in a largely-unrelated commit that added
something else.

I see no argument whatsoever for why we should have such a "smart"
selection for pixel formats, but nothing else. It goes entirely against
the way any other option works.

> It also breaks some scripts (fate is an example as it requires changes)

No, its removal reveals already broken FATE tests, as I see no
indication that those tests intended to use a different format than
indicated on the commandline. It seems far more likely that this
misfeature confused the tests' authors as to what format is actually
used. That is IMO yet another argument for removing this.

> If the removial of that feature is intended, it should be argued somewhere
> that this feature is never usefull.

https://xkcd.com/1172/

> To me as a lazy person it surely feels usefull to be able to ask for
> both "exactly rgb" as well as something close to rgb (like bgr or gbrp)
> without needing to know what each individual codec uses to return R,G,B

1) This code does not give you the ability to specify "something close to rgb".
   You specify a precise pixel format, and this code gives you
   something. That something might be what you asked for, or something
   close to it, or something completely unrelated.
   E.g.
     ffmpeg -i in.mkv -map 0:v -c:v libx264 -pix_fmt pal8 -t 1 out.mkv
   produces yuv444p. How is it close to pal8?
2) If you want syntax for fuzzy format specification, patches are
   welcome. But it should not interfere with specifying an exact format.
3) No other option in ffmpeg CLI works like this. If you specify
   something, you get that or an error.

-- 
Anton Khirnov
_______________________________________________
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".