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.

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.

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.

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);
      }
      else
      {
        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.

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.

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.