[FFmpeg-devel] [PATCH] flv duration

[Howard Chu]

Michael Niedermayer wrote:
> On Tue, Apr 06, 2010 at 05:34:16PM -0700, Howard Chu wrote:
>> Michael Niedermayer wrote:
>>> On Sun, Apr 04, 2010 at 04:24:54AM -0700, Howard Chu wrote:
>>>> elupus wrote:
>>>>> On Sat, 03 Apr 2010 18:41:24 -0700, Howard Chu wrote:
>>>>>> Spoke too soon. Not setting s->duration breaks XBMC.
>>>>>
>>>>> In what way? lavf should have populated that from the stream duration
>>>>> anyway.
>>>>
>>>> I don't know why, but it didn't, it was zero. And since by default XBMC
>>>> uses percentage-based seeks, all of my seek requests were turned into
>>>> seeks
>>>> to zero. Restoring the line to set s->duration made it all work fine.
>>>>
>>>> I have to admit I wasn't really interested in digging to see why it
>>>> wasn't
>>>> set by something else in lavf. I'm content knowing that this code was in
>>>> there originally, and things were working. My first patch preserved the
>>>> existing behavior, so I'm going to stick with it. This code base is just
>>>> too rife with unintended consequences, and there was no good reason to
>>>> remove that line in the first place.
>>>
>>> The good reason was the API, but its unpopular to read the docs
>>>      /** Decoding: duration of the stream, in AV_TIME_BASE fractional
>>>          seconds. NEVER set this value directly: it is deduced from the
>>>          AVStream values.  */
>>>       int64_t duration;
>>>
>>> maybe the (internal after all) API should be changed and demuxers should
>>> set the AVFormatContext duration if thats the only thing they know. That
>>> does seem to make sense but it requires that api text to be changed,
>>> ill do that.
>>
>> OK, on further digging I found the problem was that XBMC wasn't probing the
>> stream info before trying to open its codecs. Fixing that in XBMC also
>> fixed this problem. So sorry for the noise. This patch is fine.
>>
>> https://lists.mplayerhq.hu/pipermail/ffmpeg-devel/2010-April/086354.html
>
> that one is now no longer correct as i changed the api to allow
> AVFormatContext.duration to be set

But setting it still doesn't accomplish anything in the normal case. 
av_has_duration() still only looks for a duration in the individual streams. 
The purpose of this patch was to shut up the warning about "Estimating 
duration" and the patch does what is necessary and sufficient for that 
purpose. Of course, the only difference between that patch and the original 
one here 
https://lists.mplayerhq.hu/pipermail/ffmpeg-devel/2010-April/086328.html is 
whether it sets the AVFormatContext.duration.

At this point you're talking about one line of difference. If you're OK with 
the patch otherwise, then it would be simpler for you to just commit whatever 
fix you think is appropriate. All of this back and forth is just wasting time.
-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/