Opened 5 years ago

Last modified 4 years ago

#1096 new defect

Reference decoder treats IRAP pictures as trailing pictures, as regards RPS rules

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

Description

I have a stream with a CRA NAL that references a RADL picture from the preceding IRAP. The stream looks like this:

IDR picture (POC=0)
RADL picture (POC=-4)
(other leading and trailing pictures)
CRA picture (POC=25)

The CRA picture references the RADL picture with a POC of -4. Using the reference decoder, this asserts on line 621 of TComSlice.cpp, in this loop in checkCRA():

for(Int i = 0; i < pReferencePictureSet->getNumberOfNegativePictures()+pReferencePictureSet->getNumberOfPositivePictures(); i++)
  {
    if(pocCRA < MAX_UINT && getPOC() > pocCRA)
    {
      assert(getPOC()+pReferencePictureSet->getDeltaPOC(i) >= pocCRA);
    }
  }

This loop is quite correct as regards trailing pictures, but should the same rules apply to CRA pictures? It looks to me from the spec as though CRA pictures should be able to reference pictures that came before the preceding IRAP in output order (although not in decoding order). Spec section 8.3.2:

  • When the current picture is a CRA picture, there shall be no picture included in the RPS that precedes, in decoding order, any preceding IRAP picture in decoding order (when present).

and

  • When the current picture is a trailing picture, there shall be no picture in the RPS that precedes the associated IRAP picture in output order or decoding order.

So it looks like the reference decoder is incorrectly treating IRAP pictures the same as trailing pictures, unless I'm missing something in the spec. This problem could be solved by moving the two for loops at the top of checkCRA() below the if statements currently at the bottom, so that pocCRA would be updated before running the checks.

Change History (8)

comment:1 Changed 5 years ago by DefaultCC Plugin

  • Cc fbossen ksuehring davidf jct-vc@… added

comment:2 Changed 5 years ago by ksuehring

I understand the problem. But I don't understand how you are trying to solve the issue. Maybe you can add a patch file to clarify what change you are proposing?

I think the constraint should be changed to trailing pictures, but we need to add another constraint for CRA to not reference pictures that precede the previous IRAP in decoding order (instead of output order).

comment:3 Changed 5 years ago by jackh

Thanks for the speedy reply. My suggestion would only correctly enforce the output order constraint - as you say, another constraint should be added to enforce the decoding order constraint. My suggestion was that the two halves of the function should be swapped, so that the function now looks like this:

Void TComSlice::checkCRA(TComReferencePictureSet *pReferencePictureSet, Int& pocCRA, Bool& prevRAPisBLA, TComList<TComPic *>& rcListPic)
{
  if ( getNalUnitType() == NAL_UNIT_CODED_SLICE_IDR_W_RADL || getNalUnitType() == NAL_UNIT_CODED_SLICE_IDR_N_LP ) // IDR picture found
  {
    pocCRA = getPOC();
    prevRAPisBLA = false;
  }
  else if ( getNalUnitType() == NAL_UNIT_CODED_SLICE_CRA ) // CRA picture found
  {
    pocCRA = getPOC();
    prevRAPisBLA = false;
  }
  else if ( getNalUnitType() == NAL_UNIT_CODED_SLICE_BLA_W_LP
         || getNalUnitType() == NAL_UNIT_CODED_SLICE_BLA_W_RADL
         || getNalUnitType() == NAL_UNIT_CODED_SLICE_BLA_N_LP ) // BLA picture found
  {
    pocCRA = getPOC();
    prevRAPisBLA = true;
  }
  for(Int i = 0; i < pReferencePictureSet->getNumberOfNegativePictures()+pReferencePictureSet->getNumberOfPositivePictures(); i++)
  {
    if(pocCRA < MAX_UINT && getPOC() > pocCRA)
    {
      assert(getPOC()+pReferencePictureSet->getDeltaPOC(i) >= pocCRA);
    }
  }
  for(Int i = pReferencePictureSet->getNumberOfNegativePictures()+pReferencePictureSet->getNumberOfPositivePictures(); i < pReferencePictureSet->getNumberOfPictures(); i++)
  {
    if(pocCRA < MAX_UINT && getPOC() > pocCRA)
    {
      if (!pReferencePictureSet->getCheckLTMSBPresent(i))
      {
        assert(xGetLongTermRefPic(rcListPic, pReferencePictureSet->getPOC(i), false)->getPOC() >= pocCRA);
      }
      else
      {
        assert(pReferencePictureSet->getPOC(i) >= pocCRA);
      }
    }
  }
}

In this way, when this function is entered with an IRAP NAL, pocCRA gets updated before the checks run, so no checks run as getPOC()>pocCRA is no longer true - which is the correct behaviour as regards output order. I haven't looked into the code sufficiently yet to be able to suggest a solution for the decoding order constraint.

comment:4 Changed 5 years ago by ksuehring

Ah, I missed the comparison. So there would be no checking for CRA or BLA pictures.

comment:5 Changed 5 years ago by adarsh

I think the existing software is correct. Although it is not obvious, but an IRAP picture is also a trailing picture (w.r.t to the preceding IRAP picture); it should not contain RASL/RADL pictures in its RPS that are associated with the preceding IRAP picture. In the example, the CRA with POC 25 should not use RADL picture (POC=-4) (as given in the example) for reference, because it is associated with the previous IRAP picture.

This particular situation occurs in the following case (decoding order): an IRAP pic A, associated RASL/RADL pictures of pic A, [X], another IRAP pic B. If there were trailing pictures (non-IRAP) in position [X], then the constraints on trailing pictures will ensure they don't contain any RASL/RADL associated with pic A.

When the current picture is a trailing picture, there shall be no picture in the RPS that precedes the associated IRAP picture in output order or decoding order.

If there are no (non-IRAP) trailing pictures in position [X], even then RPS of pic B should not contain any RADL pictures associated with pic A (with the understanding the pic B is trailing w.r.t. to pic A).

comment:6 Changed 5 years ago by jackh

I see your point. However, on the very same page of the spec that defines the rules for trailing pictures, it says:

"When the current picture is a CRA picture, there shall be no picture included in the RPS that precedes, in decoding order, any preceding IRAP picture in decoding order (when present)."

This sentence is redundant if CRA pictures are to be considered in the same class as trailing pictures with respect to their preceding IRAPs. This sentence doesn't mention output order at all, which makes me think that the spec deliberately intends to make a distinction here between trailing pictures and CRAs.

I think it would make perfect sense for things to work the way you suggest, but if that's the case, the spec needs changing to reflect it.

comment:7 Changed 4 years ago by jackh

I have just come across another case that would support the spec being changed to match adarsh's comment above. If you have a stream with the following NALs:

BLA_W_LP
RASL_R
CRA_NUT

it is legal by the current spec for the CRA picture to refer to the RASL, as it's not before the BLA in decoding order. However, the RASL can't be decoded, so this breaks the note in the first paragraph of section 8.3.3.1:

...During the decoding process, any RASL pictures associated with an IRAP picture that has NoRaslOutputFlag equal to 1 may be ignored, as these pictures are not specified for output and have no effect on the decoding process of any other pictures that are specified for output.

Should this be added as a text ticket?

comment:8 Changed 4 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

  • Adarsh Krishnan Ramasubramonian(Participant)
  • David Flynn(Subscriber, Always)
  • Frank Bossen(Subscriber)
  • Jack Haughton(Reporter, Participant)
  • jct-vc@…(Subscriber)
  • Karl Sharman(Always)
  • Karsten Suehring(Subscriber, Participant, Always)