Opened 7 years ago

Closed 7 years ago

#1097 closed defect (invalid)

Incorrect long term reference POC calculation in reference decoder

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


I think the reference decoder has an incorrect calculation for long term reference picture POCs in the case where delta_poc_msb_present_flag is equal to zero and the current POC's MSB is non-zero. The incorrect code can be found in TDecCAVLC.cpp, lines 1005 and 1006:

            rps->setPOC     (j, pocLsbLt);
            rps->setDeltaPOC(j, - rpcSlice->getPOC() + pocLsbLt);

The spec states that when delta_poc_msb_cycle_lt is not present (i.e. delta_poc_msb_present_flag is equal to zero), it is inferred to be zero. The section of code above the incorrect section (which calculates long term reference POCs for the case when delta_poc_msb_present_flag==1) is correct, and says:

            Int pocLTCurr = rpcSlice->getPOC() - deltaPocMSBCycleLT * maxPicOrderCntLSB 
                                        - iPOClsb + pocLsbLt;
            rps->setPOC     (j, pocLTCurr); 
            rps->setDeltaPOC(j, - rpcSlice->getPOC() + pocLTCurr);

Setting delta_poc_msb_present_flag==0 should have the effect of making deltaPocMSBCycleLT be assumed to be zero, i.e. the correct code at lines 1005 and 1006 would be:

            Int pocLTCurr = rpcSlice->getPOC() 
                                        - iPOClsb + pocLsbLt;
            rps->setPOC     (j, pocLTCurr); 
            rps->setDeltaPOC(j, - rpcSlice->getPOC() + pocLTCurr);

exactly as above, but with deltaPocMSBCycleLT=0. Substituting this for the current code gives the correct behaviour.

Change History (8)

comment:1 Changed 7 years ago by DefaultCC Plugin

  • Cc fbossen ksuehring davidf jct-vc@… added

comment:2 Changed 7 years ago by ksuehring

I think the original code follows equation 8-5 which defines:

pocLt = PocLsbLt[i]
if( delta_poc_msb_present_flag[i] )
  pocLt += PicOrderCntVal − DeltaPocMsbCycleLt[i] * MaxPicOrderCntLsb − slice_pic_order_cnt_lsb

So this looks correct to me.

comment:3 Changed 7 years ago by jackh

Thanks for your reply. You're right - I had misinterpreted the meaning of delta_poc_msb_present_flag==0 - I thought it meant that the MSB was the same as the current picture's, whereas in fact it means 'find any reference picture with this LSB and use it'.

However, I think there's still a problem. setDeltaPOC currently gets called with the difference between the LSB of the reference picture and the full POC of the current picture, which I don't think can be right. In my test stream I have MaxPicOrderCntLsb=512, and I have a picture with POC 824 referencing a picture with POC 751. This is referenced as a long term picture with LSB 751&(MaxPicOrderCntLsb-1) which is 239. The current code sets the deltaPOC as -824+239, which gives -585. This erroneous result causes decoding to fail further down the line in TComSlice.cpp line 1054, which adds 824 and -585 and reasonably claims that it can't find picture 239 in the DPB.

It looks to me as though what needs to happen here is a search of the DPB for the given LSB, and then the deltaPOC should be set appropriately for the found picture. Let me know if I've got the wrong end of the stick again.

comment:4 Changed 7 years ago by adarsh

In the decoder software, the delta POC values of LTRPs are never (or rather should not be) used. In all occurences, first it is checked whether a reference picture is a short-term (STRP) or a long-term reference picture (LTRP). In case that it is an STRP, the delta POC value (the variable to setDeltaPoc()) is used. In case it is an LTRP, the POC value (the variable corresponding to setPOC()) is used; although the delta POC variable is set for LTRPs it is not used (at least that is the intention). You will notice that in all such cases, first it is checked whether the picture is LTRP and only then the POC value is used.

I could not find the code that you're referring to in line 1054 in TComSlice.cpp in HM10.0; did you refer the code in createExplicitReferencePictureSetFromReference()? Since that function was invoked only when pictures are lost, I think it never came up in testing CTC.

Please do see if it is the code/patch corresponding to Ticket #1052 addresses part of your concern. Something similar to what you suggested was necessary when CRA pictures were present (where the actual POC value of the picture had to be found out from the DPB). However, I do agree that your suggestion could be applied to calculate the correct value of delta POC for all the pictures - that would be a cleaner solution.

comment:5 Changed 7 years ago by jackh

Thanks for the clarification. I've now fetched branches/HM-10.0-dev which includes changeset 3392 from Ticket #1052, and tried the test stream again. The stream (same one as detailed above) now causes the decoder to assert in checkCRA():

  for(Int i = pReferencePictureSet->getNumberOfNegativePictures()+pReferencePictureSet->getNumberOfPositivePictures(); i < pReferencePictureSet->getNumberOfPictures(); i++)
    if(pocCRA < MAX_UINT && getPOC() > pocCRA)
      if (!pReferencePictureSet->getCheckLTMSBPresent(i))
        //This is the assert that fires
        assert(xGetLongTermRefPic(rcListPic, pReferencePictureSet->getPOC(i), false)->getPOC() >= pocCRA);
        assert(pReferencePictureSet->getPOC(i) >= pocCRA);

The problem seems to be this. When setPOC() is called in the section of TDecCAVLC.cpp we've been discussing, it passes just the LSB of the referenced picture, not the full POC - in order to do this it would have to do the DPB lookup. xGetLongTermRefPic() goes through the DPB and finds the picture correctly, but when getPOC() is called on that picture it returns the LSB, because that's what was stored originally (239 in my example). This is then compared to the full POC for the previous CRA (751 in my example), and the assert fires.

The solution seems to be to do the lookup in TDecCAVLC, and call setPOC and setDeltaPOC according to the full POC found in the DPB.

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

comment:6 Changed 7 years ago by adarsh

Although, I do agree that using the setPOC() and setDeltaPOC() functions to use the full POC of the LTRPs from the DPB is desirable for a cleaner code/solution, I think the problem that you've mentioned above is due to some other reason. xGetLongTermRefPic(rcListPic, pReferencePictureSet->getPOC(i), false)->getPOC() returns the POC value from the TComPic object, not the TComReferencePictureSet object (RPS object). Only in the RPS object do you set only the LSB. In the TComPic object, the full POC value will be stored (derived using the slice_pic_order_cnt_lsb). If the correct picture LTRP was found, xGetLongTermRefPic(rcListPic, pReferencePictureSet->getPOC(i), false)->getPOC() should return the full POC value. You may want to ensure that a (non-IRAP :) ) trailing picture is not referring to a picture preceding the associated CRA in decoding order - that is when this assert is triggered.

comment:7 Changed 7 years ago by jackh

Apologies for the long delay in replying. I've now gone over this again and found that indeed the bug was in our encoder code. Many thanks for your help, and apologies for the mix-up.

comment:8 Changed 7 years ago by fbossen

  • Resolution set to invalid
  • Status changed from new to closed
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)
  • Frank Bossen(Subscriber, Participant)
  • Jack Haughton(Reporter, Participant)
  • jct-vc@…(Subscriber)
  • karl.sharman@…(Always)
  • Karsten Suehring(Subscriber, Participant, Always)