[FFmpeg-devel] [PATCH] AVPacket.display_duration

[Baptiste Coudurier]

Michael Niedermayer wrote:
> On Wed, Sep 10, 2008 at 10:51:25AM -0700, Baptiste Coudurier wrote:
>> Michael Niedermayer wrote:
>>> On Mon, Sep 08, 2008 at 07:21:09PM -0700, Baptiste Coudurier wrote:
>>>> Hi guys,
>>>>
>>>> Michael Niedermayer wrote:
>>>>> On Tue, Sep 09, 2008 at 01:56:29AM +0200, Aurelien Jacobs wrote:
>>>>>> Michael Niedermayer wrote:
>>>>>>
>>>>>>> On Fri, Sep 05, 2008 at 05:35:13PM +0200, Aurelien Jacobs wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> For some subtitles track, each AVPacket must carry the display duration
>>>>>>>> of the contained subtitle.
>>>>>>>> The attached patch adds a dedicated AVPacket.display_duration field.
>>>>>>> Iam not against this, but i would like to understand better for which cases
>>>>>>> it is needed.
>>>>>>>
>>>>>>> plain UTF-8 and ASCII ?
>>>>>> Exactly.
>>>>>>
>>>>>>> If so does mov, avi, asf, ... support plain UTF-8 and ASCII ?
>>>>>> I don't think so...
>>>>>>
>>>>>>> if yes how do they know the display_duration ?
>>>>>> The fact that they can't store any display duration just prevent
>>>>>> them from storing plain text subtitles. At least I think so.
>>>>>>
>>>>>> Another possibility would be to not support any kind of plain text
>>>>>> subtitles at all in ffmpeg. Demuxers could convert them to SRT,
>>>>>> which is basically plain text + timing information. But I'm still
>>>>>> not sure if I really like this solution.
>>>>> iam fine with adding display_duration to AVPacket, the only minor
>>>>> dislike i have with this solution is that codecs with very small
>>>>> packets might see a minor speedloss with added fields. But then
>>>>> not adding needed fields is no solution, some other solution for
>>>>> these small packet audio codecs has to be found if it turns out that
>>>>> the AVPacket handling overhead matters.
>>>>>
>>>>> So again iam ok with adding display_duration.
>>>>>
>>>> So Matroska packets will have 3 kinds of duration ? normal, convergence,
>>>> and display ? Or is display supposed to replace convergence ? Or am I
>>>> missing something ?
>>> There would be 3 different kinds of durations, i do not know if matroska will
>>> set convergence_duration though, probably not.
>> If matroska does not use convergence_duration, I think it should rename
>> the field to display_duration, 
> 
>> after all convergence_duration was added
>> for Matroska. 
> 
> I added it for h.264 and nut
> thats also why i called the time when i added it unwise, it confused almost
> everyone.

Okay, however IMHO it's better to add fields when they are actually used.

>> IMHO better not having useless fields.
> 
> it is not used yet but not useless.
> that said i do not mind it to be replaced but once we add support for some
> kinds of h.264 it could come in handy and might be added back again.

Yes, of course if the field is needed and used.

>>> I also do not know if this is the best solution, the obvious alternative
>>> to display_duration is to have the matroska demuxer convert the codec
>>> bitstream to a representation that contains the display_duration.
>>> It seems that for all subtitle formats such representations exists, ASS as
>>> well as plain text (SRT).
>>>
>> What about stream copy ? Should the muxer reformat packets too ?
> 
> IMHO, if the muxer ever reformats then it should always do, so yes

Im fine with this.

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