On 17/03/16 07:51, Hendrik Leppkes wrote:
> On Thu, Mar 17, 2016 at 12:50 AM, Mark Thompson <s...@jkqxz.net> wrote:
>> On 16/03/16 23:41, Hendrik Leppkes wrote:
>>> On Wed, Mar 16, 2016 at 9:37 PM, Mark Thompson <s...@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?
I apologise if I'm not understanding exactly what you mean, but wouldn't that
then accept any trailing garbage at all as long as there is one valid NAL unit?
I was trying to make the minimum change to additionally accept all streams with
trailing zero bytes like the one in the bug report. A broader modification like
that would also work, but it's not obvious (to me) that it would be a positive
change overall. (Though admittedly it does already accept up to three bytes of
trailing garbage because of the < 4 test.)
> PS: You probably want to check pkt->nb_nals, not pkt->nals, the second
> is a memory buffer.
Yep, thanks for catching that. (And somewhat disappointed that the compiler
didn't - seems to be in Wextra but not Wall for gcc.)
- Mark
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
