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

[Reimar Döffinger]

On Mon, Sep 14, 2009 at 01:55:06AM +0200, Michael Niedermayer wrote:
> On Sun, Sep 13, 2009 at 12:41:09PM +0200, Reimar D?ffinger wrote:
> > currently max_analyze_duration does not work for e.g.
> > http://208.80.52.108:80/KDLDFM because the mpeg audio parser is very
> > thorough in making a mess of timestamps.
> > Basically, only every 4th packet has a timestamp and thus none at all
> > have any duration.
> > Attached patch handles this case by using the difference between the
> > minimum and current dts for comparing against max_analyze_duration when
> > we have no other duration info.
> 
> i think your code introduces a nasty bug, namely it fails fatally in case
> of positive timestamp discontinuities, that is the code retunrns before its done

IMO that is less "fatally" than the current code that for live streams
like the one I gave where it _never_ returns, not after days or
downloading GBs of data.

> also we already have the last_dts table that could be used but really i dont
> think this is the correct solution

You may think so. I originally thought so too, but last_dts in this case
is always the dts of the last packet, not the last valid one, so either
last_dts or the current dts are always AV_NOPTS_VALUE and thus it is
completely useless for this.

> 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)).