[FFmpeg-devel] [PATCH] libswscale/swscale_unscaled: fix DITHER_COPY macro
Michael Niedermayer
michael at niedermayer.cc
Wed Sep 6 00:37:48 EEST 2017
On Tue, Sep 05, 2017 at 04:42:06PM +0200, Mateusz wrote:
> W dniu 2017-09-05 o 15:40, Michael Niedermayer pisze:
> > On Mon, Sep 04, 2017 at 09:33:34AM +0200, Mateusz wrote:
> >> If ffmpeg reduces bit-depth to 8-bit or more, it uses DITHER_COPY macro.
> >> The problem is DITHER_COPY macro make images darker (on all planes).
> >>
> >> In x265 project there is issue #365 which is caused by this DITHER_COPY bug.
> >> I think it is time to fix -- there are more and more high bit-depth sources.
> >>
> >> Please review.
> >>
> >> libswscale/swscale_unscaled.c | 44 +++++++++++++------------------------------
> >> 1 file changed, 13 insertions(+), 31 deletions(-)
> >
> > please provide a git compatible patch with with commit message as produced
> > by git format-patch or git send-email
> >
> > this mail is not accepted as is by git
> > Applying: libswscale/swscale_unscaled: fix DITHER_COPY macro
> > error: corrupt patch at line 6
> > error: could not build fake ancestor
> > Patch failed at 0001 libswscale/swscale_unscaled: fix DITHER_COPY macro
> >
> > [...]
>
> I've attached the result of git format-patch command.
>
> Sorry for 1 private e-mail (I clicked wrong button).
>
> Mateusz
> swscale_unscaled.c | 44 +++++++++++++-------------------------------
> 1 file changed, 13 insertions(+), 31 deletions(-)
> 9973b13b3f74319abe9c97302ee87b2b3468b3b6 0001-fix-DITHER_COPY-macro-to-avoid-make-images-darker.patch
> From 9b96d612fef46ccec7e148cd7f8e8666b4e7a4cd Mon Sep 17 00:00:00 2001
> From: Mateusz <mateuszb at poczta.onet.pl>
> Date: Tue, 5 Sep 2017 16:09:28 +0200
> Subject: [PATCH] fix DITHER_COPY macro to avoid make images darker
>
> ---
> libswscale/swscale_unscaled.c | 44 +++++++++++++------------------------------
> 1 file changed, 13 insertions(+), 31 deletions(-)
>
> diff --git a/libswscale/swscale_unscaled.c b/libswscale/swscale_unscaled.c
> index ef36aec..3b7a5a9 100644
> --- a/libswscale/swscale_unscaled.c
> +++ b/libswscale/swscale_unscaled.c
> @@ -110,24 +110,6 @@ DECLARE_ALIGNED(8, static const uint8_t, dithers)[8][8][8]={
> { 112, 16,104, 8,118, 22,110, 14,},
> }};
>
> -static const uint16_t dither_scale[15][16]={
> -{ 2, 3, 3, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5,},
> -{ 2, 3, 7, 7, 13, 13, 25, 25, 25, 25, 25, 25, 25, 25, 25, 25,},
> -{ 3, 3, 4, 15, 15, 29, 57, 57, 57, 113, 113, 113, 113, 113, 113, 113,},
> -{ 3, 4, 4, 5, 31, 31, 61, 121, 241, 241, 241, 241, 481, 481, 481, 481,},
> -{ 3, 4, 5, 5, 6, 63, 63, 125, 249, 497, 993, 993, 993, 993, 993, 1985,},
> -{ 3, 5, 6, 6, 6, 7, 127, 127, 253, 505, 1009, 2017, 4033, 4033, 4033, 4033,},
> -{ 3, 5, 6, 7, 7, 7, 8, 255, 255, 509, 1017, 2033, 4065, 8129,16257,16257,},
> -{ 3, 5, 6, 8, 8, 8, 8, 9, 511, 511, 1021, 2041, 4081, 8161,16321,32641,},
> -{ 3, 5, 7, 8, 9, 9, 9, 9, 10, 1023, 1023, 2045, 4089, 8177,16353,32705,},
> -{ 3, 5, 7, 8, 10, 10, 10, 10, 10, 11, 2047, 2047, 4093, 8185,16369,32737,},
> -{ 3, 5, 7, 8, 10, 11, 11, 11, 11, 11, 12, 4095, 4095, 8189,16377,32753,},
> -{ 3, 5, 7, 9, 10, 12, 12, 12, 12, 12, 12, 13, 8191, 8191,16381,32761,},
> -{ 3, 5, 7, 9, 10, 12, 13, 13, 13, 13, 13, 13, 14,16383,16383,32765,},
> -{ 3, 5, 7, 9, 10, 12, 14, 14, 14, 14, 14, 14, 14, 15,32767,32767,},
> -{ 3, 5, 7, 9, 11, 12, 14, 15, 15, 15, 15, 15, 15, 15, 16,65535,},
> -};
> -
>
> static void fillPlane(uint8_t *plane, int stride, int width, int height, int y,
> uint8_t val)
> @@ -1502,22 +1484,22 @@ static int packedCopyWrapper(SwsContext *c, const uint8_t *src[],
> }
>
> #define DITHER_COPY(dst, dstStride, src, srcStride, bswap, dbswap)\
> - uint16_t scale= dither_scale[dst_depth-1][src_depth-1];\
> - int shift= src_depth-dst_depth + dither_scale[src_depth-2][dst_depth-1];\
> + unsigned shift= src_depth-dst_depth, tmp;\
> for (i = 0; i < height; i++) {\
> - const uint8_t *dither= dithers[src_depth-9][i&7];\
> + const uint8_t *dither= dithers[shift-1][i&7];\
> for (j = 0; j < length-7; j+=8){\
> - dst[j+0] = dbswap((bswap(src[j+0]) + dither[0])*scale>>shift);\
> - dst[j+1] = dbswap((bswap(src[j+1]) + dither[1])*scale>>shift);\
> - dst[j+2] = dbswap((bswap(src[j+2]) + dither[2])*scale>>shift);\
> - dst[j+3] = dbswap((bswap(src[j+3]) + dither[3])*scale>>shift);\
> - dst[j+4] = dbswap((bswap(src[j+4]) + dither[4])*scale>>shift);\
> - dst[j+5] = dbswap((bswap(src[j+5]) + dither[5])*scale>>shift);\
> - dst[j+6] = dbswap((bswap(src[j+6]) + dither[6])*scale>>shift);\
> - dst[j+7] = dbswap((bswap(src[j+7]) + dither[7])*scale>>shift);\
> + tmp = (bswap(src[j+0]) + dither[0])>>shift; dst[j+0] = dbswap(tmp - (tmp>>dst_depth));\
> + tmp = (bswap(src[j+1]) + dither[1])>>shift; dst[j+1] = dbswap(tmp - (tmp>>dst_depth));\
> + tmp = (bswap(src[j+2]) + dither[2])>>shift; dst[j+2] = dbswap(tmp - (tmp>>dst_depth));\
> + tmp = (bswap(src[j+3]) + dither[3])>>shift; dst[j+3] = dbswap(tmp - (tmp>>dst_depth));\
> + tmp = (bswap(src[j+4]) + dither[4])>>shift; dst[j+4] = dbswap(tmp - (tmp>>dst_depth));\
> + tmp = (bswap(src[j+5]) + dither[5])>>shift; dst[j+5] = dbswap(tmp - (tmp>>dst_depth));\
> + tmp = (bswap(src[j+6]) + dither[6])>>shift; dst[j+6] = dbswap(tmp - (tmp>>dst_depth));\
> + tmp = (bswap(src[j+7]) + dither[7])>>shift; dst[j+7] = dbswap(tmp - (tmp>>dst_depth));\
This does not look correct
flat black should be mapped to flat black (ok)
flat white should be mapped to flat white (ok)
black+1 should not be mapped to flat black but to a dithered pattern mixing black with the next brighter color (ok)
white-1 should not be mapped to flat white but to a dithered pattern mixing white with the next darker color (not ok)
example if you take 9 bit to 7 bit
511 + {0..3} = {511..514}
{511..514} >> 2 = {127,128}
{127,128} - ({127,128} >> 7) = 127
510 + {0..3} = {510..513}
{510..513} >> 2 = {127,128}
{127,128} - ({127,128} >> 7) = 127
Thus multiple of the brigher colors all get mapped to the same output,
that way the brightest shades become indistingishable and this is not
correct.
above is based on code review not testing so i might have missed
something
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Awnsering whenever a program halts or runs forever is
On a turing machine, in general impossible (turings halting problem).
On any real computer, always possible as a real computer has a finite number
of states N, and will either halt in less than N cycles or never halt.
-------------- 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/20170905/890a876a/attachment.sig>
More information about the ffmpeg-devel
mailing list