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

[Michael Niedermayer]

On Thu, Jul 05, 2012 at 11:50:54AM -0400, Mike Scheutzow wrote:
> Mike Scheutzow wrote:
> >Michael Niedermayer wrote:
> >>On Wed, Jun 27, 2012 at 11:51:10AM -0400, Mike Scheutzow wrote:
> >>>The attached patch fixes trac 1432.
> >>>
> >>>The bad code was using the frame byte length to calculate a timestamp,
> >>>which is completely broken. Also set AVPacket.duration, since demux is
> >>>already calculating it. The existing avi muxer already properly handles
> >>>the muxing and index creation for MP3 audio frames.  Fixes the sample
> >>>file in trac 1432.
> >>
> >>code breaks make fate
> >>also please test your patch against cbr & vbr style muxed avi
> 
> Updated patch attached. Fate passes on my linux box. Tested with CFR
> and VFR avi files.
> 
> If no problems, reviewer can commit.
> 
> 
> Mike Scheutzow

>  avidec.c |   22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> aadc02ed1641c9f7c823fe61d28aa0142e0e28b1  avi-mp3-v2.patch
> From 9b49a37769c7a80906e9ac3fae2c4d971f855587 Mon Sep 17 00:00:00 2001
> From: Mike Scheutzow <mjs973 at optonline.net>
> Date: Wed,  4 Jul 2012 11:28:07 -0400
> Subject: [PATCH] avidec: Fix timestamp calc for MP3 frames.
> 
> Correctly handle the demuxing of variable length MP3 frames from an
> .avi container. The bad code was using the frame byte length to calculate
> a timestamp, which is completely broken. The existing avi muxer already
> properly handles the muxing and index creation for variable length MP3 
> audio frames.
> 
> Fixes the sample file in trac 1432. Fate passes. Tested with both Constant
> Frame Rate and Variable Frame Rate avi files.
> 
> Signed-off-by: Mike Scheutzow <mjs973 at optonline.net>
> ---
>  libavformat/avidec.c |   22 ++++++++++++++++++++++
>  1 files changed, 22 insertions(+), 0 deletions(-)
> 
> diff --git a/libavformat/avidec.c b/libavformat/avidec.c
> index fea0694..1fca257 100644
> --- a/libavformat/avidec.c
> +++ b/libavformat/avidec.c
> @@ -116,6 +116,10 @@ static int guess_ni_flag(AVFormatContext *s);
>             (tag >> 24) & 0xff,                          \
>             size)
>  
> +/* calculate the duration of the current frame, in units of AVStream.time_base.
> + * For video, or audio frames with variable byte length, the output value
> + * is defined to be '1'.
> + */
>  static inline int get_duration(AVIStream *ast, int len){
>      if(ast->sample_size){
>          return len;
> @@ -656,6 +660,20 @@ static int avi_read_header(AVFormatContext *s)
>                      if (ret < 0)
>                          return ret;
>                      ast->dshow_block_align= st->codec->block_align;
> +                    if (st->codec->codec_id == CODEC_ID_MP3 && !ast->sample_size) {
> +                        /* st->time_base gives index entry play duration */
> +                        int fps = 0;
> +                        if (st->time_base.num) {
> +                            fps = st->time_base.den / st->time_base.num;
> +                        }

> +                        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

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you think the mosad wants you dead since a long time then you are either
wrong or dead since a long time.
-------------- 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/20120712/3abf09c5/attachment.asc>