the patch has been applied in r3062

Patch for original ticket 840 patch

While this patch correctly identifies the source of a problem for WPP and slices, it also introduces a constraint on slice boundaries for WPP that is not present in the draft text.

If a slice starts in the middle of a row, then the constraint forces the slice to end at the end of the current LCU row. This does not seem to be required by the text.

The intention of the patch is to align the results of the compressSlice() and encodeSlice() functions, which is necessary to fix the bug. But I think a better solution would be to remove the constraint for both cases, rather than enforce the constraint for both.

I have attached a patch that removes the constraints. The patch was derived from modifications to ​http://hevc.hhi.fraunhofer.de/svn/svn_HEVCSoftware/branches/HM-9.0-dev@3071

It is required by the text!

Please check the semantics for entropy_coding_sync_enabled_flag.

In fact if a slice starts in the middle of an LCU row, it must terminate before the end of the current LCU row. The same restriction also applies to slice segments.

Therefore it would be better to keep the constraint in the software.

But I don't think that this is what the existing software constraint does. The existing constraint forces the second slice in an LCU row to finish at the end of the current row. The text says that the slice shall finish in the same row. In other words, it should be possible to have many slices in a single row, not just two.

To fix the original bug (encoding CTUs twice) we can either remove the constraint altogether, or fix the constraint so that it allows valid cases to be encoded. An extra conditional check to see if the slice crosses over to the next row should also fix the problem.

Original code:

if (pcSlice->getPPS()->getNumSubstreams() > 1 && (uiStartCUAddrSlice % (rpcPic->getFrameWidthInCU()*rpcPic->getNumPartInCU()) != 0) ) 
{ 
  uiBoundingCUAddrSlice = uiStartCUAddrSlice - (uiStartCUAddrSlice % (rpcPic->getFrameWidthInCU()*rpcPic->getNumPartInCU())) + (rpcPic->getFrameWidthInCU()*rpcPic->getNumPartInCU()); 
}

With an extra condition to check if the slice boundary is in the next row:

if (pcSlice->getPPS()->getNumSubstreams() > 1 && (uiStartCUAddrSlice % (rpcPic->getFrameWidthInCU()*rpcPic->getNumPartInCU()) != 0)
    && ( uiStartCUAddrSlice/rpcPic->getFrameWidthInCU() != uiBoundingCUAddrSlice/rpcPic->getFrameWidthInCU() ) ) 
{ 
  uiBoundingCUAddrSlice = uiStartCUAddrSlice - (uiStartCUAddrSlice % (rpcPic->getFrameWidthInCU()*rpcPic->getNumPartInCU())) + (rpcPic->getFrameWidthInCU()*rpcPic->getNumPartInCU()); 
}

It is allowed to have many slices in a single row. Steve's fix with extra conditional check to see if the slice crosses over to the next row to terminate a slice should fix the problem and make the software and text consistent.

I wouldn't recommend removing the constraint altogether since that would allow the encoder to create illegal streams that violate the restrictions in the text. However, the extra condition suggested above wouldn't work correctly either since uiBoundingCUAddrSlice and getFrameWidthInCU() aren't on the same scale (missing factor of getNumPartInCU()).

But I agree that the current code is enforcing a condition stronger than the one specified in the text.

I have attached an additional patch that just adds a MIN to prevent the encoder from changing the uiBoundingCUAddrSlice unless it is actually necessary. I believe that would fix the issue.

Replying to bheng:

I have attached an additional patch that just adds a MIN to prevent the encoder from changing the uiBoundingCUAddrSlice unless it is actually necessary. I believe that would fix the issue.

This looks like a good fix. It would be useful to include this fix in HM-9.1, as it correctly prevents the encoder from generating a non-conforming WPP bitstream.

I have applied slice_restriction.patch in r3085. The same restrictions apply for dependent slices (or dependent slice segments as they are defined in the text now). Can we apply the same restriction some lines below in the dependent slice code?

Thanks Karsten. Yes, it would be good to fix this for dependent slice segments as well. I have added the additional restriction in the attached patch (dependent_slice_restriction.patch).

Thank you. I have applied the patch in r3091