Hi,
On 02/08/17 14:18, Michael Niedermayer wrote:
> 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.cowg...@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
The initial value of A is unused in the old code, but not in the new code.
>>
>> 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
I confess I only know the basics of how these functions work. On the
basis that yuv2gbrp_full_X_c looks like it copies yuv2rgb_X_c_template,
and I would have thought the rgb and gbr cases should be similar, I
copied a number of things from yuv2rgba64_full_X_c_template into this
function. That value and all of the modifications inside the for loop
come from there.
>>
>> 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()
This is where the clipping code in yuv2rgba64_full_X_c_template is, and
in that function, the value of A is not clipped - only the value stored
in dest.
Thanks,
James
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
