[FFmpeg-devel] [PATCH] ADPCM IMA AMV encoder (try 1)

[Michael Niedermayer]

On Thu, Oct 11, 2007 at 10:59:30PM +0700, Vladimir Voroshilov wrote:
> 2007/10/8, Benjamin Larsson <banan at ludd.ltu.se>:
> > Hi.
> >
> > Vladimir Voroshilov wrote:
> > > 2007/10/6, Vladimir Voroshilov <voroshil at gmail.com>:
> > >
> > >> Hi, All
> > >>
> > >> Since no reply was got in "Devel question : how to calculate
> > >> frame_size in audio encoder (adpcm_encode_init) "' thread, i want (as
> > >> suggested by Vitor) show you current version of ADPCM IMA AMV encoder
> > >> in amv-codec-tools project.
> > >>
> > >> Due to fake codec tag (it uses PCM's 0x01 code) this codec can not be
> > >> easily used outside AMV container.
> > >>
> > >> Number of 16-bit audio samples  per audio frame and audio frame size are
> > >> calculated by folmulas below:
> > >>
> > >> samples_per_frame=audio_sample_rate/video_frame_rate
> > >> audio_frame_size=(samples_per_frame/2)+8
> > >>
> > >> Current code sets frame_size to 0 inside adpcm_encode_init.
> > >> frame_size receives real value at the end of avi_write_header:
> > >>
> > >> ============CUT=================
> > >>    /*
> > >>      HACK!!!
> > >>      Set correct frame_size for audio stream
> > >>     */
> > >>     if(s->nb_streams > 1 && s->streams[1]->codec->codec_type ==
> > >> CODEC_TYPE_AUDIO) {
> > >>             s->streams[1]->codec->frame_size=av_rescale(
> > >>                     s->streams[1]->codec->sample_rate,
> > >>                     s->streams[0]->codec->time_base.num,
> > >>                     s->streams[0]->codec->time_base.den);
> > >>     }
> > >> ===========CUT==================
> > >>
> > >> I want to know how much attached patch is ugly/hackish and receive
> > >> emproovement sugesstions.
> > >>
> > >
> > > Attached patch adpcm2.diff is updated version with trellis quantization.
> > >
> > >
> > >> P.S. current code does not rearrange extra samples yet (i,e for
> > >> arate=22050 and fps=16 real audio rate will be 16*(22050/16)=22048,
> > >> proprietary encodec puts this two extra samples in last 16th frame). I
> > >> will be happy to recevie good solution for extra samples.
> > >>
> > >
> > > Attached patch adpcm_samplerate_compensation.diff adds simple
> > > sompensation: putting all extra samples into last frame of the second.
> > >
> > >
> > >
> > > ------------------------------------------------------------------------
> > >
> > >
> > > --- ffmpeg.orig/libavcodec/adpcm.c    2007-10-04 22:21:14.000000000 +0700
> > > +++ ffmpeg/libavcodec/adpcm.c 2007-10-07 13:10:50.000000000 +0700
> > > @@ -146,6 +146,7 @@
> > >      int channel; /* for stereo MOVs, decode left, then decode right, then tell it's decoded */
> > >      ADPCMChannelStatus status[2];
> > >      short sample_buffer[32]; /* hold left samples while waiting for right samples */
> > > +    int extra_samples_count; //number of samples to put into next frame
> > >
> >
> > Maybe name this extra_amv_samples instead. And can you submit doxygen
> > comments for adpcm_compress_trellis() ?
> >
> 
> Variable is renamed,  trellis implemented.
> Patch in attachement.
[...]
> 
> --- ffmpeg.orig/libavcodec/adpcm.c	2007-10-04 22:21:14.000000000 +0700
> +++ ffmpeg/libavcodec/adpcm.c	2007-10-11 22:47:03.000000000 +0700
> @@ -146,6 +146,8 @@
>      int channel; /* for stereo MOVs, decode left, then decode right, then tell it's decoded */
>      ADPCMChannelStatus status[2];
>      short sample_buffer[32]; /* hold left samples while waiting for right samples */
> +    int extra_amv_samples; //number of samples to put into next frame

not doxygen compatible


> +    int samples_written;
>  } ADPCMContext;
>  
>  /* XXX: implement encoding */
> @@ -185,6 +187,16 @@
>          }
>          avctx->frame_size = 512 * (avctx->sample_rate / 11025);
>          break;
> +    case CODEC_ID_ADPCM_IMA_AMV:
> +        if (avctx->channels != 1){
> +            av_log(avctx, AV_LOG_ERROR, "Only mono sound is supported\n");
> +            return -1;
> +        }
> +        if (avctx->sample_rate != 22050){
> +            av_log(avctx, AV_LOG_ERROR, "Only 22050 sample rate is supported\n");
> +            return -1;
> +        }
> +        break;        

trailing whitespace


[...]
> +        if(!c->samples_written)
> +        c->status[0].prev_sample=*samples;

this is not sanely indented

also if this patch depends on some hack to the avi muxer then its rejected
of course as it would make no sense to apply without that hack

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you really think that XML is the answer, then you definitly missunderstood
the question -- Attila Kinali
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20071012/d6d3568a/attachment.pgp>