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

[chen]

At 2015-09-15 03:29:02,"Steve Borho" <steve at borho.org> wrote:
>On 09/14, chen wrote:
>> 
>> 
>> 
>> 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.
>
>I see, it clamping the downshift to 0 so it doesn't become a negative
>shift.
>
>If this is only a problem at high bit depths, can we make the macro
>different for each bit depth so this overhead is avoided for 8bit
>encodes?
>
Since we use C++ template to duplicated rdoQuant, it always constant shift.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/x265-devel/attachments/20150915/e647e9e3/attachment.html>