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

[Michael Niedermayer]

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
?


[...]
> --- h264.c	2008-05-25 21:16:01.134169000 -0700
> +++ h264.c	2008-05-25 21:18:26.749234000 -0700

looks ok

[...]

-- 
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080527/be18656a/attachment.pgp>