[MPlayer-dev-eng] [PATCH] Replace deprecated avcodec resamples with libswresample in af_lavcresample
Roberto Togni
rxt at rtogni.it
Fri Aug 14 23:38:09 CEST 2015
On Wed, 12 Aug 2015 14:17:58 +0200
Michael Niedermayer <michael at niedermayer.cc> wrote:
> On Mon, Aug 10, 2015 at 11:29:57PM +0200, Roberto Togni wrote:
> > On Mon, 10 Aug 2015 12:18:45 +0200
> > wm4 <nfxjfg at googlemail.com> wrote:
> >
> > > On Mon, 10 Aug 2015 01:49:12 +0200
> > > Roberto Togni <rxt at rtogni.it> wrote:
> > >
> > > > Hello,
> > > > this patch updates af_lavcresample to use libswresample instead of the
> > > > deprecated avcodec resampler (resample2).
> > > >
> > > > It works for me; please test, it especially with multichannel setup.
> > >
> > > This shouldn't change anything about multichannel. Since the input and
> > > output layouts are the same, libswresample will not remix the channels.
> > >
> > > Also, you should use av_malloc to allocate aligned buffers for
> > > libswresample, and BUFFER_PADDING doesn't look like the correct
> > > constant to use.
> >
> > Thanks for the feedback.
> > New version attached, with aligned allocation and minimal buffer
> > padding.
> >
> > I'll apply shortly if nobody complains.
> >
> > Ciao,
> > Roberto
>
> > af_lavcresample.c | 99 ++++++++++++++++++++----------------------------------
> > 1 file changed, 38 insertions(+), 61 deletions(-)
> > 81dadefee29ae11f6c94cccc97969bb5f7eb13e4 af_lavcresample.c.diff
> > Index: af_lavcresample.c
> > ===================================================================
> > --- af_lavcresample.c (revisione 37444)
> > +++ af_lavcresample.c (copia locale)
> > @@ -25,15 +25,20 @@
> >
> > #include "config.h"
> > #include "af.h"
> > -#include "libavcodec/avcodec.h"
> > #include "libavutil/rational.h"
> > +#include "libswresample/swresample.h"
> > +#include "libavutil/channel_layout.h"
> > +#include "libavutil/opt.h"
> > +#include "libavutil/mem.h"
> > +#include "libavutil/common.h"
> >
> > // Data for specific instances of this filter
> > typedef struct af_resample_s{
> > - struct AVResampleContext *avrctx;
> > - int16_t *in[AF_NCH];
> > + struct SwrContext *swrctx;
> > + uint8_t *in[1];
> > + uint8_t *tmp[1];
> > int in_alloc;
> > - int index;
> > + int tmp_alloc;
> >
> > int filter_length;
> > int linear;
> > @@ -70,8 +75,19 @@
> >
> > if (s->ctx_out_rate != af->data->rate || s->ctx_in_rate != data->rate || s->ctx_filter_size != s->filter_length ||
> > s->ctx_phase_shift != s->phase_shift || s->ctx_linear != s->linear || s->ctx_cutoff != s->cutoff) {
> > - if(s->avrctx) av_resample_close(s->avrctx);
> > - s->avrctx= av_resample_init(af->data->rate, /*in_rate*/data->rate, s->filter_length, s->phase_shift, s->linear, s->cutoff);
>
> > + if(s->swrctx) swr_free(&s->swrctx);
>
> the if() should be unneeded
Removed.
Should this be added to the swr_free doxygen, or do all the ffmpeg
context freeing functions take NULL as a valid input?
>
>
> > + s->swrctx=swr_alloc();
>
> missing (malloc) failure check
>
> > + av_opt_set_int(s->swrctx, "out_sample_rate", af->data->rate, 0);
> > + av_opt_set_int(s->swrctx, "in_sample_rate", data->rate, 0);
> > + av_opt_set_int(s->swrctx, "filter_size", s->filter_length, 0);
> > + av_opt_set_int(s->swrctx, "phase_shift", s->phase_shift, 0);
> > + av_opt_set_int(s->swrctx, "linear_interp", s->linear, 0);
> > + av_opt_set_double(s->swrctx, "cutoff", s->cutoff, 0);
> > + av_opt_set_sample_fmt(s->swrctx, "in_sample_fmt", AV_SAMPLE_FMT_S16, 0);
> > + av_opt_set_sample_fmt(s->swrctx, "out_sample_fmt", AV_SAMPLE_FMT_S16, 0);
>
> > + av_opt_set_channel_layout(s->swrctx, "in_channel_layout", av_get_default_channel_layout(af->data->nch), 0);
> > + av_opt_set_channel_layout(s->swrctx, "out_channel_layout", av_get_default_channel_layout(af->data->nch), 0);
>
> Is there a reason why you dont set the number of channels instead of
> the layout ?
> as is, failure is possible for cases where there is no default
> layout
No reason; I just see that the swr_alloc_set_opts() wants a channel
layout, so I thought it was required.
Changed.
>
>
> > + swr_init(s->swrctx);
>
> missing failure check
Added.
>
>
> > s->ctx_out_rate = af->data->rate;
> > s->ctx_in_rate = data->rate;
> > s->ctx_filter_size = s->filter_length;
> > @@ -106,11 +122,10 @@
> > free(af->data->audio);
> > free(af->data);
> > if(af->setup){
> > - int i;
> > af_resample_t *s = af->setup;
> > - if(s->avrctx) av_resample_close(s->avrctx);
> > - for (i=0; i < AF_NCH; i++)
> > - free(s->in[i]);
>
> > + if(s->swrctx) swr_free(&s->swrctx);
>
> if() should be uneededd
Removed.
>
>
> > + av_free(s->in[0]);
> > + av_free(s->tmp[0]);
> > free(s);
> > }
> > }
> > @@ -119,71 +134,33 @@
> > static af_data_t* play(struct af_instance_s* af, af_data_t* data)
> > {
> > af_resample_t *s = af->setup;
> > - int i, j, consumed, ret;
> > - int16_t *in = (int16_t*)data->audio;
> > - int16_t *out;
> > + int ret;
> > + int8_t *in = (int8_t*)data->audio;
> > + int8_t *out;
> > int chans = data->nch;
> > - int in_len = data->len/(2*chans);
> > + int in_len = data->len;
> > int out_len = in_len * af->mul + 10;
> > - int16_t tmp[AF_NCH][out_len];
> >
> > if(AF_OK != RESIZE_LOCAL_BUFFER(af,data))
> > return NULL;
> >
> > - out= (int16_t*)af->data->audio;
> > + av_fast_malloc(&s->tmp[0], &s->tmp_alloc, FFALIGN(out_len,32));
> >
> > - out_len= FFMIN(out_len, af->data->len/(2*chans));
> > + out= (int8_t*)af->data->audio;
> >
> > - if(s->in_alloc < in_len + s->index){
> > - s->in_alloc= in_len + s->index;
> > - for(i=0; i<chans; i++){
> > - s->in[i]= realloc(s->in[i], s->in_alloc*sizeof(int16_t));
> > - }
> > - }
> > + out_len= FFMIN(out_len, af->data->len);
> >
> > - if(chans==1){
> > - memcpy(&s->in[0][s->index], in, in_len * sizeof(int16_t));
> > - }else if(chans==2){
> > - for(j=0; j<in_len; j++){
> > - s->in[0][j + s->index]= *(in++);
> > - s->in[1][j + s->index]= *(in++);
> > - }
> > - }else{
> > - for(j=0; j<in_len; j++){
> > - for(i=0; i<chans; i++){
> > - s->in[i][j + s->index]= *(in++);
> > - }
> > - }
> > - }
> > - in_len += s->index;
> > + av_fast_malloc(&s->in[0], &s->in_alloc, FFALIGN(in_len,32));
> >
> > - for(i=0; i<chans; i++){
> > - ret= av_resample(s->avrctx, tmp[i], s->in[i], &consumed, in_len, out_len, i+1 == chans);
> > - }
> > - out_len= ret;
> > + memcpy(s->in[0], in, in_len);
> >
> > - s->index= in_len - consumed;
> > - for(i=0; i<chans; i++){
> > - memmove(s->in[i], s->in[i] + consumed, s->index*sizeof(int16_t));
> > - }
>
> > + ret = swr_convert(s->swrctx, &s->tmp[0], out_len/chans/2, &s->in[0], in_len/chans/2);
> > + out_len= ret*chans*2;
>
> this is missing an error check
> failure is possible due to *malloc() failure at least
Added.
Since there was no failure check (malloc or other) in the old filter, I
didn't care about them.
New version attached.
Ciao,
Roberto
-------------- next part --------------
A non-text attachment was scrubbed...
Name: af_lavcresample.c.diff
Type: text/x-patch
Size: 5091 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20150814/ad968478/attachment-0001.bin>
More information about the MPlayer-dev-eng
mailing list