[FFmpeg-devel] [RFC] H.264 error concealment and mpegvideo frame handling

[Jason Garrett-Glaser]

On Sat, Sep 25, 2010 at 7:57 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> On Sat, Sep 25, 2010 at 12:18:18AM -0700, Jason Garrett-Glaser wrote:
>> On Fri, Sep 24, 2010 at 12:17 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
>> > On Fri, Sep 24, 2010 at 11:50:52AM -0700, Jason Garrett-Glaser wrote:
>> >> On Fri, Sep 24, 2010 at 10:56 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
>> >> > On Wed, Sep 22, 2010 at 01:29:35AM -0700, Jason Garrett-Glaser wrote:
>> >> >> ffh264 does not handle the case of missing reference frames correctly.
>> >> >> ?Suppose we have 16 refs and we just decoded frame num 18. ?Here's our
>> >> >> reflist:
>> >> >>
>> >> >> 18 17 16 15 14 13 12 11 10 9 8 7 6 5 4
>> >> >>
>> >> >> Then, we decode frame num 20 (19 went missing):
>> >> >>
>> >> >> 4 18 17 16 15 14 13 12 11 10 9 8 7 6 5
>> >> >>
>> >> >> This is obviously buggy and wrong. ?So I do this to fix it:
>> >> >>
>> >> >> --- libavcodec/h264.c (revision 25148)
>> >> >> +++ libavcodec/h264.c (working copy)
>> >> >> @@ -1905,6 +1905,8 @@
>> >> >> ? ? ? ? ? ? ?s->current_picture_ptr->frame_num= h->prev_frame_num;
>> >> >> ? ? ? ? ? ? ?ff_generate_sliding_window_mmcos(h);
>> >> >> ? ? ? ? ? ? ?ff_h264_execute_ref_pic_marking(h, h->mmco, h->mmco_index);
>> >> >> + ? ? ? ? ? ?ff_copy_picture(h->short_ref[0], h->short_ref[1]);
>> >> >> + ? ? ? ? ? ?h->short_ref[0]->frame_num = h->prev_frame_num;
>> >> >> ? ? ? ? ?}
>> >> >>
>> >> >> ? ? ? ? ?/* See if we have a decoded first field looking for a pair... */
>> >> >>
>> >> >> This gives us:
>> >> >>
>> >> >> 18 18 17 16 15 14 13 12 11 10 9 8 7 6 5
>> >> >
>> >> > shouldnt that be 19 18 ... ?
>> >>
>> >> 19 doesn't exist. ?We want to copy 18 into where 19 would have been,
>> >> and give it 19's frame number.
>> >
>> > thats what i ve meant, yes
>> >
>> >
>> >>
>> >> > either way ff_copy_picture() is wrong, we should for the entries that are
>> >> > left after gap handling allocate seperate pictures and fill them (this
>> >> > filling could then in the future be replaced by code that uses the b frame
>> >> > direct mode and motion vectors of the surrounding frames)
>> >>
>> >> Attached is an extremely hacky and stupid patch that fixes the
>> >> problem. ?Here's a sample that makes it blatantly obvious:
>> >> http://www.mediafire.com/?6huh99qemkrp11y . ?This sample has a frame
>> >> dropped every 5 frames, then has the encoder fix it via reference
>> >> invalidation.
>> >>
>> >> Dark Shikari
>> >
>> >> ?h264.c | ? ?8 ++++++++
>> >> ?1 file changed, 8 insertions(+)
>> >> 507842c8cfd74067c2725dc2cf32b77f392a0ba9 ?test2.diff
>> >
>> > the principle idea looks good
>> > it should use av_image_copy()
>> > and feel fre to commit once you think its clean enough
>>
>> Do we have to worry at all about emulated edge stuff? ?i.e. do any of
>> the refs have pixels beyond the edges we should copy?
>
> yes, they can have, ive forgotten about that

Possible solutions:

1.  Come up with an av_image_copy that copies those pixels?

2.  Ignore them, since out of frame MVs are not a big deal compared to
every other aspect of error concealment.

Dark Shikari