[FFmpeg-devel] [PATCH 1/2] avutil: Add MSB packed YUV444P10 format

[Philip Langdale]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Thu, 2 Feb 2017 12:08:59 +0100
Michael Niedermayer <michaelni at gmx.at> wrote:

> On Thu, Feb 02, 2017 at 11:48:49AM +0100, Michael Niedermayer wrote:
> > On Thu, Feb 02, 2017 at 11:26:26AM +0100, Timo Rothenpieler wrote:  
> > > >> In the mean time, common 12 bit content (YUV420P12 or P016)
> > > >> will be correctly converted to P010 for nvenc.  
> > > > 
> > > > What does this change have to do with 4:2:0 content?  
> > > 
> > > Apparently swscale decides that any kind of 12 or 16 bit content,
> > > even if 4:2:0, whould be converted to YUV444P16 instead of
> > > downsampling it to P010.  
> > 
> > The code decididing pix fmts is in libavutil
> > av_find_best_pix_fmt_of_2() and code using that (if i guess
> > correctly where the choice is made)  
> 
> maybe i worded this unclear, iam not sure thats the only path pixel
> format decissions are made in but this should be the main one
> 
> If the function makes a mistake instead of teh formats available to
> it being restricted by something then fixing it could be an easy
> solution
> 
> [...]
> 
> 

There's nothing wrong with the pixfmt selection logic. The problem is
that we are pretending nvenc supports a particular format when it does
not.

I will try and provide a more streamlined explanation.

* nvenc supports 8 and 10 bit encoding
* It supports two 10 bit input formats: P010 and (YUV444P10 with MSB
  packing)
* ffmpeg supports P010 and (YUV444P10 with LSB packing)
* Mapping the two YUV444P10 formats to each other results in a mess
  because the bits are interpreted differently on each side. This is no
  surprise.
* It was observed that 10bit data in YUV444P16, which is then passed to
  nvenc saying it is YUV444P10 will produce correct results because the
  bits end up in the right place. This is what the code does today.
* However, that is only true for 10bit content.
* If you try and provide 12bit (or higher) content to nvenc, the logic
  that tries to find the best pixfmt will, rightly, prefer YUV444P16 to
  P010 because it fully preserves the bit depth.
* However, remember that nvenc doesn't support >10bit content. It will
  discard/ignore the extra bits.
* Instead, we would much rather that P010 is selected and swscale used
  to dither from 12bits to 10bits, and then this data goes to nvenc.

To avoid this, you have two options

* Do not map the nvenc YUV444P10 to anything: ffmpeg doesn't support
  this format, so forget about it. This is easy.
* Map YUV444P10 to something that accurately represents what it is,
  which is what my change here does (it's not the only way. It's *a*
  way). Then the pixfmt selection code can do the right thing because
  it has accurate data to work with.

If people don't like the new pixfmt, I'm happy to just remove the
mapping. But it's really not ok to keep what we have today which is
just going to cause incorrect handling of >10bit content.

- --phil
-----BEGIN PGP SIGNATURE-----

iEYEARECAAYFAliTWsgACgkQ+NaxlGp1aC4OmwCdGRqRBtktmrN2y3PiIj7HJVgV
yyQAn1lYX06uakA8ZDE/cZSpe4zmH2VU
=DZ7X
-----END PGP SIGNATURE-----