diff mbox series

[FFmpeg-devel,v2,2/4] swscale/x86: add sse4 {lum, chr}ConvertRange

Message ID 20240611122858.196658-2-ramiro.polla@gmail.com
State New
Headers show
Series [FFmpeg-devel,v2,1/4] checkasm: add tests for {lum, chr}ConvertRange | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Ramiro Polla June 11, 2024, 12:28 p.m. UTC 
chrRangeFromJpeg_8_c: 28.7
chrRangeFromJpeg_8_sse4: 16.2
chrRangeFromJpeg_24_c: 152.7
chrRangeFromJpeg_24_sse4: 29.7
chrRangeFromJpeg_128_c: 366.5
chrRangeFromJpeg_128_sse4: 233.0
chrRangeFromJpeg_144_c: 408.0
chrRangeFromJpeg_144_sse4: 182.5
chrRangeFromJpeg_256_c: 698.7
chrRangeFromJpeg_256_sse4: 325.5
chrRangeFromJpeg_512_c: 1348.7
chrRangeFromJpeg_512_sse4: 660.2
chrRangeToJpeg_8_c: 37.7
chrRangeToJpeg_8_sse4: 16.2
chrRangeToJpeg_24_c: 115.7
chrRangeToJpeg_24_sse4: 36.2
chrRangeToJpeg_128_c: 631.2
chrRangeToJpeg_128_sse4: 163.7
chrRangeToJpeg_144_c: 710.7
chrRangeToJpeg_144_sse4: 183.0
chrRangeToJpeg_256_c: 1253.0
chrRangeToJpeg_256_sse4: 343.5
chrRangeToJpeg_512_c: 2491.2
chrRangeToJpeg_512_sse4: 654.2
lumRangeFromJpeg_8_c: 11.7
lumRangeFromJpeg_8_sse4: 10.5
lumRangeFromJpeg_24_c: 38.5
lumRangeFromJpeg_24_sse4: 19.0
lumRangeFromJpeg_128_c: 237.5
lumRangeFromJpeg_128_sse4: 79.2
lumRangeFromJpeg_144_c: 255.7
lumRangeFromJpeg_144_sse4: 90.5
lumRangeFromJpeg_256_c: 441.5
lumRangeFromJpeg_256_sse4: 161.7
lumRangeFromJpeg_512_c: 879.0
lumRangeFromJpeg_512_sse4: 333.2
lumRangeToJpeg_8_c: 20.0
lumRangeToJpeg_8_sse4: 11.7
lumRangeToJpeg_24_c: 61.5
lumRangeToJpeg_24_sse4: 17.7
lumRangeToJpeg_128_c: 357.5
lumRangeToJpeg_128_sse4: 80.0
lumRangeToJpeg_144_c: 371.5
lumRangeToJpeg_144_sse4: 93.2
lumRangeToJpeg_256_c: 651.5
lumRangeToJpeg_256_sse4: 164.5
lumRangeToJpeg_512_c: 1279.0
lumRangeToJpeg_512_sse4: 333.7
---
 libswscale/swscale_internal.h    |   1 +
 libswscale/utils.c               |   2 +
 libswscale/x86/Makefile          |   1 +
 libswscale/x86/range_convert.asm | 130 +++++++++++++++++++++++++++++++
 libswscale/x86/swscale.c         |  36 +++++++++
 5 files changed, 170 insertions(+)
 create mode 100644 libswscale/x86/range_convert.asm

Comments

James Almer June 11, 2024, 12:32 p.m. UTC | #1 
On 6/11/2024 9:28 AM, Ramiro Polla wrote:
> chrRangeFromJpeg_8_c: 28.7
> chrRangeFromJpeg_8_sse4: 16.2
> chrRangeFromJpeg_24_c: 152.7
> chrRangeFromJpeg_24_sse4: 29.7
> chrRangeFromJpeg_128_c: 366.5
> chrRangeFromJpeg_128_sse4: 233.0
> chrRangeFromJpeg_144_c: 408.0
> chrRangeFromJpeg_144_sse4: 182.5
> chrRangeFromJpeg_256_c: 698.7
> chrRangeFromJpeg_256_sse4: 325.5
> chrRangeFromJpeg_512_c: 1348.7
> chrRangeFromJpeg_512_sse4: 660.2
> chrRangeToJpeg_8_c: 37.7
> chrRangeToJpeg_8_sse4: 16.2
> chrRangeToJpeg_24_c: 115.7
> chrRangeToJpeg_24_sse4: 36.2
> chrRangeToJpeg_128_c: 631.2
> chrRangeToJpeg_128_sse4: 163.7
> chrRangeToJpeg_144_c: 710.7
> chrRangeToJpeg_144_sse4: 183.0
> chrRangeToJpeg_256_c: 1253.0
> chrRangeToJpeg_256_sse4: 343.5
> chrRangeToJpeg_512_c: 2491.2
> chrRangeToJpeg_512_sse4: 654.2
> lumRangeFromJpeg_8_c: 11.7
> lumRangeFromJpeg_8_sse4: 10.5
> lumRangeFromJpeg_24_c: 38.5
> lumRangeFromJpeg_24_sse4: 19.0
> lumRangeFromJpeg_128_c: 237.5
> lumRangeFromJpeg_128_sse4: 79.2
> lumRangeFromJpeg_144_c: 255.7
> lumRangeFromJpeg_144_sse4: 90.5
> lumRangeFromJpeg_256_c: 441.5
> lumRangeFromJpeg_256_sse4: 161.7
> lumRangeFromJpeg_512_c: 879.0
> lumRangeFromJpeg_512_sse4: 333.2
> lumRangeToJpeg_8_c: 20.0
> lumRangeToJpeg_8_sse4: 11.7
> lumRangeToJpeg_24_c: 61.5
> lumRangeToJpeg_24_sse4: 17.7
> lumRangeToJpeg_128_c: 357.5
> lumRangeToJpeg_128_sse4: 80.0
> lumRangeToJpeg_144_c: 371.5
> lumRangeToJpeg_144_sse4: 93.2
> lumRangeToJpeg_256_c: 651.5
> lumRangeToJpeg_256_sse4: 164.5
> lumRangeToJpeg_512_c: 1279.0
> lumRangeToJpeg_512_sse4: 333.7
> ---
>   libswscale/swscale_internal.h    |   1 +
>   libswscale/utils.c               |   2 +
>   libswscale/x86/Makefile          |   1 +
>   libswscale/x86/range_convert.asm | 130 +++++++++++++++++++++++++++++++
>   libswscale/x86/swscale.c         |  36 +++++++++
>   5 files changed, 170 insertions(+)
>   create mode 100644 libswscale/x86/range_convert.asm
> 
> diff --git a/libswscale/swscale_internal.h b/libswscale/swscale_internal.h
> index 5007dd422f..d5e7b5e71c 100644
> --- a/libswscale/swscale_internal.h
> +++ b/libswscale/swscale_internal.h
> @@ -698,6 +698,7 @@ void ff_updateMMXDitherTables(SwsContext *c, int dstY);
>   
>   av_cold void ff_sws_init_range_convert(SwsContext *c);
>   av_cold void ff_sws_init_range_convert_loongarch(SwsContext *c);
> +av_cold void ff_sws_init_range_convert_x86(SwsContext *c);
>   
>   SwsFunc ff_yuv2rgb_init_x86(SwsContext *c);
>   SwsFunc ff_yuv2rgb_init_ppc(SwsContext *c);
> diff --git a/libswscale/utils.c b/libswscale/utils.c
> index 476a24fea5..8dfa57b5ff 100644
> --- a/libswscale/utils.c
> +++ b/libswscale/utils.c
> @@ -1082,6 +1082,8 @@ int sws_setColorspaceDetails(struct SwsContext *c, const int inv_table[4],
>           ff_sws_init_range_convert(c);
>   #if ARCH_LOONGARCH64
>           ff_sws_init_range_convert_loongarch(c);
> +#elif ARCH_X86
> +        ff_sws_init_range_convert_x86(c);
>   #endif
>       }
>   
> diff --git a/libswscale/x86/Makefile b/libswscale/x86/Makefile
> index 68391494be..f00154941d 100644
> --- a/libswscale/x86/Makefile
> +++ b/libswscale/x86/Makefile
> @@ -12,6 +12,7 @@ X86ASM-OBJS                     += x86/input.o                          \
>                                      x86/output.o                         \
>                                      x86/scale.o                          \
>                                      x86/scale_avx2.o                          \
> +                                   x86/range_convert.o                  \
>                                      x86/rgb_2_rgb.o                      \
>                                      x86/yuv_2_rgb.o                      \
>                                      x86/yuv2yuvX.o                       \
> diff --git a/libswscale/x86/range_convert.asm b/libswscale/x86/range_convert.asm
> new file mode 100644
> index 0000000000..13983a386b
> --- /dev/null
> +++ b/libswscale/x86/range_convert.asm
> @@ -0,0 +1,130 @@
> +;******************************************************************************
> +;* Copyright (c) 2024 Ramiro Polla
> +;*
> +;* This file is part of FFmpeg.
> +;*
> +;* FFmpeg is free software; you can redistribute it and/or
> +;* modify it under the terms of the GNU Lesser General Public
> +;* License as published by the Free Software Foundation; either
> +;* version 2.1 of the License, or (at your option) any later version.
> +;*
> +;* FFmpeg is distributed in the hope that it will be useful,
> +;* but WITHOUT ANY WARRANTY; without even the implied warranty of
> +;* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +;* Lesser General Public License for more details.
> +;*
> +;* You should have received a copy of the GNU Lesser General Public
> +;* License along with FFmpeg; if not, write to the Free Software
> +;* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> +;******************************************************************************
> +
> +%include "libavutil/x86/x86util.asm"
> +
> +SECTION_RODATA
> +
> +chr_to_mult:        times 4 dd 4663
> +chr_to_offset:      times 4 dd -9289992
> +%define chr_to_shift 12
> +
> +chr_from_mult:      times 4 dd 1799
> +chr_from_offset:    times 4 dd 4081085
> +%define chr_from_shift 11
> +
> +lum_to_mult:        times 4 dd 19077
> +lum_to_offset:      times 4 dd -39057361
> +%define lum_to_shift 14
> +
> +lum_from_mult:      times 4 dd 14071
> +lum_from_offset:    times 4 dd 33561947
> +%define lum_from_shift 14
> +
> +SECTION .text
> +
> +; NOTE: there is no need to clamp the input when converting to jpeg range
> +;       (like we do in the C code) because packssdw will saturate the output.
> +
> +;-----------------------------------------------------------------------------
> +; lumConvertRange
> +;
> +; void ff_lumRangeToJpeg_<opt>(int16_t *dst, int width);
> +; void ff_lumRangeFromJpeg_<opt>(int16_t *dst, int width);
> +;
> +;-----------------------------------------------------------------------------
> +
> +%macro LUMCONVERTRANGE 4
> +cglobal %1, 2, 3, 3, dst, width, x
> +    movsxdifnidn widthq, widthd
> +    xor              xq, xq
> +    mova             m4, [%2]
> +    mova             m5, [%3]
> +.loop:
> +    pmovsxwd         m0, [dstq+xq*2]
> +    pmovsxwd         m1, [dstq+xq*2+mmsize/2]
> +    pmulld           m0, m4
> +    pmulld           m1, m4

Can't you use pmaddwd without sign extending to dword instead? pmulld is 
pretty slow.

> +    paddd            m0, m5
> +    paddd            m1, m5
> +    psrad            m0, %4
> +    psrad            m1, %4
> +    packssdw         m0, m0
> +    packssdw         m1, m1
> +    movq    [dstq+xq*2], m0
> +    movq    [dstq+xq*2+mmsize/2], m1
> +    add              xq, mmsize / 2
> +    cmp              xd, widthd
> +    jl .loop
> +    RET
> +%endmacro
> +
> +;-----------------------------------------------------------------------------
> +; chrConvertRange
> +;
> +; void ff_chrRangeToJpeg_<opt>(int16_t *dstU, int16_t *dstV, int width);
> +; void ff_chrRangeFromJpeg_<opt>(int16_t *dstU, int16_t *dstV, int width);
> +;
> +;-----------------------------------------------------------------------------
> +
> +%macro CHRCONVERTRANGE 4
> +cglobal %1, 3, 4, 4, dstU, dstV, width, x
> +    movsxdifnidn widthq, widthd
> +    xor              xq, xq
> +    mova             m4, [%2]
> +    mova             m5, [%3]
> +.loop:
> +    pmovsxwd         m0, [dstUq+xq*2]
> +    pmovsxwd         m1, [dstUq+xq*2+mmsize/2]
> +    pmovsxwd         m2, [dstVq+xq*2]
> +    pmovsxwd         m3, [dstVq+xq*2+mmsize/2]
> +    pmulld           m0, m4
> +    pmulld           m1, m4
> +    pmulld           m2, m4
> +    pmulld           m3, m4
> +    paddd            m0, m5
> +    paddd            m1, m5
> +    paddd            m2, m5
> +    paddd            m3, m5
> +    psrad            m0, %4
> +    psrad            m1, %4
> +    psrad            m2, %4
> +    psrad            m3, %4
> +    packssdw         m0, m0
> +    packssdw         m1, m1
> +    packssdw         m2, m2
> +    packssdw         m3, m3
> +    movq   [dstUq+xq*2], m0
> +    movq   [dstUq+xq*2+mmsize/2], m1
> +    movq   [dstVq+xq*2], m2
> +    movq   [dstVq+xq*2+mmsize/2], m3
> +    add              xq, mmsize / 2
> +    cmp              xd, widthd
> +    jl .loop
> +    RET
> +%endmacro
> +
> +%if ARCH_X86_64
> +INIT_XMM sse4
> +LUMCONVERTRANGE lumRangeToJpeg,   lum_to_mult,   lum_to_offset,   lum_to_shift
> +CHRCONVERTRANGE chrRangeToJpeg,   chr_to_mult,   chr_to_offset,   chr_to_shift
> +LUMCONVERTRANGE lumRangeFromJpeg, lum_from_mult, lum_from_offset, lum_from_shift
> +CHRCONVERTRANGE chrRangeFromJpeg, chr_from_mult, chr_from_offset, chr_from_shift
> +%endif
> diff --git a/libswscale/x86/swscale.c b/libswscale/x86/swscale.c
> index 5a9da23265..8f477b7b72 100644
> --- a/libswscale/x86/swscale.c
> +++ b/libswscale/x86/swscale.c
> @@ -453,6 +453,38 @@ INPUT_PLANAR_RGB_UV_ALL_DECL(avx2);
>   INPUT_PLANAR_RGB_A_ALL_DECL(avx2);
>   #endif
>   
> +#if ARCH_X86_64
> +#define RANGE_CONVERT_FUNCS(opt) do {                                       \
> +    if (c->dstBpc <= 14) {                                                  \
> +        if (c->srcRange) {                                                  \
> +            c->lumConvertRange = ff_lumRangeFromJpeg_ ##opt;                \
> +            c->chrConvertRange = ff_chrRangeFromJpeg_ ##opt;                \
> +        } else {                                                            \
> +            c->lumConvertRange = ff_lumRangeToJpeg_ ##opt;                  \
> +            c->chrConvertRange = ff_chrRangeToJpeg_ ##opt;                  \
> +        }                                                                   \
> +    }                                                                       \
> +} while (0)
> +
> +#define RANGE_CONVERT_FUNCS_DECL(opt)                                       \
> +void ff_lumRangeFromJpeg_ ##opt(int16_t *dst, int width);                   \
> +void ff_chrRangeFromJpeg_ ##opt(int16_t *dstU, int16_t *dstV, int width);   \
> +void ff_lumRangeToJpeg_ ##opt(int16_t *dst, int width);                     \
> +void ff_chrRangeToJpeg_ ##opt(int16_t *dstU, int16_t *dstV, int width);     \
> +
> +RANGE_CONVERT_FUNCS_DECL(sse4);
> +
> +av_cold void ff_sws_init_range_convert_x86(SwsContext *c)
> +{
> +    if (c->srcRange != c->dstRange && !isAnyRGB(c->dstFormat)) {
> +        int cpu_flags = av_get_cpu_flags();
> +        if (EXTERNAL_SSE4(cpu_flags)) {
> +            RANGE_CONVERT_FUNCS(sse4);
> +        }
> +    }
> +}
> +#endif
> +
>   av_cold void ff_sws_init_swscale_x86(SwsContext *c)
>   {
>       int cpu_flags = av_get_cpu_flags();
> @@ -820,4 +852,8 @@ switch(c->dstBpc){ \
>       }
>   
>   #endif
> +
> +#if ARCH_X86_64
> +    ff_sws_init_range_convert_x86(c);
> +#endif
>   }
Michael Niedermayer June 11, 2024, 6:26 p.m. UTC | #2 
On Tue, Jun 11, 2024 at 02:28:56PM +0200, Ramiro Polla wrote:
> chrRangeFromJpeg_8_c: 28.7
> chrRangeFromJpeg_8_sse4: 16.2
> chrRangeFromJpeg_24_c: 152.7
> chrRangeFromJpeg_24_sse4: 29.7
> chrRangeFromJpeg_128_c: 366.5
> chrRangeFromJpeg_128_sse4: 233.0
> chrRangeFromJpeg_144_c: 408.0
> chrRangeFromJpeg_144_sse4: 182.5
> chrRangeFromJpeg_256_c: 698.7
> chrRangeFromJpeg_256_sse4: 325.5
> chrRangeFromJpeg_512_c: 1348.7
> chrRangeFromJpeg_512_sse4: 660.2
> chrRangeToJpeg_8_c: 37.7
> chrRangeToJpeg_8_sse4: 16.2
> chrRangeToJpeg_24_c: 115.7
> chrRangeToJpeg_24_sse4: 36.2
> chrRangeToJpeg_128_c: 631.2
> chrRangeToJpeg_128_sse4: 163.7
> chrRangeToJpeg_144_c: 710.7
> chrRangeToJpeg_144_sse4: 183.0
> chrRangeToJpeg_256_c: 1253.0
> chrRangeToJpeg_256_sse4: 343.5
> chrRangeToJpeg_512_c: 2491.2
> chrRangeToJpeg_512_sse4: 654.2
> lumRangeFromJpeg_8_c: 11.7
> lumRangeFromJpeg_8_sse4: 10.5
> lumRangeFromJpeg_24_c: 38.5
> lumRangeFromJpeg_24_sse4: 19.0
> lumRangeFromJpeg_128_c: 237.5
> lumRangeFromJpeg_128_sse4: 79.2
> lumRangeFromJpeg_144_c: 255.7
> lumRangeFromJpeg_144_sse4: 90.5
> lumRangeFromJpeg_256_c: 441.5
> lumRangeFromJpeg_256_sse4: 161.7
> lumRangeFromJpeg_512_c: 879.0
> lumRangeFromJpeg_512_sse4: 333.2
> lumRangeToJpeg_8_c: 20.0
> lumRangeToJpeg_8_sse4: 11.7
> lumRangeToJpeg_24_c: 61.5
> lumRangeToJpeg_24_sse4: 17.7
> lumRangeToJpeg_128_c: 357.5
> lumRangeToJpeg_128_sse4: 80.0
> lumRangeToJpeg_144_c: 371.5
> lumRangeToJpeg_144_sse4: 93.2
> lumRangeToJpeg_256_c: 651.5
> lumRangeToJpeg_256_sse4: 164.5
> lumRangeToJpeg_512_c: 1279.0
> lumRangeToJpeg_512_sse4: 333.7
> ---
>  libswscale/swscale_internal.h    |   1 +
>  libswscale/utils.c               |   2 +
>  libswscale/x86/Makefile          |   1 +
>  libswscale/x86/range_convert.asm | 130 +++++++++++++++++++++++++++++++
>  libswscale/x86/swscale.c         |  36 +++++++++
>  5 files changed, 170 insertions(+)
>  create mode 100644 libswscale/x86/range_convert.asm

breaks x86-32 build

LD	ffmpeg_g
/usr/lib/gcc-cross/i686-linux-gnu/7/../../../../i686-linux-gnu/bin/ld: libswscale/libswscale.a(utils.o): in function `sws_setColorspaceDetails':
ffmpeg/linux32/src/libswscale/utils.c:1086: undefined reference to `ff_sws_init_range_convert_x86'
collect2: error: ld returned 1 exit status
make: *** [Makefile:139: ffmpeg_g] Error 1

thx

[...]
James Almer June 11, 2024, 6:43 p.m. UTC | #3 
On 6/11/2024 3:26 PM, Michael Niedermayer wrote:
> On Tue, Jun 11, 2024 at 02:28:56PM +0200, Ramiro Polla wrote:
>> chrRangeFromJpeg_8_c: 28.7
>> chrRangeFromJpeg_8_sse4: 16.2
>> chrRangeFromJpeg_24_c: 152.7
>> chrRangeFromJpeg_24_sse4: 29.7
>> chrRangeFromJpeg_128_c: 366.5
>> chrRangeFromJpeg_128_sse4: 233.0
>> chrRangeFromJpeg_144_c: 408.0
>> chrRangeFromJpeg_144_sse4: 182.5
>> chrRangeFromJpeg_256_c: 698.7
>> chrRangeFromJpeg_256_sse4: 325.5
>> chrRangeFromJpeg_512_c: 1348.7
>> chrRangeFromJpeg_512_sse4: 660.2
>> chrRangeToJpeg_8_c: 37.7
>> chrRangeToJpeg_8_sse4: 16.2
>> chrRangeToJpeg_24_c: 115.7
>> chrRangeToJpeg_24_sse4: 36.2
>> chrRangeToJpeg_128_c: 631.2
>> chrRangeToJpeg_128_sse4: 163.7
>> chrRangeToJpeg_144_c: 710.7
>> chrRangeToJpeg_144_sse4: 183.0
>> chrRangeToJpeg_256_c: 1253.0
>> chrRangeToJpeg_256_sse4: 343.5
>> chrRangeToJpeg_512_c: 2491.2
>> chrRangeToJpeg_512_sse4: 654.2
>> lumRangeFromJpeg_8_c: 11.7
>> lumRangeFromJpeg_8_sse4: 10.5
>> lumRangeFromJpeg_24_c: 38.5
>> lumRangeFromJpeg_24_sse4: 19.0
>> lumRangeFromJpeg_128_c: 237.5
>> lumRangeFromJpeg_128_sse4: 79.2
>> lumRangeFromJpeg_144_c: 255.7
>> lumRangeFromJpeg_144_sse4: 90.5
>> lumRangeFromJpeg_256_c: 441.5
>> lumRangeFromJpeg_256_sse4: 161.7
>> lumRangeFromJpeg_512_c: 879.0
>> lumRangeFromJpeg_512_sse4: 333.2
>> lumRangeToJpeg_8_c: 20.0
>> lumRangeToJpeg_8_sse4: 11.7
>> lumRangeToJpeg_24_c: 61.5
>> lumRangeToJpeg_24_sse4: 17.7
>> lumRangeToJpeg_128_c: 357.5
>> lumRangeToJpeg_128_sse4: 80.0
>> lumRangeToJpeg_144_c: 371.5
>> lumRangeToJpeg_144_sse4: 93.2
>> lumRangeToJpeg_256_c: 651.5
>> lumRangeToJpeg_256_sse4: 164.5
>> lumRangeToJpeg_512_c: 1279.0
>> lumRangeToJpeg_512_sse4: 333.7
>> ---
>>   libswscale/swscale_internal.h    |   1 +
>>   libswscale/utils.c               |   2 +
>>   libswscale/x86/Makefile          |   1 +
>>   libswscale/x86/range_convert.asm | 130 +++++++++++++++++++++++++++++++
>>   libswscale/x86/swscale.c         |  36 +++++++++
>>   5 files changed, 170 insertions(+)
>>   create mode 100644 libswscale/x86/range_convert.asm
> 
> breaks x86-32 build
> 
> LD	ffmpeg_g
> /usr/lib/gcc-cross/i686-linux-gnu/7/../../../../i686-linux-gnu/bin/ld: libswscale/libswscale.a(utils.o): in function `sws_setColorspaceDetails':
> ffmpeg/linux32/src/libswscale/utils.c:1086: undefined reference to `ff_sws_init_range_convert_x86'
> collect2: error: ld returned 1 exit status
> make: *** [Makefile:139: ffmpeg_g] Error 1
> 
> thx

The functions are wrapped in ARCH_X86_64 checks for seemingly no reason, 
so they should be removed in the next iteration.
Ramiro Polla June 12, 2024, 2:54 p.m. UTC | #4 
Hi,

On Tue, Jun 11, 2024 at 8:42 PM James Almer <jamrial@gmail.com> wrote:
>
> On 6/11/2024 3:26 PM, Michael Niedermayer wrote:
> > On Tue, Jun 11, 2024 at 02:28:56PM +0200, Ramiro Polla wrote:
> >> chrRangeFromJpeg_8_c: 28.7
> >> chrRangeFromJpeg_8_sse4: 16.2
> >> chrRangeFromJpeg_24_c: 152.7
> >> chrRangeFromJpeg_24_sse4: 29.7
> >> chrRangeFromJpeg_128_c: 366.5
> >> chrRangeFromJpeg_128_sse4: 233.0
> >> chrRangeFromJpeg_144_c: 408.0
> >> chrRangeFromJpeg_144_sse4: 182.5
> >> chrRangeFromJpeg_256_c: 698.7
> >> chrRangeFromJpeg_256_sse4: 325.5
> >> chrRangeFromJpeg_512_c: 1348.7
> >> chrRangeFromJpeg_512_sse4: 660.2
> >> chrRangeToJpeg_8_c: 37.7
> >> chrRangeToJpeg_8_sse4: 16.2
> >> chrRangeToJpeg_24_c: 115.7
> >> chrRangeToJpeg_24_sse4: 36.2
> >> chrRangeToJpeg_128_c: 631.2
> >> chrRangeToJpeg_128_sse4: 163.7
> >> chrRangeToJpeg_144_c: 710.7
> >> chrRangeToJpeg_144_sse4: 183.0
> >> chrRangeToJpeg_256_c: 1253.0
> >> chrRangeToJpeg_256_sse4: 343.5
> >> chrRangeToJpeg_512_c: 2491.2
> >> chrRangeToJpeg_512_sse4: 654.2
> >> lumRangeFromJpeg_8_c: 11.7
> >> lumRangeFromJpeg_8_sse4: 10.5
> >> lumRangeFromJpeg_24_c: 38.5
> >> lumRangeFromJpeg_24_sse4: 19.0
> >> lumRangeFromJpeg_128_c: 237.5
> >> lumRangeFromJpeg_128_sse4: 79.2
> >> lumRangeFromJpeg_144_c: 255.7
> >> lumRangeFromJpeg_144_sse4: 90.5
> >> lumRangeFromJpeg_256_c: 441.5
> >> lumRangeFromJpeg_256_sse4: 161.7
> >> lumRangeFromJpeg_512_c: 879.0
> >> lumRangeFromJpeg_512_sse4: 333.2
> >> lumRangeToJpeg_8_c: 20.0
> >> lumRangeToJpeg_8_sse4: 11.7
> >> lumRangeToJpeg_24_c: 61.5
> >> lumRangeToJpeg_24_sse4: 17.7
> >> lumRangeToJpeg_128_c: 357.5
> >> lumRangeToJpeg_128_sse4: 80.0
> >> lumRangeToJpeg_144_c: 371.5
> >> lumRangeToJpeg_144_sse4: 93.2
> >> lumRangeToJpeg_256_c: 651.5
> >> lumRangeToJpeg_256_sse4: 164.5
> >> lumRangeToJpeg_512_c: 1279.0
> >> lumRangeToJpeg_512_sse4: 333.7
> >> ---
> >>   libswscale/swscale_internal.h    |   1 +
> >>   libswscale/utils.c               |   2 +
> >>   libswscale/x86/Makefile          |   1 +
> >>   libswscale/x86/range_convert.asm | 130 +++++++++++++++++++++++++++++++
> >>   libswscale/x86/swscale.c         |  36 +++++++++
> >>   5 files changed, 170 insertions(+)
> >>   create mode 100644 libswscale/x86/range_convert.asm
> >
> > breaks x86-32 build
> >
> > LD    ffmpeg_g
> > /usr/lib/gcc-cross/i686-linux-gnu/7/../../../../i686-linux-gnu/bin/ld: libswscale/libswscale.a(utils.o): in function `sws_setColorspaceDetails':
> > ffmpeg/linux32/src/libswscale/utils.c:1086: undefined reference to `ff_sws_init_range_convert_x86'
> > collect2: error: ld returned 1 exit status
> > make: *** [Makefile:139: ffmpeg_g] Error 1
> >
> > thx
>
> The functions are wrapped in ARCH_X86_64 checks for seemingly no reason,
> so they should be removed in the next iteration.

Fixed.

James walked me through on IRC to optimize and improve the functions
in a way that they work both with sse2 and avx2. New patch attached.
Ramiro Polla June 14, 2024, 3:46 p.m. UTC | #5 
On Wed, Jun 12, 2024 at 4:54 PM Ramiro Polla <ramiro.polla@gmail.com> wrote:
>
> Hi,
>
> On Tue, Jun 11, 2024 at 8:42 PM James Almer <jamrial@gmail.com> wrote:
> >
> > On 6/11/2024 3:26 PM, Michael Niedermayer wrote:
> > > On Tue, Jun 11, 2024 at 02:28:56PM +0200, Ramiro Polla wrote:
> > >> chrRangeFromJpeg_8_c: 28.7
> > >> chrRangeFromJpeg_8_sse4: 16.2
> > >> chrRangeFromJpeg_24_c: 152.7
> > >> chrRangeFromJpeg_24_sse4: 29.7
> > >> chrRangeFromJpeg_128_c: 366.5
> > >> chrRangeFromJpeg_128_sse4: 233.0
> > >> chrRangeFromJpeg_144_c: 408.0
> > >> chrRangeFromJpeg_144_sse4: 182.5
> > >> chrRangeFromJpeg_256_c: 698.7
> > >> chrRangeFromJpeg_256_sse4: 325.5
> > >> chrRangeFromJpeg_512_c: 1348.7
> > >> chrRangeFromJpeg_512_sse4: 660.2
> > >> chrRangeToJpeg_8_c: 37.7
> > >> chrRangeToJpeg_8_sse4: 16.2
> > >> chrRangeToJpeg_24_c: 115.7
> > >> chrRangeToJpeg_24_sse4: 36.2
> > >> chrRangeToJpeg_128_c: 631.2
> > >> chrRangeToJpeg_128_sse4: 163.7
> > >> chrRangeToJpeg_144_c: 710.7
> > >> chrRangeToJpeg_144_sse4: 183.0
> > >> chrRangeToJpeg_256_c: 1253.0
> > >> chrRangeToJpeg_256_sse4: 343.5
> > >> chrRangeToJpeg_512_c: 2491.2
> > >> chrRangeToJpeg_512_sse4: 654.2
> > >> lumRangeFromJpeg_8_c: 11.7
> > >> lumRangeFromJpeg_8_sse4: 10.5
> > >> lumRangeFromJpeg_24_c: 38.5
> > >> lumRangeFromJpeg_24_sse4: 19.0
> > >> lumRangeFromJpeg_128_c: 237.5
> > >> lumRangeFromJpeg_128_sse4: 79.2
> > >> lumRangeFromJpeg_144_c: 255.7
> > >> lumRangeFromJpeg_144_sse4: 90.5
> > >> lumRangeFromJpeg_256_c: 441.5
> > >> lumRangeFromJpeg_256_sse4: 161.7
> > >> lumRangeFromJpeg_512_c: 879.0
> > >> lumRangeFromJpeg_512_sse4: 333.2
> > >> lumRangeToJpeg_8_c: 20.0
> > >> lumRangeToJpeg_8_sse4: 11.7
> > >> lumRangeToJpeg_24_c: 61.5
> > >> lumRangeToJpeg_24_sse4: 17.7
> > >> lumRangeToJpeg_128_c: 357.5
> > >> lumRangeToJpeg_128_sse4: 80.0
> > >> lumRangeToJpeg_144_c: 371.5
> > >> lumRangeToJpeg_144_sse4: 93.2
> > >> lumRangeToJpeg_256_c: 651.5
> > >> lumRangeToJpeg_256_sse4: 164.5
> > >> lumRangeToJpeg_512_c: 1279.0
> > >> lumRangeToJpeg_512_sse4: 333.7
> > >> ---
> > >>   libswscale/swscale_internal.h    |   1 +
> > >>   libswscale/utils.c               |   2 +
> > >>   libswscale/x86/Makefile          |   1 +
> > >>   libswscale/x86/range_convert.asm | 130 +++++++++++++++++++++++++++++++
> > >>   libswscale/x86/swscale.c         |  36 +++++++++
> > >>   5 files changed, 170 insertions(+)
> > >>   create mode 100644 libswscale/x86/range_convert.asm
> > >
> > > breaks x86-32 build
> > >
> > > LD    ffmpeg_g
> > > /usr/lib/gcc-cross/i686-linux-gnu/7/../../../../i686-linux-gnu/bin/ld: libswscale/libswscale.a(utils.o): in function `sws_setColorspaceDetails':
> > > ffmpeg/linux32/src/libswscale/utils.c:1086: undefined reference to `ff_sws_init_range_convert_x86'
> > > collect2: error: ld returned 1 exit status
> > > make: *** [Makefile:139: ffmpeg_g] Error 1
> > >
> > > thx
> >
> > The functions are wrapped in ARCH_X86_64 checks for seemingly no reason,
> > so they should be removed in the next iteration.
>
> Fixed.
>
> James walked me through on IRC to optimize and improve the functions
> in a way that they work both with sse2 and avx2. New patch attached.

I'll apply tomorrow if there are no more comments.
diff mbox series

Patch

diff --git a/libswscale/swscale_internal.h b/libswscale/swscale_internal.h
index 5007dd422f..d5e7b5e71c 100644
--- a/libswscale/swscale_internal.h
+++ b/libswscale/swscale_internal.h
@@ -698,6 +698,7 @@  void ff_updateMMXDitherTables(SwsContext *c, int dstY);
 
 av_cold void ff_sws_init_range_convert(SwsContext *c);
 av_cold void ff_sws_init_range_convert_loongarch(SwsContext *c);
+av_cold void ff_sws_init_range_convert_x86(SwsContext *c);
 
 SwsFunc ff_yuv2rgb_init_x86(SwsContext *c);
 SwsFunc ff_yuv2rgb_init_ppc(SwsContext *c);
diff --git a/libswscale/utils.c b/libswscale/utils.c
index 476a24fea5..8dfa57b5ff 100644
--- a/libswscale/utils.c
+++ b/libswscale/utils.c
@@ -1082,6 +1082,8 @@  int sws_setColorspaceDetails(struct SwsContext *c, const int inv_table[4],
         ff_sws_init_range_convert(c);
 #if ARCH_LOONGARCH64
         ff_sws_init_range_convert_loongarch(c);
+#elif ARCH_X86
+        ff_sws_init_range_convert_x86(c);
 #endif
     }
 
diff --git a/libswscale/x86/Makefile b/libswscale/x86/Makefile
index 68391494be..f00154941d 100644
--- a/libswscale/x86/Makefile
+++ b/libswscale/x86/Makefile
@@ -12,6 +12,7 @@  X86ASM-OBJS                     += x86/input.o                          \
                                    x86/output.o                         \
                                    x86/scale.o                          \
                                    x86/scale_avx2.o                          \
+                                   x86/range_convert.o                  \
                                    x86/rgb_2_rgb.o                      \
                                    x86/yuv_2_rgb.o                      \
                                    x86/yuv2yuvX.o                       \
diff --git a/libswscale/x86/range_convert.asm b/libswscale/x86/range_convert.asm
new file mode 100644
index 0000000000..13983a386b
--- /dev/null
+++ b/libswscale/x86/range_convert.asm
@@ -0,0 +1,130 @@ 
+;******************************************************************************
+;* Copyright (c) 2024 Ramiro Polla
+;*
+;* This file is part of FFmpeg.
+;*
+;* FFmpeg is free software; you can redistribute it and/or
+;* modify it under the terms of the GNU Lesser General Public
+;* License as published by the Free Software Foundation; either
+;* version 2.1 of the License, or (at your option) any later version.
+;*
+;* FFmpeg is distributed in the hope that it will be useful,
+;* but WITHOUT ANY WARRANTY; without even the implied warranty of
+;* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+;* Lesser General Public License for more details.
+;*
+;* You should have received a copy of the GNU Lesser General Public
+;* License along with FFmpeg; if not, write to the Free Software
+;* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+;******************************************************************************
+
+%include "libavutil/x86/x86util.asm"
+
+SECTION_RODATA
+
+chr_to_mult:        times 4 dd 4663
+chr_to_offset:      times 4 dd -9289992
+%define chr_to_shift 12
+
+chr_from_mult:      times 4 dd 1799
+chr_from_offset:    times 4 dd 4081085
+%define chr_from_shift 11
+
+lum_to_mult:        times 4 dd 19077
+lum_to_offset:      times 4 dd -39057361
+%define lum_to_shift 14
+
+lum_from_mult:      times 4 dd 14071
+lum_from_offset:    times 4 dd 33561947
+%define lum_from_shift 14
+
+SECTION .text
+
+; NOTE: there is no need to clamp the input when converting to jpeg range
+;       (like we do in the C code) because packssdw will saturate the output.
+
+;-----------------------------------------------------------------------------
+; lumConvertRange
+;
+; void ff_lumRangeToJpeg_<opt>(int16_t *dst, int width);
+; void ff_lumRangeFromJpeg_<opt>(int16_t *dst, int width);
+;
+;-----------------------------------------------------------------------------
+
+%macro LUMCONVERTRANGE 4
+cglobal %1, 2, 3, 3, dst, width, x
+    movsxdifnidn widthq, widthd
+    xor              xq, xq
+    mova             m4, [%2]
+    mova             m5, [%3]
+.loop:
+    pmovsxwd         m0, [dstq+xq*2]
+    pmovsxwd         m1, [dstq+xq*2+mmsize/2]
+    pmulld           m0, m4
+    pmulld           m1, m4
+    paddd            m0, m5
+    paddd            m1, m5
+    psrad            m0, %4
+    psrad            m1, %4
+    packssdw         m0, m0
+    packssdw         m1, m1
+    movq    [dstq+xq*2], m0
+    movq    [dstq+xq*2+mmsize/2], m1
+    add              xq, mmsize / 2
+    cmp              xd, widthd
+    jl .loop
+    RET
+%endmacro
+
+;-----------------------------------------------------------------------------
+; chrConvertRange
+;
+; void ff_chrRangeToJpeg_<opt>(int16_t *dstU, int16_t *dstV, int width);
+; void ff_chrRangeFromJpeg_<opt>(int16_t *dstU, int16_t *dstV, int width);
+;
+;-----------------------------------------------------------------------------
+
+%macro CHRCONVERTRANGE 4
+cglobal %1, 3, 4, 4, dstU, dstV, width, x
+    movsxdifnidn widthq, widthd
+    xor              xq, xq
+    mova             m4, [%2]
+    mova             m5, [%3]
+.loop:
+    pmovsxwd         m0, [dstUq+xq*2]
+    pmovsxwd         m1, [dstUq+xq*2+mmsize/2]
+    pmovsxwd         m2, [dstVq+xq*2]
+    pmovsxwd         m3, [dstVq+xq*2+mmsize/2]
+    pmulld           m0, m4
+    pmulld           m1, m4
+    pmulld           m2, m4
+    pmulld           m3, m4
+    paddd            m0, m5
+    paddd            m1, m5
+    paddd            m2, m5
+    paddd            m3, m5
+    psrad            m0, %4
+    psrad            m1, %4
+    psrad            m2, %4
+    psrad            m3, %4
+    packssdw         m0, m0
+    packssdw         m1, m1
+    packssdw         m2, m2
+    packssdw         m3, m3
+    movq   [dstUq+xq*2], m0
+    movq   [dstUq+xq*2+mmsize/2], m1
+    movq   [dstVq+xq*2], m2
+    movq   [dstVq+xq*2+mmsize/2], m3
+    add              xq, mmsize / 2
+    cmp              xd, widthd
+    jl .loop
+    RET
+%endmacro
+
+%if ARCH_X86_64
+INIT_XMM sse4
+LUMCONVERTRANGE lumRangeToJpeg,   lum_to_mult,   lum_to_offset,   lum_to_shift
+CHRCONVERTRANGE chrRangeToJpeg,   chr_to_mult,   chr_to_offset,   chr_to_shift
+LUMCONVERTRANGE lumRangeFromJpeg, lum_from_mult, lum_from_offset, lum_from_shift
+CHRCONVERTRANGE chrRangeFromJpeg, chr_from_mult, chr_from_offset, chr_from_shift
+%endif
diff --git a/libswscale/x86/swscale.c b/libswscale/x86/swscale.c
index 5a9da23265..8f477b7b72 100644
--- a/libswscale/x86/swscale.c
+++ b/libswscale/x86/swscale.c
@@ -453,6 +453,38 @@  INPUT_PLANAR_RGB_UV_ALL_DECL(avx2);
 INPUT_PLANAR_RGB_A_ALL_DECL(avx2);
 #endif
 
+#if ARCH_X86_64
+#define RANGE_CONVERT_FUNCS(opt) do {                                       \
+    if (c->dstBpc <= 14) {                                                  \
+        if (c->srcRange) {                                                  \
+            c->lumConvertRange = ff_lumRangeFromJpeg_ ##opt;                \
+            c->chrConvertRange = ff_chrRangeFromJpeg_ ##opt;                \
+        } else {                                                            \
+            c->lumConvertRange = ff_lumRangeToJpeg_ ##opt;                  \
+            c->chrConvertRange = ff_chrRangeToJpeg_ ##opt;                  \
+        }                                                                   \
+    }                                                                       \
+} while (0)
+
+#define RANGE_CONVERT_FUNCS_DECL(opt)                                       \
+void ff_lumRangeFromJpeg_ ##opt(int16_t *dst, int width);                   \
+void ff_chrRangeFromJpeg_ ##opt(int16_t *dstU, int16_t *dstV, int width);   \
+void ff_lumRangeToJpeg_ ##opt(int16_t *dst, int width);                     \
+void ff_chrRangeToJpeg_ ##opt(int16_t *dstU, int16_t *dstV, int width);     \
+
+RANGE_CONVERT_FUNCS_DECL(sse4);
+
+av_cold void ff_sws_init_range_convert_x86(SwsContext *c)
+{
+    if (c->srcRange != c->dstRange && !isAnyRGB(c->dstFormat)) {
+        int cpu_flags = av_get_cpu_flags();
+        if (EXTERNAL_SSE4(cpu_flags)) {
+            RANGE_CONVERT_FUNCS(sse4);
+        }
+    }
+}
+#endif
+
 av_cold void ff_sws_init_swscale_x86(SwsContext *c)
 {
     int cpu_flags = av_get_cpu_flags();
@@ -820,4 +852,8 @@  switch(c->dstBpc){ \
     }
 
 #endif
+
+#if ARCH_X86_64
+    ff_sws_init_range_convert_x86(c);
+#endif
 }