[FFmpeg-devel] [PATCH 2/4] swresample/resample: rework resample_one function to work the same way as the others
Michael Niedermayer
michael at niedermayer.cc
Tue Feb 27 00:19:40 EET 2024
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 at 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
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Everything should be made as simple as possible, but not simpler.
-- Albert Einstein
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20240226/7f9d265b/attachment.sig>
More information about the ffmpeg-devel
mailing list