Opened 12 years ago

Closed 12 years ago

#700 closed defect (fixed)

Derivation process of chroma offset in WP when an input bit depth is beyond 8bit

Reported by: Tanizawa Owned by:
Priority: minor Milestone: HM-9.1
Component: HM Version: HM-8.0
Keywords: Cc: fbossen, ksuehring, davidf, jct-vc@…

Description

When an input bit depth is beyond 8bit, the derivation process of chroma offset in the weighted prediction is incorrect. Since the value of chroma offset is always held in 8bit (see equations (8-224) and (8-225)), the predicted value of chroma offset must be also held in 8bit.
The original source code in the function of TDecCavlc::xParsePredWeightTable( TComSlice* pcSlice ) should be modified simply as follows (same for TEncCavlc):

Original;

Int shift = ((1<<(g_uiBitDepth+g_uiBitIncrement-1)));

Modified;

Int shift = 128;

The modified patch is provided in this ticket.

Attachments (2)

BugFix_ChromaOffset.patch​ (2.4 KB) - added by Tanizawa 12 years ago.
Patch file for chroma offset
BugFix_ChromaOffsetR1.patch​ (2.5 KB) - added by Tanizawa 12 years ago.

Download all attachments as: .zip

Change History (7)

comment:1 Changed 12 years ago by DefaultCC Plugin

  • Cc fbossen ksuehring davidf jct-vc@… added

Changed 12 years ago by Tanizawa

Patch file for chroma offset

comment:2 Changed 12 years ago by ksuehring

I think I cannot follow this problem: Equation 7-52 contains:

bdShift = 1 << ( BitDepthC − 1 )

which looks like the original code. Is the implementation somehow diverging from the draft text?

comment:3 follow-up: Changed 12 years ago by Tanizawa

The derivation process of ChromaOffsetLx in equation (7-52) is described as follows:

ChromaOffsetL0[ i ][ j ] = Clip3( -128, 127, ( delta_chroma_offset_l0[ i ][ j ] -

( ( 128 * ChromaWeightL0[ i ][ j ] ) >> ChromaLog2WeightDenom ) - 128 ) )

Reflecting Ticket#699, the bdShift was modified to "128".
In HM source code, the equation of bdShift is described as follows:

Int shift = ((1<<(g_uiBitDepth+g_uiBitIncrement-1)));

Therefore, the shift value must be modified to "128", too.
So the simple solution to cope with it is "Int shift = 128;".
However, this modification might seem to be not compatible to the draft text.
Then, I think it is better to modify the following:

original:

Int shift = ((1<<(g_uiBitDepth+g_uiBitIncrement-1)));
Int pred = ( shift - ( ( shift*wp[j].iWeight)>>(wp[j].uiLog2WeightDenom) ) );

modified:

Int pred = ( 128 - ( ( 128*wp[j].iWeight)>>(wp[j].uiLog2WeightDenom) ) );

The new patch is provided in this ticket.

Changed 12 years ago by Tanizawa

comment:4 in reply to: ↑ 3 Changed 12 years ago by yanye

I verified that the attached patch file is correct. For >8 bits video, the WP weights should still be represented in 8-bit precision.

Yan

Replying to Tanizawa:

The derivation process of ChromaOffsetLx in equation (7-52) is described as follows:

ChromaOffsetL0[ i ][ j ] = Clip3( -128, 127, ( delta_chroma_offset_l0[ i ][ j ] -

( ( 128 * ChromaWeightL0[ i ][ j ] ) >> ChromaLog2WeightDenom ) - 128 ) )

Reflecting Ticket#699, the bdShift was modified to "128".
In HM source code, the equation of bdShift is described as follows:

Int shift = ((1<<(g_uiBitDepth+g_uiBitIncrement-1)));

Therefore, the shift value must be modified to "128", too.
So the simple solution to cope with it is "Int shift = 128;".
However, this modification might seem to be not compatible to the draft text.
Then, I think it is better to modify the following:

original:

Int shift = ((1<<(g_uiBitDepth+g_uiBitIncrement-1)));
Int pred = ( shift - ( ( shift*wp[j].iWeight)>>(wp[j].uiLog2WeightDenom) ) );

modified:

Int pred = ( 128 - ( ( 128*wp[j].iWeight)>>(wp[j].uiLog2WeightDenom) ) );

The new patch is provided in this ticket.

comment:5 Changed 12 years ago by fbossen

  • Milestone changed from HM-8.1 to HM-9.1
  • Resolution set to fixed
  • Status changed from new to closed

Fixed in r3116

Note: See TracTickets for help on using tickets.

This list contains all users that will be notified about changes made to this ticket.

These roles will be notified: Reporter, Owner, Subscriber, Participant

  • David Flynn(Subscriber)
  • Frank Bossen(Subscriber, Participant)
  • jct-vc@…(Subscriber)
  • karl.sharman@…(Always)
  • Karsten Suehring(Subscriber, Participant, Always)
  • yanye(Participant)