Opened 11 years ago

Closed 6 years ago

#1046 closed defect (fixed)

Possible errors in semantics for scaling_list data

Reported by: peterderivaz Owned by:
Priority: minor Milestone: HM-10.1
Component: Text Version: D10 (L1003) v33
Keywords: Cc: bbross, wjhan, jct-vc@…

Description

Range for loop variable

In 7.4.5 "Scaling list data semantics" the definition of "scaling_list_pred_matrix_id_delta" involves setting values for i in the range 0..Min(64,...), but the Table 7-6 is only defined for values up to 63.

I believe the Min(64, should be changed to Min(63,

Note that this appears twice in the definition of "scaling_list_pred_matrix_id_delta".

Default for DC value

When scaling_list_pred_mode_flag==0, the values for ScalingList are derived based on previous values of ScalingList.

I believe there should be a similar derivation for the values of scaling_list_dc_coef_minus8 based on previous values.

At the moment, ScalingFactor[3][matrixId][0][0] is based on scaling_list_dc_coef_minus8[1][matrixId] which will take its default value of 8 if scaling_list_pred_matrix_flag[3][matrixId]==0.

The reference code appears to have code to deal with this case in TDecCavlc::parseScalingList

if( sizeId > SCALING_LIST_8x8 )
{
   scalingList->setScalingListDC(sizeId,listId,((listId == scalingList->getRefMatrixId (sizeId,listId))? 16 :scalingList->getScalingListDC(sizeId, scalingList->getRefMatrixId (sizeId,listId))));
}

Attachments (1)

JCTVC-L1003_v34_#1046_fix.zip (96.8 KB) - added by o.nakagami 11 years ago.

Download all attachments as: .zip

Change History (6)

comment:1 Changed 11 years ago by DefaultCC Plugin

  • Cc bbross wjhan jct-vc@… added

comment:2 Changed 11 years ago by peterderivaz

Thinking about the range for the loop variable, there is also a problem with the second part of the Min.

It currently reads i = 0..Min( 64, ( 1 << ( 4 + ( sizeId << 1 ) ) ) ).

I think this should become i = 0..(Min( 64, ( 1 << ( 4 + ( sizeId << 1 ) ) ) )-1).

(and the same change in equation 7-31)

Changed 11 years ago by o.nakagami

comment:3 Changed 11 years ago by o.nakagami

Thank you for the comment. I think you're right.

Attached is a suggested fix. Please check it.

comment:4 Changed 11 years ago by peterderivaz

Thanks Nakagami, the content of your change looks correct to me.

Stylistically, it feels slightly odd to be setting the syntax element scaling_list_dc_coef_minus8 to a value in the semantics for a different element.
However, I think the meaning is perfectly clear and I do not have a better suggestion, so thanks again!

comment:5 Changed 6 years ago by bbross

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

Fixed in the 2016/12 release of the HEVC specification.

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

  • Benjamin Bross(Subscriber, Participant)
  • jct-vc@…(Subscriber)
  • karl.sharman@…(Always)
  • Karsten Suehring(Always)
  • Ohji Nakagami(Participant)
  • Peter de Rivaz(Reporter, Participant)
  • Woo-Jin Han(Subscriber)