[MPlayer-dev-eng] [PATCH] Replace deprecated avcodec resamples with libswresample in af_lavcresample

[Michael Niedermayer]

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


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


> +        swr_init(s->swrctx);

missing failure check


>          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


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


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

What does censorship reveal? It reveals fear. -- Julian Assange
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20150812/036ce097/attachment.sig>