[FFmpeg-devel] [PATCH] H.264: fix decoding of plain still images (broken since revision 14289)
Sun Jan 4 17:00:09 CET 2009
On Sun, Jan 04, 2009 at 04:18:10PM +0100, Reinhard Nissl wrote:
> 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
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
He who knows, does not speak. He who speaks, does not know. -- Lao Tsu
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Size: 189 bytes
Desc: Digital signature
More information about the ffmpeg-devel