[FFmpeg-devel] [ffmpeg-devel] [PATCH 4/4] lavfi: add audio resample filter

[Stefano Sabatini]

On date Monday 2011-08-01 12:34:10 +0300, Mina Nagy Zaki encoded:
> On Wed, Jul 27, 2011 at 12:53:01PM +0200, Stefano Sabatini wrote:
> > On date Wednesday 2011-07-27 01:05:20 +0300, Mina Nagy Zaki encoded:
> [...]
> > > +        for (i = 0; i < nb_channels; i++) {
> > > +            resample->cached_data[i]    =
> > > +                av_realloc(resample->cached_data[i], cached_nb_samples * sizeof(int16_t));
> > 
> > > +            resample->resampled_data[i] =
> > > +                av_realloc(resample->resampled_data[i], 4 * requested_out_nb_samples + 16);
> > 
> > More readable
> > 4 * requested_out_nb_samples + 16 =>
> > 
> > requested_out_nb_samples * sizeof(int16_t) * 2 + 16
> > 
> > 16 may be replaced by a macro ALIGN,
> 
> Can you elaborate?

#define ALIGN 16

or simply add a note: // 16 is for alignment

> 
> > *2 is from the libavcodec resample code, and I can't comment on it,
> > what happens if you drop it?
> 
> Like I mentioned before, I don't quite see what the extra 2 factor is for.
> Removing it has no side effects (besides lower memory usage that is...)

So I'd say, just drop it.

> > > +        }
> > > +        if (resample->outsamplesref)
> > > +            avfilter_unref_buffer(resample->outsamplesref);
> > > +        resample->outsamplesref = avfilter_get_audio_buffer(outlink,
> > > +                                                            AV_PERM_WRITE | AV_PERM_REUSE2,
> > > +                                                            inlink->format,
> > > +                                                            requested_out_nb_samples,
> > > +                                                            insamplesref->audio->channel_layout,
> > > +                                                            insamplesref->audio->planar);
> > 

> > Possibly missing avfilter_copy_buffer_ref_props(outsamplesref,
> > insamplesref);
> 
> I don't think so, this is just a normal output buffer request, and besides
> everything has already been done in get_audio_buffer.

What happens of the pts/pos info? What's the interpretation we
should give to PTS when filtering audio?

I suppose the natural interpretation is:

number of units expressed in 1/sample_rate unit.

So you need to rescale according to the relation:
pts1 / sample_rate1 = pts2 / sample_rate2
 
> > 
> > > +        resample->outsamplesref->audio->sample_rate = outlink->sample_rate;
> > 
> > 
> > > +        resample->cached_nb_samples = cached_nb_samples;
> > 
> > This should be set in the previous block (when the buffers are realloced).
> 
> Moved.
> 
> [...]
> > Missing docs.
> 
> Added simple doc.
> 

> From 39b6f0e5a16d1662980f694690e954b5f10b1d05 Mon Sep 17 00:00:00 2001
> From: Stefano Sabatini <stefano.sabatini-lala at poste.it>
> Date: Sun, 13 Feb 2011 18:00:41 +0100
> Subject: [PATCH 05/11] lavfi: add audio resample filter
> 
> ---
>  doc/filters.texi           |   11 ++
>  libavfilter/Makefile       |    2 +
>  libavfilter/af_aresample.c |  355 ++++++++++++++++++++++++++++++++++++++++++++
>  libavfilter/allfilters.c   |    1 +
>  4 files changed, 369 insertions(+), 0 deletions(-)
>  create mode 100644 libavfilter/af_aresample.c
> 
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 5538848..24f9050 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -121,6 +121,17 @@ aformat=s16:mono\\,stereo:packed\\,planar
>  
>  Pass the audio source unchanged to the output.
>  
> + at section aresample
> +
> +Resample the input audio to the specified sample rate.
> +
> +The filter accepts exactly one required parameter, the sample rate.

Mention which is the behavior if the argument is not specified.

[...]

Looks fine otherwise.
-- 
FFmpeg = Fancy and Fabulous Murdering Picky Elegant Game