Opened 4 years ago

Closed 4 years ago

#655 closed defect (fixed)

BDOF multiplication of sGxGy and vx

Reported by: bheng Owned by:
Priority: minor Milestone:
Component: spec Version: VVC D7 vB
Keywords: Cc: ksuehring, bbross, XiangLi, fbossen, jvet@…

Description

Is there a reason sGxGy is split into Hi/Lo bits in eq 8-806 and 8-807?

sGxGym = sGxGy >> 12
sGxGys = sGxGy & ( ( 1 << 12 ) − 1 )

The two halves are immediately put back together after the multiplication with vx below:

( ( vx * sGxGym ) << 12 + vx * sGxGys )

And that is the only place these variables are used.

Isn't this equivalent to just (vx * sGxGy), without needing all these extra steps?

Change history (5)

comment:1 Changed 4 years ago by fbossen

It's not equivalent: + has precedence over <<. This is really broken...

comment:2 Changed 4 years ago by bbross

If this is broken, what would be the way to fix it so it as aligned with VTM?

comment:3 Changed 4 years ago by fbossen

VTM does the following:

      int     tmpData = tmpx * mainsGxGy;
      tmpData = ((tmpData << 12) + tmpx*secsGxGy) >> 1;

It has an extra set of parentheses around the << 12 operation.

Brian's suggestion actually looks good. As far as I can tell it fixes the issue and makes things cleaner :-)

comment:4 Changed 4 years ago by bbross

Thanks for confirming!
Following Brians suggestion, I will replace:
( ((vx*sGxGym)<<12) + vx*sGxGys )
with
(vx * sGxGy)
and remove sGxGym and sGxGys in JVET-P2001-vC.

comment:5 Changed 4 years ago by bbross

  • Resolution set to fixed
  • Status changed from new to closed

Fixed it as stated above for JVET-P2001-vC.

Note: See TracTickets for help on using tickets.