Re: [FFmpeg-devel] [PATCH] libswresample: avoid s16p internal processing format

[Michael Niedermayer <michael@niedermayer.cc>]

[-- Attachment #1.1: Type: text/plain, Size: 4920 bytes --]

On Sun, Jan 08, 2023 at 04:18:44PM +0100, Paul B Mahol wrote:
> On 1/8/23, Michael Niedermayer <michael@niedermayer.cc> wrote:
> > On Fri, Jan 06, 2023 at 07:01:06PM +0100, Paul B Mahol wrote:
> >> On Fri, Jan 6, 2023 at 6:25 PM Michael Niedermayer
> >> <michael@niedermayer.cc>
> >> wrote:
> >>
> >> > On Thu, Jan 05, 2023 at 11:08:25PM +0100, Paul B Mahol wrote:
> >> > > On Thu, Jan 5, 2023 at 9:53 PM Michael Niedermayer <
> >> > michael@niedermayer.cc>
> >> > > wrote:
> >> > >
> >> > > > On Thu, Jan 05, 2023 at 01:44:10PM +0100, Paul B Mahol wrote:
> >> > > > > Patch attached.
> >> > > >
> >> > > > >  swresample.c |    3 ++-
> >> > > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> >> > > > > eee7a0685b44aa867562138a2e2437ecb8844612
> >> > > > 0001-libswresample-swresample-avoid-s16p-internal-transfe.patch
> >> > > > > From 9c4cd60e2dd41cf98d693c8251f4cfade0807073 Mon Sep 17 00:00:00
> >> > 2001
> >> > > > > From: Paul B Mahol <onemda@gmail.com>
> >> > > > > Date: Thu, 5 Jan 2023 13:40:12 +0100
> >> > > > > Subject: [PATCH] libswresample/swresample: avoid s16p internal
> >> > transfer
> >> > > > format
> >> > > > >
> >> > > > > Instead use float one by default for sample rate conversions.
> >> > > > > The s16p internal transfer format produces visible and hearable
> >> > > > > quantization artifacts.
> >> > > >
> >> > > > When does this occur and why?
> >> > > >
> >> > >
> >> > > It occurs always. Just compare output with 16bit and
> >> > > int32/float/double.
> >> > > Look at other people report on internet.
> >> > > Look at src.infinitewave.ca
> >> >
> >> > src.infinitewave.ca uses 32bit none of what it shows should touch the
> >> > codepath
> >> > you change.
> >> >
> >> > if we look at src.infinitewave.ca for swr we see 2 types of artifacts
> >> > 1. Aliassing which is at maybe -120db with the actual signal at 0db
> >> >    i would like to see some evidence that a human can hear this
> >> >
> >>
> >> For s16p<->s16p it is much lower, around -78dB thus this patch.
> >
> > ok but you pointed to the website that apparently uses >=32bit if i trust
> > what they write.
> > And even if they test this i cannot use that website to replicate the issue
> > and the fix
> 
> If one use pure 16bit processing sweep results are even worse.
> 
> Just resample using fltp/dblp/s32p and s16p and compare (it does not
> matter what, just not simple sine constant frequencies waves)
> 
> The s16p result is much worse and contains huge quantization noises.
> 
> They are not that obviously easy to hear, but are there, and
> difference is > -80dB for dithered 16bit input.
> 
> You can generate and display sweep with/out resampling all with
> ffmpeg/ffplay.ffplay -f lavfi -i
> aevalsrc="sin(PI*t*t*t*100):s=96000",aresample=44100:tsf=s16p,showspectrum=scale=log:color=cool:overlap=0:fps=25:drange=96:legend=1
> 
> Play with tsf=values and drange=[96-150]
> So, for 16bit input, drange=96 and tsf=s16p looks fine, but that web
> pages shows bad results,

ok, i see the issue, thanks


[...]
> >> How so?
> >> I really doubt that this patch do that.
> >
> > I could be missing something but
> > int_sample_fmt is set to before 16bit and afterwards 32bit
> > and alot of things are using this:
> >     set_audiodata_fmt(&s->postin, s->int_sample_fmt);
> >     set_audiodata_fmt(&s->midbuf, s->int_sample_fmt);
> >     set_audiodata_fmt(&s->preout, s->int_sample_fmt);
> >
> > rematrix seems using these
> >             swri_rematrix(s, preout, midbuf, out_count, preout==out);
> > ...
> >             swri_rematrix(s, midbuf, postin, in_count, midbuf==out);
> >
> > so i assumed that this patch makes a difference for it. Again i could be
> > missing
> > something
> 
> Yes, but only if sample rates differ the fltp is used instead of s16p
> for resampling.

yes, i misread the patch

so, i do agree that flt is a better default for resampling s16->s16
on modern hw using 16bit coeffs seems an optimization at the wrong place
the patch breaks fate though

also, i suggest to leave the s16p if either input or output is s8

teh default change is ok, if it doesnt cause bitexactness issues,
otherwise it also needs to be done conditional on bitexact flag

As a note, if someone is interrested, it should be possible to improve
the quantization noise by using some more fancy noise shaping when building
the coefficients. That would not reduce the quantization noise but it would
spread it out so it would be less audible and vissible. Though thats only
really usefull if one actually did hear it. 

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Whats the most studid thing your enemy could do ? Blow himself up
Whats the most studid thing you could do ? Give up your rights and
freedom because your enemy blew himself up.


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".