[FFmpeg-devel] [PATCH] flv duration
Wed Apr 7 13:44:58 CEST 2010
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
>>>>> 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
>>>>> 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
>>>>> set by something else in lavf. I'm content knowing that this code was
>>>>> 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
>>>>> 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
>>> 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.
>> 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
Size: 189 bytes
Desc: Digital signature
More information about the ffmpeg-devel