From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <ffmpeg-devel-bounces@ffmpeg.org>
Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100])
by master.gitmailbox.com (Postfix) with ESMTP id 0B32F48F49
for <ffmpegdev@gitmailbox.com>; Fri, 1 Mar 2024 03:30:29 +0000 (UTC)
Received: from [127.0.1.1] (localhost [127.0.0.1])
by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 38F6068D25C;
Fri, 1 Mar 2024 05:30:26 +0200 (EET)
Received: from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net
[217.70.183.198])
by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id F3EAC68D13E
for <ffmpeg-devel@ffmpeg.org>; Fri, 1 Mar 2024 05:30:18 +0200 (EET)
Received: by mail.gandi.net (Postfix) with ESMTPSA id A8B54C0004
for <ffmpeg-devel@ffmpeg.org>; Fri, 1 Mar 2024 03:30:17 +0000 (UTC)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=niedermayer.cc;
s=gm1; t=1709263818;
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=QcEu9UcK2zKiXxVBLpZ2AnALl3twY1B8C0h5yzGK344=;
b=pKV7NocPQDUNWHfLF9nWQfXYT9+obplqD8jepD3aT1qUS+sWsaOA7NjHRlzaKsw8ivWUzZ
iCF3HHR0gVCoozXDnO10moJ1jwAhSJHoKsXIReEK7HNyYs7GMw11EuZsDjSmvQDxmI37Iu
YIyO/Frezp0txggzE4ePwmgPvvvFmFg3ZmEm/CNweHCwLFQ9fESBENWFCk2VnXj9KtLHvc
g7cFvnUE/htIrQrtYQGVZGOTwGhiaYt/k54GsFUUji7D3dekULrE2NJtEITCTGXUonIg8z
M9svJyxhynp6lWSq2JgRSXTqu1EKoPIuKtJmYyvQ6m2Gz/E9VyxZoF7UsgnWOA==
Date: Fri, 1 Mar 2024 04:30:16 +0100
From: Michael Niedermayer <michael@niedermayer.cc>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Message-ID: <20240301033016.GH6420@pb2>
References: <20240226221940.GV6420@pb2> <20240227094810.1182-1-cus@passwd.hu>
<20240227094810.1182-2-cus@passwd.hu> <20240229122205.GD6420@pb2>
<a46c7130-73f2-ed99-cdde-ea50f7f4fab5@passwd.hu>
MIME-Version: 1.0
In-Reply-To: <a46c7130-73f2-ed99-cdde-ea50f7f4fab5@passwd.hu>
X-GND-Sasl: michael@niedermayer.cc
Subject: Re: [FFmpeg-devel] [PATCH 2/2] 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 <ffmpeg-devel.ffmpeg.org>
List-Unsubscribe: <https://ffmpeg.org/mailman/options/ffmpeg-devel>,
<mailto:ffmpeg-devel-request@ffmpeg.org?subject=unsubscribe>
List-Archive: <https://ffmpeg.org/pipermail/ffmpeg-devel>
List-Post: <mailto:ffmpeg-devel@ffmpeg.org>
List-Help: <mailto:ffmpeg-devel-request@ffmpeg.org?subject=help>
List-Subscribe: <https://ffmpeg.org/mailman/listinfo/ffmpeg-devel>,
<mailto:ffmpeg-devel-request@ffmpeg.org?subject=subscribe>
Reply-To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Content-Type: multipart/mixed; boundary="===============5213282158250621908=="
Errors-To: ffmpeg-devel-bounces@ffmpeg.org
Sender: "ffmpeg-devel" <ffmpeg-devel-bounces@ffmpeg.org>
Archived-At: <https://master.gitmailbox.com/ffmpegdev/20240301033016.GH6420@pb2/>
List-Archive: <https://master.gitmailbox.com/ffmpegdev/>
List-Post: <mailto:ffmpegdev@gitmailbox.com>
--===============5213282158250621908==
Content-Type: multipart/signed; micalg=pgp-sha256;
protocol="application/pgp-signature"; boundary="dDbcrhea8joeNwVv"
Content-Disposition: inline
--dDbcrhea8joeNwVv
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Thu, Feb 29, 2024 at 06:55:01PM +0100, Marton Balint wrote:
>=20
>=20
> On Thu, 29 Feb 2024, Michael Niedermayer wrote:
>=20
> > On Tue, Feb 27, 2024 at 10:48:10AM +0100, Marton Balint wrote:
> > > Signed-off-by: Marton Balint <cus@passwd.hu>
> > > ---
> > > libswresample/resample.c | 29 +++++++----------------------
> > > libswresample/resample.h | 4 ++--
> > > libswresample/resample_template.c | 14 ++++++++++++--
> > > 3 files changed, 21 insertions(+), 26 deletions(-)
> > >=20
> > > diff --git a/libswresample/resample.c b/libswresample/resample.c
> > > index 17cebad01b..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
> > >=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->index + 1;
> > > - int64_t incr=3D (1LL<<32) * c->dst_incr / c->src_incr + 1;
> > > - 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=
, index2, 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->phase_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,=
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 =3D (c->linear && (c->frac || c->dst_incr_=
mod)) ?
> > > - c->dsp.resample_linear : c->dsp.resample=
_common;
> > > + 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=
], dst_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 c=
ompensation 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_ct=
x);
> > > int (*resample_linear)(struct ResampleContext *c, void *dst,
> > > diff --git a/libswresample/resample_template.c b/libswresample/resamp=
le_template.c
> > > index 4c227b9940..a8114ea918 100644
> > > --- a/libswresample/resample_template.c
> > > +++ b/libswresample/resample_template.c
> > > @@ -72,17 +72,27 @@
> > >=20
> > > #endif
> > >=20
> > > -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 dst_size, int update_ctx)
> > > {
> > > DELEM *dst =3D dest;
> > > const DELEM *src =3D source;
> > > int dst_index;
> >=20
> > > + int64_t index2 =3D (1LL << 32) * c->frac / c->src_incr + 1 +=
(1LL << 32) * c->index;
> > > + int64_t incr =3D (1LL << 32) * c->dst_incr / c->src_incr + 1;
> >=20
> > This computation is done repeatedly for each channel, thats not needed
> > its enough if its done once
>=20
> I consider that negligable for real cases, and it makes the code cleaner
> doing the computations here.
It would make asm optimized functions more difficult to implement too while
being less efficient.
>=20
> If you insist on this, then it is better to rework all the resample funcs=
to
> work on all channels in a separate patch.
That would be work as IIRC there are asm optimiezd versions
of these. Also iam not sure about the complexity overall decreasing with
such change
either way iam not "insisting" on anything, i just want the code to be
simple, clean and fast. I think the original code was close to achieving
this, it just was buggy.
Really iam happy about any ovarall improvment you want to do.
But this started out as a bugfix, and in that context seeing a non
bugfix change looks wrong to me
If you want to simplify or otherwise improve swr, iam not in your way
but please make sure the code really does improve and doesnt just
look better to you.
For example when its obvious a change would affect cases with small
blocks and many channels the worst, the speed of such case should be
tested. Not just a mono case
thx
[...]
--=20
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire
--dDbcrhea8joeNwVv
Content-Type: application/pgp-signature; name="signature.asc"
-----BEGIN PGP SIGNATURE-----
iF0EABEIAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCZeFLxAAKCRBhHseHBAsP
q5XfAJ9wbERlHGHQUgILewK/NMBCQQO0ngCfXf0cdnPEeUxdseUIVXOwLzYdPlQ=
=ImKs
-----END PGP SIGNATURE-----
--dDbcrhea8joeNwVv--
--===============5213282158250621908==
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".
--===============5213282158250621908==--