diff mbox series

[FFmpeg-devel] libswresample/swresample: avoid s16p internal transfer format

Message ID 20230923210210.3830-1-michael@niedermayer.cc
State New
Headers show
Series [FFmpeg-devel] libswresample/swresample: avoid s16p internal transfer format | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 fail Make fate failed

Commit Message

Michael Niedermayer Sept. 23, 2023, 9:02 p.m. UTC
From: Paul B Mahol <onemda@gmail.com>

Instead use float one by default for sample rate conversions.
The s16p internal transfer format produces visible and hearable
quantization artifacts.

Signed-off-by: Paul B Mahol <onemda@gmail.com>

for S8 we continue to use S16 as it should have enough precision
Fate is adjusted so bitexactness is maintained between mips/arm/x86
if more tests became bit-inexact on some platform, the same change
can be done to them

The use of higher precision and float intermediates inevitably
leads to more differences between platforms.

Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libswresample/swresample.c    |  8 +++++++-
 tests/fate/acodec.mak         |  8 ++++----
 tests/fate/lavf-container.mak | 16 ++++++++--------
 3 files changed, 19 insertions(+), 13 deletions(-)

Comments

Paul B Mahol Sept. 23, 2023, 9:32 p.m. UTC | #1
On Sat, Sep 23, 2023 at 11:02 PM Michael Niedermayer <michael@niedermayer.cc>
wrote:

> From: Paul B Mahol <onemda@gmail.com>
>
> Instead use float one by default for sample rate conversions.
> The s16p internal transfer format produces visible and hearable
> quantization artifacts.
>
> Signed-off-by: Paul B Mahol <onemda@gmail.com>
>
> for S8 we continue to use S16 as it should have enough precision
> Fate is adjusted so bitexactness is maintained between mips/arm/x86
> if more tests became bit-inexact on some platform, the same change
> can be done to them
>
> The use of higher precision and float intermediates inevitably
> leads to more differences between platforms.
>

Could add S64 precision path, but would be slower than float, probably.
LGTM.


>
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libswresample/swresample.c    |  8 +++++++-
>  tests/fate/acodec.mak         |  8 ++++----
>  tests/fate/lavf-container.mak | 16 ++++++++--------
>  3 files changed, 19 insertions(+), 13 deletions(-)
>
> diff --git a/libswresample/swresample.c b/libswresample/swresample.c
> index fb3d7bccbf1..f2a9b404743 100644
> --- a/libswresample/swresample.c
> +++ b/libswresample/swresample.c
> @@ -331,8 +331,14 @@ av_cold int swr_init(struct SwrContext *s){
>                   s->rematrix_custom;
>
>      if(s->int_sample_fmt == AV_SAMPLE_FMT_NONE){
> +        // 16bit or less to 16bit or less with the same sample rate
>          if(   av_get_bytes_per_sample(s-> in_sample_fmt) <= 2
> -           && av_get_bytes_per_sample(s->out_sample_fmt) <= 2){
> +           && av_get_bytes_per_sample(s->out_sample_fmt) <= 2
> +           && s->out_sample_rate==s->in_sample_rate) {
> +            s->int_sample_fmt= AV_SAMPLE_FMT_S16P;
> +        // 8 -> 8, 16->8, 8->16bit
> +        } else if(   av_get_bytes_per_sample(s-> in_sample_fmt)
> +                    +av_get_bytes_per_sample(s->out_sample_fmt) <= 3 ) {
>              s->int_sample_fmt= AV_SAMPLE_FMT_S16P;
>          }else if(   av_get_bytes_per_sample(s-> in_sample_fmt) <= 2
>             && !s->rematrix
> diff --git a/tests/fate/acodec.mak b/tests/fate/acodec.mak
> index b15b1dcc205..7b09e3bd63d 100644
> --- a/tests/fate/acodec.mak
> +++ b/tests/fate/acodec.mak
> @@ -156,14 +156,14 @@ fate-acodec-ra144: CMP_SHIFT = -320
>  FATE_ACODEC-$(call ENCDEC, ROQ_DPCM, ROQ, ARESAMPLE_FILTER) +=
> fate-acodec-roqaudio
>  fate-acodec-roqaudio: FMT = roq
>  fate-acodec-roqaudio: CODEC = roq_dpcm
> -fate-acodec-roqaudio: ENCOPTS = -ar 22050
> -fate-acodec-roqaudio: DECOPTS = -ar 44100
> +fate-acodec-roqaudio: ENCOPTS = -af aresample=22050:tsf=s16p
> +fate-acodec-roqaudio: DECOPTS = -af aresample=44100:tsf=s16p
>
>  FATE_ACODEC-$(call ENCDEC, S302M, MPEGTS, ARESAMPLE_FILTER) +=
> fate-acodec-s302m
>  fate-acodec-s302m: FMT = mpegts
>  fate-acodec-s302m: CODEC = s302m
> -fate-acodec-s302m: ENCOPTS = -ar 48000 -strict -2
> -fate-acodec-s302m: DECOPTS = -ar 44100
> +fate-acodec-s302m: ENCOPTS = -af aresample=48000:tsf=s16p -strict -2
> +fate-acodec-s302m: DECOPTS = -af aresample=44100:tsf=s16p
>
>  FATE_ACODEC-$(call ENCDEC, WAVPACK, WV, ARESAMPLE_FILTER) +=
> fate-acodec-wavpack
>  fate-acodec-wavpack: FMT = wv
> diff --git a/tests/fate/lavf-container.mak b/tests/fate/lavf-container.mak
> index d4a0ac7510f..69330ae3b6f 100644
> --- a/tests/fate/lavf-container.mak
> +++ b/tests/fate/lavf-container.mak
> @@ -39,13 +39,13 @@ $(FATE_LAVF_CONTAINER): $(AREF) $(VREF)
>  fate-lavf-asf: CMD = lavf_container "" "-c:a mp2 -ar 44100" "-r 25"
>  fate-lavf-avi fate-lavf-nut: CMD = lavf_container "" "-c:a mp2 -ar 44100
> -threads 1"
>  fate-lavf-dv:  CMD = lavf_container "-ar 48000 -channel_layout stereo"
> "-r 25 -s pal"
> -fate-lavf-dv_pal:  CMD = lavf_container_timecode_nodrop "-ar 48000 -r 25
> -s pal -ac 2 -f dv"
> -fate-lavf-dv_ntsc:  CMD = lavf_container_timecode_drop "-ar 48000
> -pix_fmt yuv411p -s ntsc -ac 2 -f dv"
> +fate-lavf-dv_pal:  CMD = lavf_container_timecode_nodrop "-af
> aresample=48000:tsf=s16p -r 25 -s pal -ac 2 -f dv"
> +fate-lavf-dv_ntsc:  CMD = lavf_container_timecode_drop "-af
> aresample=48000:tsf=s16p -pix_fmt yuv411p -s ntsc -ac 2 -f dv"
>  fate-lavf-flv fate-lavf-swf: CMD = lavf_container "" "-an"
>  fate-lavf-flm: CMD = lavf_container "" "-pix_fmt rgba"
> -fate-lavf-gxf: CMD = lavf_container "-ar 48000" "-r 25 -s pal -ac 1
> -threads 1"
> -fate-lavf-gxf_pal: CMD = lavf_container_timecode_nodrop "-ar 48000 -r 25
> -s pal -ac 1 -threads 1 -f gxf"
> -fate-lavf-gxf_ntsc: CMD = lavf_container_timecode_drop "-ar 48000 -s ntsc
> -ac 1 -threads 1 -f gxf"
> +fate-lavf-gxf: CMD = lavf_container "-af aresample=48000:tsf=s16p" "-r 25
> -s pal -ac 1 -threads 1"
> +fate-lavf-gxf_pal: CMD = lavf_container_timecode_nodrop "-af
> aresample=48000:tsf=s16p -r 25 -s pal -ac 1 -threads 1 -f gxf"
> +fate-lavf-gxf_ntsc: CMD = lavf_container_timecode_drop "-af
> aresample=48000:tsf=s16p -s ntsc -ac 1 -threads 1 -f gxf"
>  fate-lavf-ismv: CMD = lavf_container_timecode "-an -write_tmcd 1 -c:v
> mpeg4 -threads 1"
>  fate-lavf-mkv: CMD = lavf_container "" "-c:a mp2 -c:v mpeg4 -ar 44100
> -threads 1"
>  fate-lavf-mkv_attachment: CMD = lavf_container_attach "-c:a mp2 -c:v
> mpeg4 -threads 1 -f matroska"
> @@ -53,9 +53,9 @@ fate-lavf-mov: CMD = lavf_container_timecode "-movflags
> +faststart -c:a pcm_alaw
>  fate-lavf-mov_rtphint: CMD = lavf_container "" "-movflags +rtphint -c:a
> pcm_alaw -c:v mpeg4 -threads 1 -f mov"
>  fate-lavf-mp4: CMD = lavf_container_timecode "-c:v mpeg4 -an -threads 1"
>  fate-lavf-mpg: CMD = lavf_container_timecode "-ar 44100 -threads 1"
> -fate-lavf-mxf: CMD = lavf_container_timecode "-ar 48000 -bf 2 -threads 1"
> -fate-lavf-mxf_d10: CMD = lavf_container "-ar 48000 -ac 2" "-r 25 -vf
> scale=720:576,pad=720:608:0:32,setfield=tff -c:v mpeg2video -g 0 -flags
> +ildct+low_delay -dc 10 -non_linear_quant 1 -intra_vlc 1 -qscale 1 -ps 1
> -qmin 1 -rc_max_vbv_use 1 -rc_min_vbv_use 1 -pix_fmt yuv422p -minrate
> 30000k -maxrate 30000k -b 30000k -bufsize 1200000 -rc_init_occupancy
> 1200000 -qmax 12 -f mxf_d10"
> -fate-lavf-mxf_dv25: CMD = lavf_container "-ar 48000 -ac 2" "-r 25 -vf
> scale=720:576,setdar=4/3,setfield=bff -c:v dvvideo -pix_fmt yuv420p -b
> 25000k -f mxf"
> +fate-lavf-mxf: CMD = lavf_container_timecode "-af
> aresample=48000:tsf=s16p -bf 2 -threads 1"
> +fate-lavf-mxf_d10: CMD = lavf_container "-af aresample=48000:tsf=s16p -ac
> 2" "-r 25 -vf scale=720:576,pad=720:608:0:32,setfield=tff -c:v mpeg2video
> -g 0 -flags +ildct+low_delay -dc 10 -non_linear_quant 1 -intra_vlc 1
> -qscale 1 -ps 1 -qmin 1 -rc_max_vbv_use 1 -rc_min_vbv_use 1 -pix_fmt
> yuv422p -minrate 30000k -maxrate 30000k -b 30000k -bufsize 1200000
> -rc_init_occupancy 1200000 -qmax 12 -f mxf_d10"
> +fate-lavf-mxf_dv25: CMD = lavf_container "-af aresample=48000:tsf=s16p
> -ac 2" "-r 25 -vf scale=720:576,setdar=4/3,setfield=bff -c:v dvvideo
> -pix_fmt yuv420p -b 25000k -f mxf"
>  fate-lavf-mxf_dvcpro50: CMD = lavf_container "-ar 48000 -ac 2" "-r 25 -vf
> scale=720:576,setdar=16/9,setfield=bff -c:v dvvideo -pix_fmt yuv422p -b
> 50000k -f mxf"
>  fate-lavf-mxf_dvcpro100: CMD = lavf_container "-ar 48000 -ac 2" "-r 25
> -vf scale=1440:1080,setdar=16/9,setfield=bff -c:v dvvideo -pix_fmt yuv422p
> -b 100000k -f mxf"
>  fate-lavf-mxf_ffv1: CMD = lavf_container "-an" "-r 25 -vf
> scale=720:576,setdar=4/3 -c:v ffv1 -level 3 -pix_fmt yuv420p -f mxf"
> --
> 2.17.1
>
>
Michael Niedermayer Sept. 24, 2023, 6:48 p.m. UTC | #2
On Sat, Sep 23, 2023 at 11:32:28PM +0200, Paul B Mahol wrote:
> On Sat, Sep 23, 2023 at 11:02 PM Michael Niedermayer <michael@niedermayer.cc>
> wrote:
> 
> > From: Paul B Mahol <onemda@gmail.com>
> >
> > Instead use float one by default for sample rate conversions.
> > The s16p internal transfer format produces visible and hearable
> > quantization artifacts.
> >
> > Signed-off-by: Paul B Mahol <onemda@gmail.com>
> >
> > for S8 we continue to use S16 as it should have enough precision
> > Fate is adjusted so bitexactness is maintained between mips/arm/x86
> > if more tests became bit-inexact on some platform, the same change
> > can be done to them
> >
> > The use of higher precision and float intermediates inevitably
> > leads to more differences between platforms.
> >
> 
> Could add S64 precision path, but would be slower than float, probably.
> LGTM.

will apply later without 2 lines (bugs noticed on rerunning with distclean)

thx

[...]
diff mbox series

Patch

diff --git a/libswresample/swresample.c b/libswresample/swresample.c
index fb3d7bccbf1..f2a9b404743 100644
--- a/libswresample/swresample.c
+++ b/libswresample/swresample.c
@@ -331,8 +331,14 @@  av_cold int swr_init(struct SwrContext *s){
                  s->rematrix_custom;
 
     if(s->int_sample_fmt == AV_SAMPLE_FMT_NONE){
+        // 16bit or less to 16bit or less with the same sample rate
         if(   av_get_bytes_per_sample(s-> in_sample_fmt) <= 2
-           && av_get_bytes_per_sample(s->out_sample_fmt) <= 2){
+           && av_get_bytes_per_sample(s->out_sample_fmt) <= 2
+           && s->out_sample_rate==s->in_sample_rate) {
+            s->int_sample_fmt= AV_SAMPLE_FMT_S16P;
+        // 8 -> 8, 16->8, 8->16bit
+        } else if(   av_get_bytes_per_sample(s-> in_sample_fmt)
+                    +av_get_bytes_per_sample(s->out_sample_fmt) <= 3 ) {
             s->int_sample_fmt= AV_SAMPLE_FMT_S16P;
         }else if(   av_get_bytes_per_sample(s-> in_sample_fmt) <= 2
            && !s->rematrix
diff --git a/tests/fate/acodec.mak b/tests/fate/acodec.mak
index b15b1dcc205..7b09e3bd63d 100644
--- a/tests/fate/acodec.mak
+++ b/tests/fate/acodec.mak
@@ -156,14 +156,14 @@  fate-acodec-ra144: CMP_SHIFT = -320
 FATE_ACODEC-$(call ENCDEC, ROQ_DPCM, ROQ, ARESAMPLE_FILTER) += fate-acodec-roqaudio
 fate-acodec-roqaudio: FMT = roq
 fate-acodec-roqaudio: CODEC = roq_dpcm
-fate-acodec-roqaudio: ENCOPTS = -ar 22050
-fate-acodec-roqaudio: DECOPTS = -ar 44100
+fate-acodec-roqaudio: ENCOPTS = -af aresample=22050:tsf=s16p
+fate-acodec-roqaudio: DECOPTS = -af aresample=44100:tsf=s16p
 
 FATE_ACODEC-$(call ENCDEC, S302M, MPEGTS, ARESAMPLE_FILTER) += fate-acodec-s302m
 fate-acodec-s302m: FMT = mpegts
 fate-acodec-s302m: CODEC = s302m
-fate-acodec-s302m: ENCOPTS = -ar 48000 -strict -2
-fate-acodec-s302m: DECOPTS = -ar 44100
+fate-acodec-s302m: ENCOPTS = -af aresample=48000:tsf=s16p -strict -2
+fate-acodec-s302m: DECOPTS = -af aresample=44100:tsf=s16p
 
 FATE_ACODEC-$(call ENCDEC, WAVPACK, WV, ARESAMPLE_FILTER) += fate-acodec-wavpack
 fate-acodec-wavpack: FMT = wv
diff --git a/tests/fate/lavf-container.mak b/tests/fate/lavf-container.mak
index d4a0ac7510f..69330ae3b6f 100644
--- a/tests/fate/lavf-container.mak
+++ b/tests/fate/lavf-container.mak
@@ -39,13 +39,13 @@  $(FATE_LAVF_CONTAINER): $(AREF) $(VREF)
 fate-lavf-asf: CMD = lavf_container "" "-c:a mp2 -ar 44100" "-r 25"
 fate-lavf-avi fate-lavf-nut: CMD = lavf_container "" "-c:a mp2 -ar 44100 -threads 1"
 fate-lavf-dv:  CMD = lavf_container "-ar 48000 -channel_layout stereo" "-r 25 -s pal"
-fate-lavf-dv_pal:  CMD = lavf_container_timecode_nodrop "-ar 48000 -r 25 -s pal -ac 2 -f dv"
-fate-lavf-dv_ntsc:  CMD = lavf_container_timecode_drop "-ar 48000 -pix_fmt yuv411p -s ntsc -ac 2 -f dv"
+fate-lavf-dv_pal:  CMD = lavf_container_timecode_nodrop "-af aresample=48000:tsf=s16p -r 25 -s pal -ac 2 -f dv"
+fate-lavf-dv_ntsc:  CMD = lavf_container_timecode_drop "-af aresample=48000:tsf=s16p -pix_fmt yuv411p -s ntsc -ac 2 -f dv"
 fate-lavf-flv fate-lavf-swf: CMD = lavf_container "" "-an"
 fate-lavf-flm: CMD = lavf_container "" "-pix_fmt rgba"
-fate-lavf-gxf: CMD = lavf_container "-ar 48000" "-r 25 -s pal -ac 1 -threads 1"
-fate-lavf-gxf_pal: CMD = lavf_container_timecode_nodrop "-ar 48000 -r 25 -s pal -ac 1 -threads 1 -f gxf"
-fate-lavf-gxf_ntsc: CMD = lavf_container_timecode_drop "-ar 48000 -s ntsc -ac 1 -threads 1 -f gxf"
+fate-lavf-gxf: CMD = lavf_container "-af aresample=48000:tsf=s16p" "-r 25 -s pal -ac 1 -threads 1"
+fate-lavf-gxf_pal: CMD = lavf_container_timecode_nodrop "-af aresample=48000:tsf=s16p -r 25 -s pal -ac 1 -threads 1 -f gxf"
+fate-lavf-gxf_ntsc: CMD = lavf_container_timecode_drop "-af aresample=48000:tsf=s16p -s ntsc -ac 1 -threads 1 -f gxf"
 fate-lavf-ismv: CMD = lavf_container_timecode "-an -write_tmcd 1 -c:v mpeg4 -threads 1"
 fate-lavf-mkv: CMD = lavf_container "" "-c:a mp2 -c:v mpeg4 -ar 44100 -threads 1"
 fate-lavf-mkv_attachment: CMD = lavf_container_attach "-c:a mp2 -c:v mpeg4 -threads 1 -f matroska"
@@ -53,9 +53,9 @@  fate-lavf-mov: CMD = lavf_container_timecode "-movflags +faststart -c:a pcm_alaw
 fate-lavf-mov_rtphint: CMD = lavf_container "" "-movflags +rtphint -c:a pcm_alaw -c:v mpeg4 -threads 1 -f mov"
 fate-lavf-mp4: CMD = lavf_container_timecode "-c:v mpeg4 -an -threads 1"
 fate-lavf-mpg: CMD = lavf_container_timecode "-ar 44100 -threads 1"
-fate-lavf-mxf: CMD = lavf_container_timecode "-ar 48000 -bf 2 -threads 1"
-fate-lavf-mxf_d10: CMD = lavf_container "-ar 48000 -ac 2" "-r 25 -vf scale=720:576,pad=720:608:0:32,setfield=tff -c:v mpeg2video -g 0 -flags +ildct+low_delay -dc 10 -non_linear_quant 1 -intra_vlc 1 -qscale 1 -ps 1 -qmin 1 -rc_max_vbv_use 1 -rc_min_vbv_use 1 -pix_fmt yuv422p -minrate 30000k -maxrate 30000k -b 30000k -bufsize 1200000 -rc_init_occupancy 1200000 -qmax 12 -f mxf_d10"
-fate-lavf-mxf_dv25: CMD = lavf_container "-ar 48000 -ac 2" "-r 25 -vf scale=720:576,setdar=4/3,setfield=bff -c:v dvvideo -pix_fmt yuv420p -b 25000k -f mxf"
+fate-lavf-mxf: CMD = lavf_container_timecode "-af aresample=48000:tsf=s16p -bf 2 -threads 1"
+fate-lavf-mxf_d10: CMD = lavf_container "-af aresample=48000:tsf=s16p -ac 2" "-r 25 -vf scale=720:576,pad=720:608:0:32,setfield=tff -c:v mpeg2video -g 0 -flags +ildct+low_delay -dc 10 -non_linear_quant 1 -intra_vlc 1 -qscale 1 -ps 1 -qmin 1 -rc_max_vbv_use 1 -rc_min_vbv_use 1 -pix_fmt yuv422p -minrate 30000k -maxrate 30000k -b 30000k -bufsize 1200000 -rc_init_occupancy 1200000 -qmax 12 -f mxf_d10"
+fate-lavf-mxf_dv25: CMD = lavf_container "-af aresample=48000:tsf=s16p -ac 2" "-r 25 -vf scale=720:576,setdar=4/3,setfield=bff -c:v dvvideo -pix_fmt yuv420p -b 25000k -f mxf"
 fate-lavf-mxf_dvcpro50: CMD = lavf_container "-ar 48000 -ac 2" "-r 25 -vf scale=720:576,setdar=16/9,setfield=bff -c:v dvvideo -pix_fmt yuv422p -b 50000k -f mxf"
 fate-lavf-mxf_dvcpro100: CMD = lavf_container "-ar 48000 -ac 2" "-r 25 -vf scale=1440:1080,setdar=16/9,setfield=bff -c:v dvvideo -pix_fmt yuv422p -b 100000k -f mxf"
 fate-lavf-mxf_ffv1: CMD = lavf_container "-an" "-r 25 -vf scale=720:576,setdar=4/3 -c:v ffv1 -level 3 -pix_fmt yuv420p -f mxf"