[FFmpeg-devel] [RFC] Error concealment for B-frames/fixing issue 824

[Baptiste Coudurier]

Baptiste Coudurier wrote:
> On 5/5/2009 8:34 PM, Michael Niedermayer wrote:
>> On Tue, May 05, 2009 at 07:56:39PM -0700, Baptiste Coudurier wrote:
>>> On 5/5/2009 5:17 PM, Michael Niedermayer wrote:
>>>> On Tue, May 05, 2009 at 02:08:33PM -0700, Baptiste Coudurier wrote:
>>>>> Baptiste Coudurier wrote:
>>>>>> On Fri, Apr 10, 2009 at 06:50:08PM -0700, Baptiste Coudurier wrote:
>>>>>>> Hi Reimar,
>>>>>>>
>>>>>>> On 4/9/2009 3:13 PM, Reimar D?ffinger wrote:
>>>>>>>> On Thu, Apr 09, 2009 at 10:52:32PM +0200, Michael Niedermayer wrote:
>>>>>>>>> there are 2 very seperate things
>>>>>>>>> 1. errors
>>>>>>>>> 2. b frames without reference frames after seeking
>>>>>>>>>
>>>>>>>>> normal users dont want to see randomly trashed frames after seeking
>>>>>>>>> in undamged files
>>>>>>>> I think I agree... Is there a counter somewhere that could be used to
>>>>>>>> find out which frame in the GOP we are at?
>>>>>>> Yes temp_ref in pic structure.
>>>>>>>
>>>>>>>> Some options I can think of:
>>>>>>>> 1) change code to not skip a B-frame if it is the second frame in a closed GOP
>>>>>>>> 2) only skip B-frames if they directly follow the first I-frame of a non-closed GOP
>>>>>>>> 3) (without really knowing what "broken link" means) only skip B-frames if "broken
>>>>>>>> link" is set
>>>>>>> From what I understand, broken link explicitly state that the 2 next b
>>>>>>> frames cannot be decoded because the stream was cut at the preceding I
>>>>>>> frame, therefore the original reference I frame is no more available.
>>>>>>>
>>>>>>> And I think your strategy should be ok :)
>>>>>> Here is another attempt.
>>>>>>
>>>>>> It will decodes them only is closed gop is set.
>>>>>> I got a sample with open gop and B frames before the I frame, it
>>>>>> works, also, decoding them anyway does not crash when dummy picture is
>>>>>> allocated some block are gray.
>>>>>>
>>>>>> Patch attached.
>>>>>>
>>>>> New patch attached, more correct.
>>>> looks ok
>>> I've tested it more to be sure and I've encountered this problem during
>>> seeking with ffplay:
>>>
>>> [mpeg2video @ 0x223f6a0]pic->data[0]!=NULL in avcodec_default_get_buffer
>>> [mpeg2video @ 0x223f6a0]get_buffer() failed (-1 2147483647 8 0x3269950)
>>> [mpeg2video @ 0x223f6a0]pic->data[0]!=NULL in avcodec_default_get_buffer
>>> [mpeg2video @ 0x223f6a0]get_buffer() failed (-1 2147483647 8 0x3269950)
>>> Seek to 63% ( 0:01:23) of total duration ( 0:02:12)
>>> [mpeg2video @ 0x223f6a0]pic->data[0]!=NULL in avcodec_default_get_buffer
>>> [mpeg2video @ 0x223f6a0]get_buffer() failed (-1 2147483647 8 0x3140e30)
>>> [mpeg2video @ 0x223f6a0]pic->data[0]!=NULL in avcodec_default_get_buffer
>>> [mpeg2video @ 0x223f6a0]get_buffer() failed (-1 2147483647 8 0x3140e30)
>>>
>>> So I guess allocating last_picture this way was not correct.
>>>
>>> I think copying next picture to last picture is better then.
>> i dont mind the picture being copied (pixel wise) but having
>> s2->last_picture_ptr == s2->next_picture_ptr
>> gives me a bad feeling, this is not a conditiom that could be true otherwise
> 
> Ok. It seems copying and not setting last_picture_ptr to
> next_picture_ptr does not work and crashes. So I feel going to back to
> first solution and finding where to correctly free the pic. Any hint ?
> 

Ok, I've done some investigation and it seems the crash is because
default_release_buffer is called while pic->type == FF_BUFFER_TYPE_COPY,

I'm not sure where the fix should take place though, but I can find a
few places where free_frame_buffer is called while pic->type is
potentially COPY.

int MPV_frame_start(MpegEncContext *s, AVCodecContext *avctx)
{
    int i;
    AVFrame *pic;
    s->mb_skipped = 0;

    assert(s->last_picture_ptr==NULL || s->out_format != FMT_H264 ||
s->codec_id == CODEC_ID_SVQ3);

    /* mark&release old frames */
    if (s->pict_type != FF_B_TYPE && s->last_picture_ptr &&
s->last_picture_ptr != s->next_picture_ptr &&
s->last_picture_ptr->data[0]) {
      if(s->out_format != FMT_H264 || s->codec_id == CODEC_ID_SVQ3){
          free_frame_buffer(s, s->last_picture_ptr);

Does this check implies that in some situation, last_picture_ptr might
be next_picture_ptr ? In this case, is it so wrong to set it so for the
closed gop case ?

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
FFmpeg maintainer                                  http://www.ffmpeg.org