Opened 4 years ago

Last modified 3 years ago

#1101 new defect

Multiple CVSs and skipped leading pictures crash the reference decoder

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


I have a stream with multiple CVSs in one bitstream, and also with BLA pictures in the stream with associated RASLs. This causes the reference decoder to assert in TComInputBitstream::read(), as it runs over the end of the buffer. The attached patch fixes the two problems which together cause this. An explanation of them is below.

Firstly, the reference decoder doesn't recognise the beginning of a new CVS (i.e. the presence of an EOS, BLA or IDR NAL) and reset m_pocRandomAccess accordingly. The patch fixes this by setting m_pocRandomAccess back to MAX_INT (its initial state) whenever such a condition is encountered, so that isRandomAccessSkipPicture() will pick up the new random access POC when it is called.

Secondly, when skipping frames, the decoder calls copySliceInfo to copy the previous slice segment's information, but as the previous slice segment was skipped, this brings in the previous POC, dropping the decoder out of skipping early. The patch fixes this with an extra flag which signals that the previous slice segment was skipped, so copySliceInfo should not be called. The flag also suppresses any hash SUFFIX_SEI NALs associated with skipped pictures from being processed, as otherwise the hash NALs of skipped frames would be taken as applying to the picture before the skipping began, causing a multiple hash SEI warning.

Change History (13)

comment:1 Changed 4 years ago by DefaultCC Plugin

  • Cc fbossen ksuehring davidf jct-vc@… added

comment:2 Changed 4 years ago by fbossen

Thank you for providing a patch. Unfortunately it appears to produce a segmentation fault on the NUT_A_ericsson_3 conformance stream.
Would you be able to share the bitstream that you used to produce the error that you describe? Thank you.

Changed 4 years ago by jackh

comment:3 Changed 4 years ago by jackh

Thanks for your reply. I'm now working against the HM-10.1-dev branch, and have updated the patch so it applies to the head of that branch. Unfortunately I'm not permitted to post the test stream at the moment. However, I've tested the new patch against the conformance stream you mentioned and it runs fine for me. Could you test it again?

comment:4 Changed 4 years ago by fbossen

Could you update the patch to apply to the head of trunk? (HM-10.1-dev was merged into it and some changes were then reverted)

comment:5 Changed 4 years ago by jackh

New patch attached, tested with my test stream and with NUT_A_ericsson_3.bit.

comment:6 Changed 4 years ago by jackh

Discovered an issue with the last patch as it can prevent some pictures being output. The above patch should be applied on top of the previous one, and fixes the issue.

Edit: updated the additional patch with another fix.

Last edited 4 years ago by jackh (previous) (diff)

comment:7 Changed 4 years ago by jackh

Related to this issue: when a new CVS starts with a CRA NUT, the MSB of its POC is not correctly reset to zero, so if the previous picture had a non-zero MSB, subsequent POCs will be wrong.

comment:8 Changed 4 years ago by ksuehring

I have reviewed the combination of the last two patches:

It seems m_prevSuppressOutput is set, but never used for anything?

Also the patches use tabs instead of spaces for indentation and don't follow the coding style for placing of braces which makes reviewing hard. m_bLastNALSkipped is not a good name (how does one skip a network abstraction layer?)

Last edited 4 years ago by ksuehring (previous) (diff)

comment:9 Changed 3 years ago by ksuehring

  • Milestone HM-10.1 deleted
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, Always)
  • Frank Bossen(Subscriber, Participant)
  • Jack Haughton(Reporter, Participant)
  • jct-vc@…(Subscriber)
  • Karl Sharman(Always)
  • Karsten Suehring(Subscriber, Participant, Always)