[FFmpeg-devel] [PATCH] libswscale/swscale_unscaled: fix DITHER_COPY macro

[Mateusz Brzostek]

W dniu 2017-03-16 o 19:17, Michael Niedermayer pisze:
> On Wed, Mar 15, 2017 at 10:52:29PM +0100, Mateusz Brzostek wrote:
>> Hello!
>>
>> There are 3 problems with DITHER_COPY macro in libswscale/swscale_unscaled.c:
>> 1) there is overflow in dithering from 12-bit to 10-bit (output value > 1023);
>> 2) for limit range the lower limit is not respected, for example from 10-bit to 8-bit value 64 is converted to 15;
>> 3) for many iteration of downscale/upscale of the same image the 200th iteration is significantly darker.
>>
>> The first bug is due to wrong dithers table (now it is OK only for 8-bit destination), fix is:
>> -        const uint8_t *dither= dithers[src_depth-9][i&7];\
>> +        const uint8_t *dither= dithers[src_depth-dst_depth-1][i&7];\
>>
>> For bugs 2) and 3) it is needed formula that do not make images darker (in attachment). So please review.
> does your code maintain white and black levels ?
> with 4 bits white is 15, with 7 bits white is 127 for example
> white should stay white
> black should stay black
> in both directions
=== short version ===

Yes, white goes to white, black goes to black.

=== long version ===

This DITHER_COPY macro is for case when destination bit-depth >= 8 and
source bit-depth > destination bit-depth and the picture type is the same
(for example yuv444p12 to yuv444p10). It is replacement for down-shift.
For destination bit-depth < 8 it is needed real dither (not simplified like this DITHER_COPY).

The formula before the patch was:
dst = (src + dither) * scale >> shift

where: dst - destination, src - source, dither - constant values from table 8x8,
scale and shift are chosen to fulfill formula (in mathematics/floating point arithmetic):
(max_value_src + max_value_dither) * scale / (2^shift) < max_value_dst + 1
This is due to lack of clip function (for speed reason).

If you consider (scale / (2^shift)) number for example for conversion 10-bit to 8-bit,
we have
(scale / (2^shift)) < 1/4
because (1023 + 3) * 1/4 = 256.5 and 256.5 >= 255 + 1
(with 1/4 there will be overflow: 1023 go to 256 for dither > 0).

So for src > 0 with 2 least significant bits = 0 and dither = 0 we have
dst <= (src >> 2) - 1
which is the darker destination effect (it is unavoidable in this model).

In my patch the number
(scale / (2^shift)) == 1/4
for 10-bit to 8-bit example. This fix the darker destination effect but create possible overflow problem.

In my patch the possible overflow go to sign bit of int32_t variable and is clipped to 0x7FFFFFFF.
In my tests the new DITHER_COPY is a little bit slower than old but is much faster than real dither in x265.

Mateusz