[x265] [PATCH] fix PSYVALUE shift overflow, Issue #180 [OUTPUT CHANGE on 12bpp]

[chen]

At 2015-09-14 12:29:42,"Steve Borho" <steve at borho.org> wrote:
>On 09/11, Min Chen wrote:
>> # HG changeset patch
>> # User Min Chen <chenm003 at 163.com>
>> # Date 1442002697 18000
>> # Node ID f520fd29f3d71d495e08fe96df917489348c377b
>> # Parent  137854992fc614bdd6c446852528e24ed52c9991
>> fix PSYVALUE shift overflow, Issue #180 [OUTPUT CHANGE on 12bpp]
>> ---
>>  source/common/quant.cpp |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>> 
>> diff -r 137854992fc6 -r f520fd29f3d7 source/common/quant.cpp
>> --- a/source/common/quant.cpp	Fri Sep 11 15:18:14 2015 -0500
>> +++ b/source/common/quant.cpp	Fri Sep 11 15:18:17 2015 -0500
>> @@ -588,7 +588,7 @@
>>  #define UNQUANT(lvl)    (((lvl) * (unquantScale[blkPos] << per) + unquantRound) >> unquantShift)
>>  #define SIGCOST(bits)   ((lambda2 * (bits)) >> 8)
>>  #define RDCOST(d, bits) ((((int64_t)d * d) << scaleBits) + SIGCOST(bits))
>> -#define PSYVALUE(rec)   ((psyScale * (rec)) >> (2 * transformShift + 1))
>> +#define PSYVALUE(rec)   ((psyScale * (rec)) >> X265_MAX(0, (2 * transformShift + 1)))
>
>this prevents a negative value, but it seems like we want really large
>psy values to be clamped to a large positive value instead of 0.


I think we need check its logic, and choice better way.


the psyScale is Q.8 x Q.8 = Q.16, it compare to (X << scaleBits) directly, means shift factor must be (16 - 2T - 1) = (15 - 2T) = scaleBits, it is equal unit
In here, the transformShift is NEGTIVE, the scaleBits will be 15 - 2x(-1) = 17, or call Q.17 format, so we need shift left 1 bit on PSYVALUE.
I test some sequence, the result very nearly, so I just simplest way to limit it to Zero.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/x265-devel/attachments/20150914/4d500f0a/attachment.html>