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

[chen]

At 2015-09-15 04:09:55,"Steve Borho" <steve at borho.org> wrote:
>On 09/15, chen wrote:
>> 
>> 
>> 
>> 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.
>
>but is the max always required?
it is macro, compiler will optimize out it

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