[FFmpeg-devel] [BUG] h264 picture reordering fails

[Michael Niedermayer]

On Sat, Jun 20, 2009 at 02:02:01PM +0900, Haruhiko Yamagata wrote:
>> On Thu, Jun 18, 2009 at 09:46:27PM +0900, Haruhiko Yamagata wrote:
>>>> On Sun, May 31, 2009 at 05:51:33PM +0900, Haruhiko Yamagata wrote:
>>>>>> On Sat, May 30, 2009, Haruhiko Yamagata wrote:
>>>>>>>> On Sat, May 30, 2009 at 07:34:32PM +0900, Haruhiko Yamagata wrote:
>>>>>>>>> I found picture reordering fails in
>>>>>>>>> http://x264.nl/h.264.samples/premiere-paff.ts
>>>>>>>>>
>>>>>>>>> Outputted pictures have POC {-4, -2, 4, 0, 2, 6}.
>>>>>>>>> Are negative POCs allowed?
>>>>>>>>>
>>>>>>>>> If yes, zero as POC is valid as well.
>>>>>>>>>
>>>>>>>
>>>>>>>> about POCs, IIRC the spec requires POC=0 to be true and only true 
>>>>>>>> for
>>>>>>>> IDR pictures, but i might misremember (would have to check the spec)
>>>>>>>> What i do know is that the POC or frame number == 0 rules in the 
>>>>>>>> spec
>>>>>>>> are not followed by some encoders.
>>>>>>>
>>>>>>> Yes, I think POC=0 means IDR. In that case, negative values as POC 
>>>>>>> are
>>>>>>> not allowed.
>>>>> On Sun, May 31, 2009, fenrir wrote:
>>>>>> This point has been discusses on the JVT mailing list, it was said 
>>>>>> that
>>>>>> negative POC are valids, they just mean that the picture has to be 
>>>>>> displayed
>>>>>> before the IDR.
>>>>>
>>>>> OK, I understand.
>>>>> Then negative POC should typically be -2, while longer reference chains 
>>>>> are theoretically possible.
>>>>>
>>>>> Obviously it is not the case with mine. The frame with POC 0 happens to 
>>>>> be a B frame.
>>>>
>>>> what POC value does the reference impl give that frame?
>>>> If a B frame ends at POC=0 that seems a bug in the encoder and we should 
>>>> find
>>>> a way to handle that POC=0 non IDR case not chnage POC
>>>> OTOH
>>>> if the reference gives it a POC != 0 then our poc code is buggy
>>>
>>> JM decoder gives POC=0 to the B frame. The log attached.
>>>
>>> I extracted the bitstream from the GDR frame to the end.
>>> If you need the .264 file I used to get the log, please let me know.
>>
>> could you try to remove all poc==0 checks related to picture ordering and
>> just leave the IDR checks, maybe that would work? (also has to be tested
>> against a few other streams ...)
>
> OK, checking h->delayed_pic[i]->key_frame looks sufficient to detect IDR.
> Patch attached.
>
> I have checked rev 14347 and 15153. I also have read issue 576.
> This change seems to be OK with cathedral-beta2-400extra-crop-avc.mp4.

it seems you have missed
cross_idr = !h->delayed_pic[0]->poc || !!h->delayed_pic[i] || h->delayed_pic[0]->key_frame;
            ^^^^^^^^^^^^^^^^^^^^^^^
or is there a reason why this is not changed?

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090620/c827da94/attachment.pgp>