[FFmpeg-devel] [PATCH] missing last frame for h264 decoding + CODEC_FLAG_TRUNCATED

[Pascal Massimino]

  Michael,

On Tue, May 27, 2008 at 10:10 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> On Sun, May 25, 2008 at 09:23:26PM -0700, Pascal Massimino wrote:
>>   Michael,
>>
>> On Sat, May 24, 2008 at 2:53 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
>> > On Fri, May 23, 2008 at 01:36:13PM -0700, Pascal Massimino wrote:
>> >>   Hi everybody,
>> >>
>> >>   attached a patch for h264 decoding in CODEC_FLAG_TRUNCATED mode:
>> >>   even during the last call to decode_frame() with buf_size = 0, i'd say
>> >>   we need to flush the buffers in the ParseContext before calling it
>> >> quit for good.
>> >>
>> >>   Also: when flushing (for seek), we need to wipe the ParseContext too, to
>> >>   get rid of pending frame fragments.
>> >>
>> >>   I've uploaded a very small 'test_2frames.264' raw h264 file into incoming/
>> >>   on the ftp.
>> >>   Without this patch, repeated call to decode_frame() wouldn't decode the
>> >>   last frame.
>> >>
>> >> skal
>> >
>> >> --- h264.c.orig       2008-05-20 08:15:32.673356000 -0700
>> >> +++ h264.c    2008-05-23 13:26:22.272216000 -0700
>> >> @@ -3314,6 +3314,9 @@
>> >>          h->s.current_picture_ptr->reference= 0;
>> >>      h->s.first_field= 0;
>> >>      ff_mpeg_flush(avctx);
>> >> +    if (h->s.flags&CODEC_FLAG_TRUNCATED){
>> >> +        memset(&h->s.parse_context, 0, sizeof(h->s.parse_context));
>> >> +    }
>> >
>> > ff_mpeg_flush() should already clean parse_context, the memset might
>> > instead result in a memleak
>>
>>  indeed. Removed.
>>
>> >
>> >
>> >>  }
>> >>
>> >>  /**
>> >> @@ -7655,6 +7658,21 @@
>> >>      s->flags= avctx->flags;
>> >>      s->flags2= avctx->flags2;
>> >>
>> >> +    if(s->flags&CODEC_FLAG_TRUNCATED){
>> >> +        int next;
>> >> +        uint8_t tmp[FF_INPUT_BUFFER_PADDING_SIZE] = { 0 };
>> >
>> >> +        if (buf_size > 0) {
>> >> +            next= ff_h264_find_frame_end(h, buf, buf_size);
>> >> +        } else {
>> >> +            buf= tmp;
>> >> +            next= END_NOT_FOUND;
>> >> +        }
>> >
>> > ff_h264_find_frame_end(, , 0) should return END_NOT_FOUND
>> > and buf should not be used with buf_size=0 thus it shouldnt matter what its
>> > value is i think
>>
>>  i'd say ff_combine_patch needs a small fix for that (attached: parser.patch).
>>  Updated the original patch (trunc.patch)
>>
>> skal
>
>> --- parser.c.orig     2008-05-25 21:10:11.949075000 -0700
>> +++ parser.c  2008-05-25 21:10:40.690929000 -0700
>> @@ -256,8 +256,10 @@
>>          if(!new_buffer)
>>              return AVERROR(ENOMEM);
>>          pc->buffer = new_buffer;
>> -        memcpy(&pc->buffer[pc->index], *buf, *buf_size);
>> -        pc->index += *buf_size;
>> +        if (*buf_size > 0) {
>> +            memcpy(&pc->buffer[pc->index], *buf, *buf_size);
>> +            pc->index += *buf_size;
>> +        }
>
> I would have expected the following to
>    /* flush remaining if EOF */
>    if(!*buf_size && next == END_NOT_FOUND){
>        next= 0;
>    }
> prevent the code above from being executed
> ?

  oh! indeed. Overlooked that. Patch was unnecessary.
>
>
> [...]
>> --- h264.c    2008-05-25 21:16:01.134169000 -0700
>> +++ h264.c    2008-05-25 21:18:26.749234000 -0700
>
> looks ok
>

  submitted.

skal

> [...]
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Let us carefully observe those good qualities wherein our enemies excel us
> and endeavor to excel them, by avoiding what is faulty, and imitating what
> is excellent in them. -- Plutarch
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.6 (GNU/Linux)
>
> iD8DBQFIPECaYR7HhwQLD6sRAjD8AKCQlm8WkRlkkk5zyN0Lgl1xPgqkiQCgkT/U
> Mr8Exjg4yjEnopQeUNsh3yw=
> =ARlk
> -----END PGP SIGNATURE-----
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at mplayerhq.hu
> https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel
>