[FFmpeg-devel] [PATCH] h264/aac in flv

[Baptiste Coudurier]

Hi Michael,

Michael Niedermayer wrote:
>>>>>>>>>>>>> [...]
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>> Updated patches considering signed ctts offset.
>>>>>>>>>>>>
>>>>>>>>>>>> Now there is a problem with timestamps, also with mov (which use signed
>>>>>>>>>>>> offsets), because pts can be < dts.
>>>>>>>>>>> dts is the time at which a packet enters the decoder
>>>>>>>>>>> pts is the time at which the correspoding decoded packet leaves the decoder
>>>>>>>>>>>
>>>>>>>>>>> Thus dts <= pts
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Also quoting  ISO/IEC 14496-12 Second edition 2005-04-01 Corrected version 2005-10-01:
>>>>>>>>>>> ---
>>>>>>>>>>> This box provides the offset between decoding time and composition time. Since decoding time must be less
>>>>>>>>>>>                                                                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>>>>>>>>> than the composition time, the offsets are expressed as unsigned numbers such that CT(n) = DT(n) +
>>>>>>>>>>> ^^^^^^^^^^^^^^^^^^^^^^^^^
>>>>>>>>>>> CTTS(n) where CTTS(n) is the (uncompressed) table entry for sample n.
>>>>>>>>>> FYI, this will be soon obsoleted by an amendment,
>>>>>>>>> link?
>>>>>>>> Sent the draft I have on hands.
>>>>>>> thanks
>>>>>>> I hope this draft is rejected, its insane.
>>>>>>> The problem is that the first dts in mov/mp4 is implicitly 0.
>>>>>>> Which doesnt work out because that just isnt always the case. Solution
>>>>>>> would be to specify this first dts.
>>>>>>>
>>>>>>> What they do is change the pts-dts offsets to signed so the pts are correct
>>>>>>> while the dts are no longer correct. (they also add various optional fields
>>>>>>> to allow one to guess the correct dts but optional == useless)
>>>>>>>
>>>>>>> The obvious implementation is to leave dts at AV_NOPTS_VALUE when there
>>>>>>> is a version 1 ctts. We could also attempt to guess the dts but this is
>>>>>>> not always possible. IP mpeg2 vs low delay IP mpeg2 being an example
>>>>>>> which is ambigous.
>>>>>>>
>>>>>> Well, I'll just let current code (ignore negative ctts) for mov/mp4 as is.
>>>>>> It seems a full H264 header parser is becoming really needed to
>>>>>> correctly compute pts (it seems gpac is good for raw h264 streams, it
>>>>>> handles bpyramid quite nicely).
>>>>>>
>>>>>> In the meantime we can still generate flv with positive dts by adding
>>>>>> delay.
>>>>> yes
>>>>>
>>>>>
>>>>>> Demuxer still handles cts as signed, this should cause no real harm,
>>>>>> except maybe some user complaining and I think Im ok with this, it will
>>>>>> remind me to think of a correct and good solution ;)
>>>>> Iam strongly against any demuxer ever returning pts < dts for undamaged
>>>>> streams.
>>>> Thing is we would loose pts for streams we would create and I don't know
>>> I dont think so. What my suggestion is, is that we would set
>>> dts to AV_NOPTS_VALUE if theres any possibility that pts<dts might occur in
>>> a stream. After all the pts values are correct its the dts values which are
>>> not
>> Yes, I understand that, problem is current code in compute_pkt_fields,
>> which will set dts to pts :(
>> Honestly I don't feel like messing with this code right now, Im pretty
>> sure to screw something up.
> 
> setting has_b_frames should fix the dts.
> (setting it too high should be rather harmless compared to setting it
> to low)

Well I tried with mov demuxer and the .mov previously sent (btw all
apple encoded .mov have negative ctts), and It didn't work, I guess
presentation delayed cannot be computed, since no parser is used. Not
sure though, I'll look further soon.

> of course setting it correctly would be best but AFAIK we cant do that
> easily. (lots of misdesigned trash containers mp4/mov flv and h264 itself of
> course which should have a mandatory reorder buffer size variable in the
> bitstream ...)

Well IMHO no reason to complain about .mp4 for now since draft is not
yet official and mandated pts >= dts.

.mov is braindead for sure, and flv even more.

> [...]
> 
>>>> +    }
>>>>      put_buffer(pb, pkt->data, size);
>>>>      put_be32(pb,size+flags_size+11); // previous tag size
>>>> -    flv->duration = pkt->pts + pkt->duration;
>>>> +    flv->duration = FFMAX(pkt->pts + pkt->duration, flv->duration);
>>> seperate comit (this might apply to a few other things as well)
>>>  
>> Sure.
> 
> patch looks ok

Applied.

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
Smartjog USA Inc.                                http://www.smartjog.com
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA