From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 0EF9D49A6E for ; Mon, 26 Feb 2024 22:19:50 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 0E52968C900; Tue, 27 Feb 2024 00:19:48 +0200 (EET) Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [217.70.183.197]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 6506168C3CE for ; Tue, 27 Feb 2024 00:19:42 +0200 (EET) Received: by mail.gandi.net (Postfix) with ESMTPSA id 952E91C0002 for ; Mon, 26 Feb 2024 22:19:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=niedermayer.cc; s=gm1; t=1708985981; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=2Plj5j4E7UXfYXWCAipiY5W1D8+WJFPzfgU+eCgqX6U=; b=NTvKtrnkrXIXnb0MBRjLTHxUsLfzrhV14PfLJ9akdFhLfv/KURTsHnkHppHTSjUARqKFFu ArMiaEwrbS9WsDsEVctqDdURfTt6xk8HPcIIHyAokC7bTtvpA1PyncPoLj5gs3AFrZj7TK Ht5WcB1K8RalU7hk2AoxR9mzZEKz+ZPkRy9w8SoweXJyFvO+tRs+bxHC7LGQiYmvgWPwWi FAPw1Jh+dFlz1evzYpEaveze/rj9bkYAylpLraPkrllbnuaMUkr0yTug/d1y2nIPx9Ke7m QiQDxd6bEqDY/VJOz/S28ox0P9+i5gnHYa29cKNQkxYLRbjr2O0c/XpOmDmdAQ== Date: Mon, 26 Feb 2024 23:19:40 +0100 From: Michael Niedermayer To: FFmpeg development discussions and patches Message-ID: <20240226221940.GV6420@pb2> References: <20240226004525.4321-1-cus@passwd.hu> <20240226004525.4321-2-cus@passwd.hu> MIME-Version: 1.0 In-Reply-To: <20240226004525.4321-2-cus@passwd.hu> X-GND-Sasl: michael@niedermayer.cc Subject: Re: [FFmpeg-devel] [PATCH 2/4] swresample/resample: rework resample_one function to work the same way as the others X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Type: multipart/mixed; boundary="===============0043009719298230205==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============0043009719298230205== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="/M1/ytuSKMDW0aUs" Content-Disposition: inline --/M1/ytuSKMDW0aUs Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Feb 26, 2024 at 01:45:23AM +0100, Marton Balint wrote: > This also fixes resampling with filter_size=3D1 and phase_shift=3D0, depe= nding on > input chunk size noticable corrpution was hearable with this command line: >=20 > ffplay -f lavfi -i "sine=3D440:r=3D8000:samples_per_frame=3D32,aresample= =3D24000:filter_size=3D1:phase_shift=3D0" >=20 > Signed-off-by: Marton Balint > --- > 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(-) >=20 > 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, Aud= ioData *dst, int dst_size, A > =20 > *consumed =3D 0; > =20 > - if (c->filter_length =3D=3D 1 && c->phase_count =3D=3D 1) { > - int64_t index2=3D (1LL<<32)*c->frac/c->src_incr + (1LL<<32)*c->i= ndex; > - int64_t incr=3D (1LL<<32) * c->dst_incr / c->src_incr; > - int new_size =3D (src_size * (int64_t)c->src_incr - c->frac + c-= >dst_incr - 1) / c->dst_incr; > - > - dst_size =3D FFMAX(FFMIN(dst_size, new_size), 0); > - if (dst_size > 0) { > - for (i =3D 0; i < dst->ch_count; i++) { > - c->dsp.resample_one(dst->ch[i], src->ch[i], dst_size, in= dex2, incr); > - if (i+1 =3D=3D dst->ch_count) { > - c->index +=3D dst_size * c->dst_incr_div; > - c->index +=3D (c->frac + dst_size * (int64_t)c->dst_= incr_mod) / c->src_incr; > - av_assert2(c->index >=3D 0); > - *consumed =3D c->index; > - c->frac =3D (c->frac + dst_size * (int64_t)c->dst_= incr_mod) % c->src_incr; > - c->index =3D 0; > - } > - } > - } > - } else { > + { > int64_t end_index =3D (1LL + src_size - c->filter_length) * c->p= hase_count; > int64_t delta_frac =3D (end_index - c->index) * c->src_incr - c-= >frac; > int delta_n =3D (delta_frac + c->dst_incr - 1) / c->dst_incr; > @@ -386,8 +367,12 @@ static int multiple_resample(ResampleContext *c, Aud= ioData *dst, int dst_size, A > if (dst_size > 0) { > /* resample_linear and resample_common should have same beha= vior > * when frac and dst_incr_mod are zero */ > - resample_func =3D (c->linear && (c->frac || c->dst_incr_mod)= ) ? > - c->dsp.resample_linear : c->dsp.resample_com= mon; > + if (c->filter_length =3D=3D 1 && c->phase_count =3D=3D 1) > + resample_func =3D c->dsp.resample_one; > + else if (c->linear && (c->frac || c->dst_incr_mod)) > + resample_func =3D c->dsp.resample_linear; > + else > + resample_func =3D c->dsp.resample_common; > for (i =3D 0; i < dst->ch_count; i++) > *consumed =3D resample_func(c, dst->ch[i], src->ch[i], d= st_size, i+1 =3D=3D 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 compe= nsation is enabled */ > =20 > 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_t= emplate.c > index 4c227b9940..0c6e0ee34d 100644 > --- a/libswresample/resample_template.c > +++ b/libswresample/resample_template.c > @@ -72,17 +72,36 @@ > =20 > #endif > =20 > -static void RENAME(resample_one)(void *dest, const void *source, > - int dst_size, int64_t index2, int64_t i= ncr) > +static int RENAME(resample_one)(ResampleContext *c, > + void *dest, const void *source, > + int n, int update_ctx) > { > DELEM *dst =3D dest; > const DELEM *src =3D source; > int dst_index; > + int frac=3D c->frac; > + int sample_index =3D c->index; > + int index =3D 0; > + > + for (dst_index =3D 0; dst_index < n; dst_index++) { > + dst[dst_index] =3D src[sample_index]; > + > + frac +=3D c->dst_incr_mod; > + index +=3D c->dst_incr_div; > + if (frac >=3D c->src_incr) { > + frac -=3D c->src_incr; > + index++; > + } > + sample_index +=3D index; > + index =3D 0; > + } > =20 > - for (dst_index =3D 0; dst_index < dst_size; dst_index++) { > - dst[dst_index] =3D src[index2 >> 32]; > - index2 +=3D incr; > + if(update_ctx){ > + c->frac=3D frac; > + c->index=3D 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 [...] --=20 Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Everything should be made as simple as possible, but not simpler. -- Albert Einstein --/M1/ytuSKMDW0aUs Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EABEIAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCZd0OdgAKCRBhHseHBAsP q7OjAJwPBBeWMVg1YjqQnLAcLrDiduxqHwCffVAqE06v6KnDjV6JyGc7QLI/de0= =pmT4 -----END PGP SIGNATURE----- --/M1/ytuSKMDW0aUs-- --===============0043009719298230205== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". --===============0043009719298230205==--