Opened 10 years ago

Closed 10 years ago

#1368 closed defect (fixed)

HM vs specification mismatch

Reported by: kolya Owned by:
Priority: minor Milestone: HM-16.3
Component: HM Version: HM-16.2
Keywords: Cc: ksuehring, davidf, karlsharman, jct-vc@…

Description

Specification tells that "The value of chroma_qp_offset_list_len_minus1 shall be in the range of 0 to 5, inclusive."

This means that the array dimension here should be changed to 5:

ChromaQpAdj m_ChromaQpAdjTable[7];

Then, it is better to change the code in matching places (see the patch for the decoder; relevant changes are needed for the encoder as well).

Specification does not prohibits the list length of 1 (value == 0), but prohibits values more than 5.

Btw, Main_422_10_A_RExt_Sony_2.bin RExt conformance bitstream seems to be non-compliant coz has chroma_qp_offset_list_len_minus1 == 6.

Attachments (1)

chroma_qp_offset_list_len_minus1.patch​ (1.3 KB) - added by kolya 10 years ago.

Download all attachments as: .zip

Change History (6)

comment:1 Changed 10 years ago by DefaultCC Plugin

  • Cc ksuehring davidf karlsharman jct-vc@… added

Changed 10 years ago by kolya

comment:2 Changed 10 years ago by kolya

Corrections:

ChromaQpAdj m_ChromaQpAdjTable[7]; not to 5 but to 6 coz value 5 is included according to the spec.

The stream is also conformant, this is an erroneous report.

Last edited 10 years ago by kolya (previous) (diff)

comment:3 Changed 10 years ago by ksuehring

This is another instance of not checking the maximum values (see #1367). The array is one entry larger than the allowed number of values, which does not hurt, but obviously still does not prevent us from writing to random memory when decoding invalid bitstreams.

I don't see how this is a "HM vs specification mismatch".

comment:4 Changed 10 years ago by karlsharman

In the HM software, the syntax elements cu_chroma_qp_offset_flag and cu_chroma_qp_offset_idx have been rolled into one variable: offset.

cu_chroma_qp_offset_flag cu_chroma_qp_offset_idx offset
0 X 0
1 0 1
1 1 2
1 2 3
1 3 4
1 4 5
1 5 6

There are 7 offsets, and these correspond to the size of the table that you
are looking at.

I believe the offset method was how this tool was originally developed, and the mapping to syntax elements arrived later.

comment:5 Changed 10 years ago by ksuehring

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

Fixed in r4262

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)
  • jct-vc@…(Subscriber)
  • Karl Sharman(Subscriber, Participant)
  • karl.sharman@…(Always)
  • Karsten Suehring(Subscriber, Participant, Always)
  • Nikolay Shlyakhov(Reporter, Participant)