On 2013-01-27 4:38 PM, Ronald S. Bultje wrote:
>> -        dst[i] = ((RY * r + GY * g + BY * b + (33 << (RGB2YUV_SHIFT + bpc - 
>> 9))) >> RGB2YUV_SHIFT);
>> +        dst[i] = ((RY * r + GY * g + BY * b + (33 << (RGB2YUV_SHIFT + bpc - 
>> 9))) >> (RGB2YUV_SHIFT + bpc - 14));
> 
> This looks wrong. The original code also btw. We're doing
> (cast_to_fixed_point)(r*ry + g*gy + b*by + 16 + 0.5). I don't think
> this does the correct thing now.

I'll look into it.

>> -        dstU[i] = (RU * r + GU * g + BU * b + (257 << (RGB2YUV_SHIFT + bpc 
>> - 9))) >> RGB2YUV_SHIFT;
>> -        dstV[i] = (RV * r + GV * g + BV * b + (257 << (RGB2YUV_SHIFT + bpc 
>> - 9))) >> RGB2YUV_SHIFT;
>> +        dstU[i] = (RU * r + GU * g + BU * b + (257 << (RGB2YUV_SHIFT + bpc 
>> - 9))) >> (RGB2YUV_SHIFT + bpc - 14);
>> +        dstV[i] = (RV * r + GV * g + BV * b + (257 << (RGB2YUV_SHIFT + bpc 
>> - 9))) >> (RGB2YUV_SHIFT + bpc - 14);
> 
> Same. The round and shift should correspond to each other in both
> cases, and now they don't.

Ditto.

>>  #define isAnyRGB(x)                    \
>>      (isRGBinInt(x)              ||     \
>> -     isBGRinInt(x))
>> +     isBGRinInt(x)              ||     \
>> +     (x)==AV_PIX_FMT_GBRP9LE    ||     \
>> +     (x)==AV_PIX_FMT_GBRP9BE    ||     \
>> +     (x)==AV_PIX_FMT_GBRP10LE   ||     \
>> +     (x)==AV_PIX_FMT_GBRP10BE   ||     \
>> +     (x)==AV_PIX_FMT_GBRP16LE   ||     \
>> +     (x)==AV_PIX_FMT_GBRP16BE   ||     \
>> +     (x)==AV_PIX_FMT_GBRP              \
>> +    )
> 
> || isPlanarRGB(x)?

Will do.

- Derek

_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to