[FFmpeg-devel] [PATCH] H.264: fix decoding of plain still images (broken since revision 14289)

[Reinhard Nissl]

Hi,

Michael Niedermayer schrieb:
> On Sat, Jan 03, 2009 at 07:01:32PM +0100, Reinhard Nissl wrote:
>> Hi,
>>
>> the offending change in that revision seems to me the following:
>>
>> -            if(prev && pics <= s->avctx->has_b_frames ||
>> out_of_order)
>> -                out = prev;
>> +            if(pics <= s->avctx->has_b_frames || out_of_order)
>> +                out = NULL;
>>
>> In the case of a plain still image, there are no previous images.
>>  Revision 14288 honored this by checking prev and therefore
>> output the current image (the still image).
>> In Revision 14289, this test is missing and therefore, no picture
>> is output. I've to add that pics as well as has_b_frames was 1
>> while out_of_order was 0 when checking the condition in both
>> revisions.
>>
>> As you can see from the attached patch, the location of this test
>> has moved meanwhile and the condition as well as the behavior has
>> been inverted, but still lacks a test for "no previous image".
>>
>> I've added such a test by using outputted_poc and comparing it to
>> INT_MIN, which is the initial value of this variable.
>>
>> For testing, I've uploaded two sample files into subdirectory
>> rnissl: one with a still frame and one with a pair of fields. See
>> readme.txt for more information.
> 
> this patch totally breaks h264 decoding, thus rejected.
> id post the list of what it breaks but its as far as i can see
> every single h264 file from the conformance suite so theres no point.
> 
> Besides i do not know what you mean by "still image" which page
> of the h264 spec describes it?

Well, didn't have a look into the spec so far, but my sample
files just contain a single I picture (either a frame or a pair
of fields). Decoding works without problem but the decoded
picture is simply not output, starting with that revision.

In my opinion, this condition has to deal with B pictures.
has_b_frames has been set due to flag found in the sequence
header (if I recall correctly), as the streams where these I
pictures are taken from contain B pictures.

In the case where this flag would have been not present in the
stream, pics would be larger than has_b_frames and the picture
would be output.

I've now attached a different patch which fixes this issue. It
checks whether out is an I picture. I hope this change is more
restrictive than the one from yesterday and doesn't break the
reference files.

Bye.
-- 
Dipl.-Inform. (FH) Reinhard Nissl
mailto:rnissl at gmx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ffmpeg_fix_stills2.patch
Type: text/x-patch
Size: 581 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090104/1edd6978/attachment.bin>