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

[Steve Borho]

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?

-- 
Steve Borho