[FFmpeg-devel] [RFC]lavc/ffv1dec: Scale msb-packed output to full 16bit

[Ronald S. Bultje]

Hi,

On Thu, Nov 17, 2016 at 1:06 PM, Kieran Kunhya <kierank at obe.tv> wrote:

> On Thu, 17 Nov 2016 at 18:05 Kieran Kunhya <kierank at obe.tv> wrote:
>
> > On Wed, 16 Nov 2016 at 23:49 Kieran Kunhya <kierank at obe.tv> wrote:
> >
> > It was lossless before and is lossless after the patch but it
> > output the 16bit format that you don't like and of which you
> > claim it breaks with find_best_pix_fmt() - I still believe this
> > should be trivial to fix.
> >
> > Currently white is gray, the patch tries to fix this.
> >
> >
> > erm...in what way does it fix that?
> > A white/black value in 16-235 needs to remain like that when shifted.
> This
> > isn't RGB-land.
> >
> >
> > This incorrect patch has been committed in spite of clear objections:
> >
> >
> > https://git.videolan.org/?p=ffmpeg.git;a=commitdiff;h=
> 55a424c5a836523828b3b2f02b7db610e898b3fc
> >
> > Regards,
> > Kieran Kunhya
> >
>
> Furthermore I will be reverting this patch in two hours.


There's no need to wait two hours, the patch is fundamentally wrong.

Carl, the reason the patch is wrong is that luma range does not scale up
from 16<<n to (236<<n)-1, but from 16<<n to (236-1)<<n, where n is bpc-8.
This is documented in numerous places, see e.g.
https://msdn.microsoft.com/en-us/library/windows/desktop/bb970578(v=vs.85).aspx
:

"For example, if the white point of an 8-bit format is 235, the
corresponding 10-bit format has a white point at 940 (235 × 4)."

Your patch is therefore theoretically wrong. The correct behaviour in
limited-range upscaling is indeed to leave the lower bits empty. For
full-range, the lower bits might be filled if the intention is for the
pixel value to be a representation of what the 16bit result would look
like, but whether this belongs in a decoder or not is up for discussion.

Ronald