[FFmpeg-devel] [PATCH] Fix timestamp calculation when demuxing MP3 frames in a .avi container.

Michael Niedermayer michaelni at gmx.at
Fri Jul 13 16:37:36 CEST 2012

On Fri, Jul 13, 2012 at 03:46:01PM +0200, Michael Niedermayer wrote:
> On Fri, Jul 13, 2012 at 08:49:24AM -0400, Mike Scheutzow wrote:
> > On 7/12/2012 10:33 PM, Michael Niedermayer wrote:
> > >On Thu, Jul 12, 2012 at 06:07:18PM -0400, Mike Scheutzow wrote:
> > >>On 7/11/2012 9:34 PM, Michael Niedermayer wrote:
> > >>>>+                        if (fps > 41) {
> > >>>>+                            // highest valid frame per sec is 48000/1152
> > >>>>+                            av_log(s, AV_LOG_WARNING, "st%d: MP3 frame rate is invalid (%d FPS)\n", st->id, fps);
> > >>>>+                        } else {
> > >>>did this if() make any difference in any of your tests ?
> > >>>iam asking because it looks kind of ugly not saying iam against it if
> > >>>it is needed
> > >>Results from web search indicate that some (broken) software tools
> > >>create avi files with the audio sample rate in stream's header
> > >>information, rather than the audio frame rate that we need. This is
> > >>a check for that problem. It will also help to notify the user why
> > >>the avi file is not playing back properly.
> > >>
> > >>I did not bother to actually find such a sample file. The check
> > >>serves a worthwhile purpose.
> > >someone might have encoded 96khz in mp3 (claiming it to be 48khz)
> > >or someone wight wanted to speedup a files a little to 49khz, the check
> > >can cause problems for these cases.
> > >Its not a big thing as it breaks nothing that works currently but
> > >it doesnt feel right as it is somehow ...
> > 
> > If this hypothetical file ever shows up in a bug report, we can
> > decide what to do with it.
> > 
> > As things are now, ffmpeg does not properly demux any 48 kHz mp3 in
> > .avi and does not even print a warning.
> > 
> > I am spending no more time on this patch.
> ok, ill look at it and try to analyze and fix the bug.
> thanks for your investigation, help and patch.
> and sorry for the troubble but i think this is a complicated bug that
> needs more investigation and the patch just doesnt look right.

Ive posted a patch that fixes the issue, it makes no changes to the
demuxer side.
The timestamps became wrong during stream copy they where not "wrong"
before, so this seems more correct to me.
It might be that theres also a bug on the demuxer side and that
your patch is fixing that but iirc the where neither arguments
supporting this view nor against it in this thread
It would be interresting to test microsofts demuxer with a few
cbr & vbr avi and compare its timestamps to our timestamps before and
after the patch. This would be a definite awnser to which way is
closer to how the "reference" demuxer is doing it.



Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In fact, the RIAA has been known to suggest that students drop out
of college or go to community college in order to be able to afford
settlements. -- The RIAA
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120713/8c8cb6eb/attachment.asc>

More information about the ffmpeg-devel mailing list