| Message ID | 20240226004525.4321-2-cus@passwd.hu |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/4] fate: never generate auto-generated references | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On Mon, Feb 26, 2024 at 01:45:23AM +0100, Marton Balint wrote: > This also fixes resampling with filter_size=1 and phase_shift=0, depending on > input chunk size noticable corrpution was hearable with this command line: > > ffplay -f lavfi -i "sine=440:r=8000:samples_per_frame=32,aresample=24000:filter_size=1:phase_shift=0" > > Signed-off-by: Marton Balint <cus@passwd.hu> > --- > libswresample/resample.c | 29 +++++++---------------------- > libswresample/resample.h | 4 ++-- > libswresample/resample_template.c | 29 ++++++++++++++++++++++++----- > tests/fate/libswresample.mak | 4 ++-- > 4 files changed, 35 insertions(+), 31 deletions(-) > > diff --git a/libswresample/resample.c b/libswresample/resample.c > index bd54a7002f..89859dec79 100644 > --- a/libswresample/resample.c > +++ b/libswresample/resample.c > @@ -356,26 +356,7 @@ static int multiple_resample(ResampleContext *c, AudioData *dst, int dst_size, A > > *consumed = 0; > > - if (c->filter_length == 1 && c->phase_count == 1) { > - int64_t index2= (1LL<<32)*c->frac/c->src_incr + (1LL<<32)*c->index; > - int64_t incr= (1LL<<32) * c->dst_incr / c->src_incr; > - int new_size = (src_size * (int64_t)c->src_incr - c->frac + c->dst_incr - 1) / c->dst_incr; > - > - dst_size = FFMAX(FFMIN(dst_size, new_size), 0); > - if (dst_size > 0) { > - for (i = 0; i < dst->ch_count; i++) { > - c->dsp.resample_one(dst->ch[i], src->ch[i], dst_size, index2, incr); > - if (i+1 == dst->ch_count) { > - c->index += dst_size * c->dst_incr_div; > - c->index += (c->frac + dst_size * (int64_t)c->dst_incr_mod) / c->src_incr; > - av_assert2(c->index >= 0); > - *consumed = c->index; > - c->frac = (c->frac + dst_size * (int64_t)c->dst_incr_mod) % c->src_incr; > - c->index = 0; > - } > - } > - } > - } else { > + { > int64_t end_index = (1LL + src_size - c->filter_length) * c->phase_count; > int64_t delta_frac = (end_index - c->index) * c->src_incr - c->frac; > int delta_n = (delta_frac + c->dst_incr - 1) / c->dst_incr; > @@ -386,8 +367,12 @@ static int multiple_resample(ResampleContext *c, AudioData *dst, int dst_size, A > if (dst_size > 0) { > /* resample_linear and resample_common should have same behavior > * when frac and dst_incr_mod are zero */ > - resample_func = (c->linear && (c->frac || c->dst_incr_mod)) ? > - c->dsp.resample_linear : c->dsp.resample_common; > + if (c->filter_length == 1 && c->phase_count == 1) > + resample_func = c->dsp.resample_one; > + else if (c->linear && (c->frac || c->dst_incr_mod)) > + resample_func = c->dsp.resample_linear; > + else > + resample_func = c->dsp.resample_common; > for (i = 0; i < dst->ch_count; i++) > *consumed = resample_func(c, dst->ch[i], src->ch[i], dst_size, i+1 == dst->ch_count); > } > diff --git a/libswresample/resample.h b/libswresample/resample.h > index 1731dad3cf..8cc29effe8 100644 > --- a/libswresample/resample.h > +++ b/libswresample/resample.h > @@ -51,8 +51,8 @@ typedef struct ResampleContext { > int phase_count_compensation; /* desired phase_count when compensation is enabled */ > > struct { > - void (*resample_one)(void *dst, const void *src, > - int n, int64_t index, int64_t incr); > + int (*resample_one)(struct ResampleContext *c, void *dst, > + const void *src, int n, int update_ctx); > int (*resample_common)(struct ResampleContext *c, void *dst, > const void *src, int n, int update_ctx); > int (*resample_linear)(struct ResampleContext *c, void *dst, > diff --git a/libswresample/resample_template.c b/libswresample/resample_template.c > index 4c227b9940..0c6e0ee34d 100644 > --- a/libswresample/resample_template.c > +++ b/libswresample/resample_template.c > @@ -72,17 +72,36 @@ > > #endif > > -static void RENAME(resample_one)(void *dest, const void *source, > - int dst_size, int64_t index2, int64_t incr) > +static int RENAME(resample_one)(ResampleContext *c, > + void *dest, const void *source, > + int n, int update_ctx) > { > DELEM *dst = dest; > const DELEM *src = source; > int dst_index; > + int frac= c->frac; > + int sample_index = c->index; > + int index = 0; > + > + for (dst_index = 0; dst_index < n; dst_index++) { > + dst[dst_index] = src[sample_index]; > + > + frac += c->dst_incr_mod; > + index += c->dst_incr_div; > + if (frac >= c->src_incr) { > + frac -= c->src_incr; > + index++; > + } > + sample_index += index; > + index = 0; > + } > > - for (dst_index = 0; dst_index < dst_size; dst_index++) { > - dst[dst_index] = src[index2 >> 32]; > - index2 += incr; > + if(update_ctx){ > + c->frac= frac; > + c->index= index; > } > + > + return sample_index; > } please correct me if iam wrong, but this looks like that resample_one() is used in some case where it doesnt produce correct results ? And to fix this your patch replaces the resample_one() optimization by unoptimized code I think what should be done is first understand why it produces wrong output and if thats some edge case that can be just handled better by teh common code or a new function. Or if some of the code related to resample_one() is buggy n which case that should be fixed. or if instead theres a issue in teh common code and you reuse resample_one there and remove its optimizations because of that new use case. I cannot tell ... thx [...]
diff --git a/libswresample/resample.c b/libswresample/resample.c index bd54a7002f..89859dec79 100644 --- a/libswresample/resample.c +++ b/libswresample/resample.c @@ -356,26 +356,7 @@ static int multiple_resample(ResampleContext *c, AudioData *dst, int dst_size, A *consumed = 0; - if (c->filter_length == 1 && c->phase_count == 1) { - int64_t index2= (1LL<<32)*c->frac/c->src_incr + (1LL<<32)*c->index; - int64_t incr= (1LL<<32) * c->dst_incr / c->src_incr; - int new_size = (src_size * (int64_t)c->src_incr - c->frac + c->dst_incr - 1) / c->dst_incr; - - dst_size = FFMAX(FFMIN(dst_size, new_size), 0); - if (dst_size > 0) { - for (i = 0; i < dst->ch_count; i++) { - c->dsp.resample_one(dst->ch[i], src->ch[i], dst_size, index2, incr); - if (i+1 == dst->ch_count) { - c->index += dst_size * c->dst_incr_div; - c->index += (c->frac + dst_size * (int64_t)c->dst_incr_mod) / c->src_incr; - av_assert2(c->index >= 0); - *consumed = c->index; - c->frac = (c->frac + dst_size * (int64_t)c->dst_incr_mod) % c->src_incr; - c->index = 0; - } - } - } - } else { + { int64_t end_index = (1LL + src_size - c->filter_length) * c->phase_count; int64_t delta_frac = (end_index - c->index) * c->src_incr - c->frac; int delta_n = (delta_frac + c->dst_incr - 1) / c->dst_incr; @@ -386,8 +367,12 @@ static int multiple_resample(ResampleContext *c, AudioData *dst, int dst_size, A if (dst_size > 0) { /* resample_linear and resample_common should have same behavior * when frac and dst_incr_mod are zero */ - resample_func = (c->linear && (c->frac || c->dst_incr_mod)) ? - c->dsp.resample_linear : c->dsp.resample_common; + if (c->filter_length == 1 && c->phase_count == 1) + resample_func = c->dsp.resample_one; + else if (c->linear && (c->frac || c->dst_incr_mod)) + resample_func = c->dsp.resample_linear; + else + resample_func = c->dsp.resample_common; for (i = 0; i < dst->ch_count; i++) *consumed = resample_func(c, dst->ch[i], src->ch[i], dst_size, i+1 == dst->ch_count); } diff --git a/libswresample/resample.h b/libswresample/resample.h index 1731dad3cf..8cc29effe8 100644 --- a/libswresample/resample.h +++ b/libswresample/resample.h @@ -51,8 +51,8 @@ typedef struct ResampleContext { int phase_count_compensation; /* desired phase_count when compensation is enabled */ struct { - void (*resample_one)(void *dst, const void *src, - int n, int64_t index, int64_t incr); + int (*resample_one)(struct ResampleContext *c, void *dst, + const void *src, int n, int update_ctx); int (*resample_common)(struct ResampleContext *c, void *dst, const void *src, int n, int update_ctx); int (*resample_linear)(struct ResampleContext *c, void *dst, diff --git a/libswresample/resample_template.c b/libswresample/resample_template.c index 4c227b9940..0c6e0ee34d 100644 --- a/libswresample/resample_template.c +++ b/libswresample/resample_template.c @@ -72,17 +72,36 @@ #endif -static void RENAME(resample_one)(void *dest, const void *source, - int dst_size, int64_t index2, int64_t incr) +static int RENAME(resample_one)(ResampleContext *c, + void *dest, const void *source, + int n, int update_ctx) { DELEM *dst = dest; const DELEM *src = source; int dst_index; + int frac= c->frac; + int sample_index = c->index; + int index = 0; + + for (dst_index = 0; dst_index < n; dst_index++) { + dst[dst_index] = src[sample_index]; + + frac += c->dst_incr_mod; + index += c->dst_incr_div; + if (frac >= c->src_incr) { + frac -= c->src_incr; + index++; + } + sample_index += index; + index = 0; + } - for (dst_index = 0; dst_index < dst_size; dst_index++) { - dst[dst_index] = src[index2 >> 32]; - index2 += incr; + if(update_ctx){ + c->frac= frac; + c->index= index; } + + return sample_index; } static int RENAME(resample_common)(ResampleContext *c, diff --git a/tests/fate/libswresample.mak b/tests/fate/libswresample.mak index 2fc55ea61a..0971a6b3ba 100644 --- a/tests/fate/libswresample.mak +++ b/tests/fate/libswresample.mak @@ -347,13 +347,13 @@ endef fate-swr-resample_nn-fltp-44100-8000: CMP_TARGET = 591.03 fate-swr-resample_nn-fltp-44100-8000: SIZE_TOLERANCE = 529200 - 20486 -fate-swr-resample_nn-fltp-8000-44100: CMP_TARGET = 3163.32 +fate-swr-resample_nn-fltp-8000-44100: CMP_TARGET = 3156.61 fate-swr-resample_nn-fltp-8000-44100: SIZE_TOLERANCE = 96000 - 20480 fate-swr-resample_nn-s16p-44100-8000: CMP_TARGET = 590.98 fate-swr-resample_nn-s16p-44100-8000: SIZE_TOLERANCE = 529200 - 20486 -fate-swr-resample_nn-s16p-8000-44100: CMP_TARGET = 3163.39 +fate-swr-resample_nn-s16p-8000-44100: CMP_TARGET = 3156.61 fate-swr-resample_nn-s16p-8000-44100: SIZE_TOLERANCE = 96000 - 20480 define ARESAMPLE_ASYNC
This also fixes resampling with filter_size=1 and phase_shift=0, depending on input chunk size noticable corrpution was hearable with this command line: ffplay -f lavfi -i "sine=440:r=8000:samples_per_frame=32,aresample=24000:filter_size=1:phase_shift=0" Signed-off-by: Marton Balint <cus@passwd.hu> --- libswresample/resample.c | 29 +++++++---------------------- libswresample/resample.h | 4 ++-- libswresample/resample_template.c | 29 ++++++++++++++++++++++++----- tests/fate/libswresample.mak | 4 ++-- 4 files changed, 35 insertions(+), 31 deletions(-)