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

[Reinhard Nissl]

Hi,

Michael Niedermayer schrieb:
> On Sun, Jan 04, 2009 at 04:18:10PM +0100, Reinhard Nissl wrote:
>> 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.
> 
> still 9 pages (and i dont mean small 25line pages) of conformance
> files failing

Sorry to bother you further, but this trial and error strategy
doesn't look promising. Can I do those conformance tests myself?

I assume that my intention of playing a single I picture picked
from a DVB recording is conform, and it worked before 14289. The
only problem is that I cannot find a proper condition which gets
this working again while not breaking the other conformance tests.

Bye.
-- 
Dipl.-Inform. (FH) Reinhard Nissl
mailto:rnissl at gmx.de