[FFmpeg-devel] [PATCH] libavcodec/h263dec.c: Duplicate the last decoded frame when xvid marks the packet as skipped.

[Michael Niedermayer]

On Thu, Oct 19, 2017 at 04:20:48PM -0700, Thierry Foucu wrote:
> On Thu, Oct 19, 2017 at 3:43 PM, Michael Niedermayer <michael at niedermayer.cc
> > wrote:
> 
> > On Thu, Oct 19, 2017 at 09:51:05AM -0700, Thierry Foucu wrote:
> > > Instead of returning nothing when we detect the xvid skipped frame, we
> > > could return the last decoded frame, if we do have one.
> >
> > FRAME_SKIPPED is not limited to xvid (packed frames). There are a few
> > other pathes that return it.
> >
> >
> What if i change the test to check for mpeg4 video codec, something like
> 
>  if (ret == FRAME_SKIPPED) {
> +        if (CONFIG_MPEG4_DECODER && actvx->codec_id == AV_CODEC_ID_MPEG4
> && s->next_picture_ptr) {
> +            if ((ret = av_frame_ref(pict, s->next_picture_ptr->f)) < 0) {
> +                return ret;
> +            }
> +            *got_frame = 1;
> +        }
>          return get_consumed_bytes(s, buf_size);
> +    }
> 
> or should i do something specific for xvid bug? I could return another
> value for the xvid (packed frames)  here
> https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/mpeg4videodec.c#L2590
> 
> and check for it instead of the frame_skipped.

FRAME_SKIPPED is returned in several cases

* vop not coded
 Your code seems correct for this but a testcase would be very usefull
 to confirm this
 It would be more ideal to limit this to the case where the last frame
 differs from the returned frame. As outputting no frame is cheaper
 than outputing a repeated frame

* some xvid/divx specific cases
 I dont know what is correct here

* some error conditions
 Your code is probably not optimal in some cases here

* skiped frames in i263
 your patch shouldnt lead to wrong output but it would slow the code
 down by having to handle more frames


> 
> For standard mpeg4video, outputing the next frame smells like violating
> > the spec.
> >
> >

> i'm not output the next frame, i'm output the last decoded frame

i dont think thats the case with B frames.
next/last are the reference frames for B frames, If the last is a B
frame it should not be in there.
What your code does seems to match what the standard mpeg4 wants for
vop_coded = 0 though. (this would make your code a bugfix for a case
i think i have never seen, vop_coded = 0 with b frames)



[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20171021/f0f9805e/attachment.sig>