[FFmpeg-devel] [PATCH] make analyze_duration work for streams with incomplete timestamps (mp3)

[Reimar Döffinger]

On Mon, Sep 14, 2009 at 03:24:30PM +0200, Michael Niedermayer wrote:
> On Mon, Sep 14, 2009 at 10:07:56AM +0200, Reimar D?ffinger wrote:
> > On Mon, Sep 14, 2009 at 01:55:06AM +0200, Michael Niedermayer wrote:
> > > the correct solution IMHO is to set duration, and actually i would naively
> > > expect compute_frame_duration)()/get_audio_frame_size() to do so, but it seems
> > > that its not otherwise this issue wouldnt exist ...
> > 
> > I think the correct solution is for the parser to set pts/dts correctly,
> > but I don't know how to do that and I don't want to wait for someone to
> > do it (because I have the impression it is expected behaviour for
> > parsers to just _SUCK_ (yes, in uppercase)).
> 
> i have more the impression people call everything they dont understand to
> be broken and to _SUCK_ granted its missing any and all documentation but
> still ...

documentation is missing and very few people know how it is supposed to
work, making it almost certain the behaviour is not even consistent.

> the next step these people do is fix that brokenness (1, see below)
> (1) if you fix it as you suggest by setting dts in the parser, duration
> still would not be set correctly and you would then require a parser for
> pretty much every codec, let me be blunt but that _SUCKS_ more

I was biased by the fact that changing this would also reduce the issue
with using AVParser in MPlayer. AFAICT you can't really use AVParser
well unless combine it with a way to also compute additional PTS/DTS values.
But unless you do that by decoding, the code for that is in libavformat
and AVParser is in libavcodec. There is no question I am rather clueless
about all this, but it just feels wrong and needlessly hard to use.

> back to the topic
> compute_pkt_fields() sets missing dts/pts for codecs that dont use frame
> reordering based on duration, not the parser. This is quite a bit simpler
> than reimplementing dts/pts advancing in the parsers, especially as that
> would require a parser for most codecs ...
> now iam still wondering why duration is not set ?

I could maybe help answering that if you gave a hint why it should be
set. I mean, it's not some kind of natural law that duration gets
magically set - and AFAICT those packets are fresh out of the parser and
just don't have all that stuff set, where should the duration have come
from (should that one be set the parser? I couldn't find a way API-wise
to do that, but I might be blind).

> PS: i think i will regret in a moment that i didnt reword a few things
> in here, ooh well iam probably in a too flameing mood

*shrug* It's not like you're wrong, though when I'm fixing bugs I consider
"I don't know how to do better" a valid argument for a patch :-P