[FFmpeg-devel] [PATCH] flv duration

[Michael Niedermayer]

On Tue, Apr 06, 2010 at 07:25:37PM -0700, Howard Chu wrote:
> 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.

I see nothing in this patch that is correct.
Hiding a warning about estimated bitrate&duration by explicitly setting
the bitrate incorrectly and thus changing a "might be wrong to a is wrong"
isnt really an improvment.

[...]

-- 
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
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100407/fd2c144f/attachment.pgp>