[FFmpeg-devel] [PATCH] lavc/hevc: Allow arbitrarily many trailing_zero_8bits after a NAL unit in bytestream format.

[Hendrik Leppkes]

On Thu, Mar 17, 2016 at 12:50 AM, Mark Thompson <sw at jkqxz.net> wrote:
> On 16/03/16 23:41, Hendrik Leppkes wrote:
>> On Wed, Mar 16, 2016 at 9:37 PM, Mark Thompson <sw at jkqxz.net> wrote:
>>> ---
>>> libavcodec/hevc_parse.c | 14 ++++++++++++++
>>> 1 file changed, 14 insertions(+)
>>>
>>> diff --git a/libavcodec/hevc_parse.c b/libavcodec/hevc_parse.c
>>> index 63ed84a..8c629ff 100644
>>> --- a/libavcodec/hevc_parse.c
>>> +++ b/libavcodec/hevc_parse.c
>>> @@ -227,6 +227,20 @@ int ff_hevc_split_packet(HEVCContext *s, HEVCPacket *pkt, const uint8_t *buf, in
>>>                 return AVERROR_INVALIDDATA;
>>>             }
>>>         } else {
>>> +            if (pkt->nals > 0) {
>>> +                // Discard arbtrarily many trailing_zero_8bits before the
>>> +                // start code of the next NAL unit.
>>> +                while (buf[0] == 0 && buf[1] == 0 && buf[2] == 0) {
>>> +                    ++buf;
>>> +                    --length;
>>> +                    if (length < 4)
>>> +                        break;
>>> +                }
>>> +                if (length < 4) {
>>> +                    // There are only zeroes left, so no more NAL units here.
>>> +                    break;
>>> +                }
>>> +            }
>>>             /* search start code */
>>>             while (buf[0] != 0 || buf[1] != 0 || buf[2] != 1) {
>>>                 ++buf;
>>
>> I'm slightly confused, wouldn't the loop right after skip over the
>> zeroes until it finds a valid start code?
>> It essentially does the same on a cursory look.
>
> Yes - it doesn't change the scanning process, but it does change the return value because in the current code we return failure (and give up on the packet entirely) if there aren't any more NAL units to be found.
>
> We actually want to return success if we already have at least one NAL unit and then there are zeroes to the end of the packet.
>

Wouldn't it be simpler then to add a condition to the existing loop to
only error out when no NALs exist?
PS: You probably want to check pkt->nb_nals, not pkt->nals, the second
is a memory buffer.

- Hendrik