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

[Michael Niedermayer]

On Thu, Feb 02, 2017 at 08:14:00AM -0800, Philip Langdale wrote:
> -----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.

my comment wasnt intended to imply an oppinion on the patch.
I just wanted to correct the pointer to the code deciding the pix fmt

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No great genius has ever existed without some touch of madness. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170202/4d5f71a7/attachment.sig>