Opened 8 years ago

Closed 8 years ago

#1060 closed defect (fixed)

Segmentation fault when decoding streams with multiple picture parameter sets

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

Description

If a stream uses a PPS that is not the most recent, then we can get a segmentation fault in the reference decoder.

This applies to both the version tagged 10.0, and the current development branch (as of 2 April 2013).

Program received signal SIGSEGV, Segmentation fault.
__memcpy_ssse3_back () at ../sysdeps/x86_64/multiarch/memcpy-ssse3-back.S:1524
	in ../sysdeps/x86_64/multiarch/memcpy-ssse3-back.S
#1  0x0000000000430135 in TDecSbac::xCopyContextsFrom (this=0xa776c8, pSrc=0xa17e48) at /apollo/pfcd/work/hevc/HM-10.0dev/build/linux/lib/TLibDecoder/../../../../source/Lib/TLibDecoder/TDecSbac.cpp:1532

The reason appears to be that the size of the context memory is set in TDecTop::xDecodePPS via line 604 of TDecTop.cpp to be

Int NumCtx = pps->getEntropyCodingSyncEnabledFlag()?2:1;

This code is activated when a PPS is decoded. The problem occurs if we have a PPS with EntropyCodingSyncEnabledFlag==1, followed by another with EntropyCodingSyncEnabledFlag==0, followed by a CVS that uses the first PPS.

This ends up trying to use m_cSliceDecoder.CTXMem[1] which is now pointing to deallocated memory.

Note that the specification includes the constraint

It is a requirement of bitstream conformance that the value of entropy_coding_sync_enabled_flag shall be the same for all PPSs that are activated within a CVS.

However, this does not appear to prohibit this case as only a single PPS is activated within the CVS.

Perhaps a workaround would be to always set NumCtx equal to 2?

Attachments (2)

wpp_contexts.patch (1.3 KB) - added by ksuehring 8 years ago.
wpp_contexts_r2.patch (1.9 KB) - added by ksuehring 8 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 8 years ago by DefaultCC Plugin

  • Cc fbossen ksuehring davidf jct-vc@… added

comment:2 Changed 8 years ago by ksuehring

I already had that on my ToDo list, but did not find time yet to work on it. Basically, I think the initialization should go to the parameter set activation code. We should also have some conformance checks there to avoid activating an invalid PPS (as described in the bitstream constraints).

Changed 8 years ago by ksuehring

comment:3 Changed 8 years ago by ksuehring

Can you please check, if the attached patch fixes the issue?

comment:4 Changed 8 years ago by ksuehring

  • Owner set to ksuehring
  • Status changed from new to assigned

comment:5 Changed 8 years ago by peterderivaz

I'm afraid that this patch did not solve my problem.

I've been digging in and I think the problem is that my IRAP picture at the start of the CVS does not use dependent slices, but the subsequent P frames do.

This means that the initialization conditional on m_apcSlicePilot->isIRAP()&& pps->getDependentSliceSegmentsEnabledFlag() never gets triggered.

Perhaps this is an invalid stream?
As far as I can tell from the spec, you must have entropy_coding_sync_enabled_flag constant over the CVS, but I have not found any such restriction for dependent slices segments, but perhaps I have missed some important constraint.

comment:6 Changed 8 years ago by ksuehring

My feeling was also that this would have been constrained to the CVS, but I also did not find that constraint in the text.

I'm attaching an update to the patch.

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

Changed 8 years ago by ksuehring

comment:7 Changed 8 years ago by yk

I don't remember it has ever been discussed whether to require the value of dependent_slice_segments_enabled_flag to be the same for all PPSs that are referred to in a CVS, and as a result we don't have such a restriction in the spec.

comment:8 Changed 8 years ago by ksuehring

Peter, could you please test the updated patch?

comment:9 Changed 8 years ago by peterderivaz

Thanks a lot! The updated patch means I can correctly decode my stream.

comment:10 Changed 8 years ago by ksuehring

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

the patch was applied in r3404

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)
  • jct-vc@…(Subscriber)
  • karl.sharman@…(Always)
  • Karsten Suehring(Owner, Subscriber, Participant, Always)
  • Peter de Rivaz(Reporter, Participant)
  • Ye-Kui Wang(Participant)