[FFmpeg-devel] [PATCH] swscale: fix gbrap16 alpha channel issues

[Michael Niedermayer]

On Tue, Aug 01, 2017 at 02:46:22PM +0100, James Cowgill wrote:
> Fixes filter-pixfmts-scale test failing on big-endian systems due to
> alpSrc not being cast to (const int32_t**).
> 
> Also fixes distortions in the output alpha channel values by copying the
> alpha channel code from the rgba64 case found elsewhere in output.c.
> 
> Fixes ticket 6555.
> 
> Signed-off-by: James Cowgill <James.Cowgill at imgtec.com>
> ---
>  libswscale/output.c                 | 15 ++++++++-------
>  tests/ref/fate/filter-pixfmts-scale |  4 ++--
>  2 files changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/libswscale/output.c b/libswscale/output.c
> index 9774e9f327..8e5ec0a256 100644
> --- a/libswscale/output.c
> +++ b/libswscale/output.c
> @@ -2026,17 +2026,18 @@ yuv2gbrp16_full_X_c(SwsContext *c, const int16_t *lumFilter,
>                      const int16_t **lumSrcx, int lumFilterSize,
>                      const int16_t *chrFilter, const int16_t **chrUSrcx,
>                      const int16_t **chrVSrcx, int chrFilterSize,
> -                    const int16_t **alpSrc, uint8_t **dest,
> +                    const int16_t **alpSrcx, uint8_t **dest,
>                      int dstW, int y)
>  {
>      const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(c->dstFormat);
>      int i;
> -    int hasAlpha = (desc->flags & AV_PIX_FMT_FLAG_ALPHA) && alpSrc;
> +    int hasAlpha = (desc->flags & AV_PIX_FMT_FLAG_ALPHA) && alpSrcx;
>      uint16_t **dest16 = (uint16_t**)dest;
>      const int32_t **lumSrc  = (const int32_t**)lumSrcx;
>      const int32_t **chrUSrc = (const int32_t**)chrUSrcx;
>      const int32_t **chrVSrc = (const int32_t**)chrVSrcx;
> -    int A = 0; // init to silence warning
> +    const int32_t **alpSrc  = (const int32_t**)alpSrcx;

> +    int A = 0xFFFF << 14;

unused value


>  
>      for (i = 0; i < dstW; i++) {
>          int j;
> @@ -2059,13 +2060,13 @@ yuv2gbrp16_full_X_c(SwsContext *c, const int16_t *lumFilter,
>          V >>= 14;
>  
>          if (hasAlpha) {
> -            A = 1 << 18;
> +            A = -0x40000000;

where does this value come from ?
it looks copy and pasted from luma, but alpha does not have a black
level offset as its not luminance


>  
>              for (j = 0; j < lumFilterSize; j++)
>                  A += alpSrc[j][i] * lumFilter[j];
>  
> -            if (A & 0xF8000000)
> -                A =  av_clip_uintp2(A, 27);
> +            A >>= 1;
> +            A += 0x20002000;
>          }
>  
>          Y -= c->yuv2rgb_y_offset;
> @@ -2083,7 +2084,7 @@ yuv2gbrp16_full_X_c(SwsContext *c, const int16_t *lumFilter,
>          dest16[1][i] = B >> 14;
>          dest16[2][i] = R >> 14;
>          if (hasAlpha)
> -            dest16[3][i] = A >> 11;
> +            dest16[3][i] = av_clip_uintp2(A, 30) >> 14;

why do you move the cliping code here, this seems unneeded
outside the removed if()


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

Asymptotically faster algorithms should always be preferred if you have
asymptotical amounts of data
-------------- 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/20170802/bc861e76/attachment.sig>