[PATCH] Replace deprecated avcodec resamples with libswresample in af_lavcresample
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. Ciao, Roberto
On Mon, 10 Aug 2015 01:49:12 +0200 Roberto Togni <rxt@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.
On Mon, 10 Aug 2015 12:18:45 +0200 wm4 <nfxjfg@googlemail.com> wrote:
On Mon, 10 Aug 2015 01:49:12 +0200 Roberto Togni <rxt@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
On Mon, Aug 10, 2015 at 11:29:57PM +0200, Roberto Togni wrote:
On Mon, 10 Aug 2015 12:18:45 +0200 wm4 <nfxjfg@googlemail.com> wrote:
On Mon, 10 Aug 2015 01:49:12 +0200 Roberto Togni <rxt@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
On Wed, 12 Aug 2015 14:17:58 +0200 Michael Niedermayer <michael@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@googlemail.com> wrote:
On Mon, 10 Aug 2015 01:49:12 +0200 Roberto Togni <rxt@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
On 14.08.2015 23:38, Roberto Togni wrote:
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"
Since this uses libswresample now, I think configure has to be changed to also check for that. Best regards, Andreas
Andreas Cadhalpun wrote on Sat, 15 Aug 2015 17:29:10 +0200:
On 14.08.2015 23:38, Roberto Togni wrote:
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"
Since this uses libswresample now, I think configure has to be changed to also check for that.
Yes, hopefully fixed in r37463. Ingo
On Fri, 14 Aug 2015 23:38:09 +0200 Roberto Togni <rxt@rtogni.it> wrote:
On Wed, 12 Aug 2015 14:17:58 +0200 Michael Niedermayer <michael@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@googlemail.com> wrote:
On Mon, 10 Aug 2015 01:49:12 +0200 Roberto Togni <rxt@rtogni.it> wrote:
Hello, this patch updates af_lavcresample to use libswresample instead of the deprecated avcodec resampler (resample2).
[...] Applied.
Ciao, Roberto
On Mon, 10 Aug 2015 01:49:12 +0200 Roberto Togni <rxt@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.
Ciao, Roberto
By the way, please have a look at the mplayer2 history to avoid duplicated work (and maybe mpv, though that takes a much more aggressive approach). I'm not sure how much API usage mplayer2 actually cleans up from a today's perspective, but at least the change to lib*resample was already done.
participants (5)
-
Andreas Cadhalpun -
Ingo Brückl -
Michael Niedermayer -
Roberto Togni -
wm4