[FFmpeg-devel] [PATCH] lavc/aarch64: hevc_sao reschedule slightly

[J. Dekker]

On 25 May 2022, at 12:23, Martin Storsjö wrote:

> On Wed, 25 May 2022, J. Dekker wrote:
>
>> Signed-off-by: J. Dekker <jdek at itanimul.li>
>> ---
>> libavcodec/aarch64/hevcdsp_sao_neon.S | 30 +++++++++++++++------------
>> 1 file changed, 17 insertions(+), 13 deletions(-)
>>
>> diff --git a/libavcodec/aarch64/hevcdsp_sao_neon.S b/libavcodec/aarch64/hevcdsp_sao_neon.S
>> index efd8112af4..39056d76ee 100644
>> --- a/libavcodec/aarch64/hevcdsp_sao_neon.S
>> +++ b/libavcodec/aarch64/hevcdsp_sao_neon.S
>> @@ -24,6 +24,10 @@
>>
>> #include "libavutil/aarch64/asm.S"
>>
>> +#define MAX_PB_SIZE 64
>> +#define AV_INPUT_BUFFER_PADDING_SIZE 64
>> +#define SAO_STRIDE (2*MAX_PB_SIZE + AV_INPUT_BUFFER_PADDING_SIZE)
>> +
>> // void sao_band_filter(uint8_t *_dst, uint8_t *_src,
>> //                      ptrdiff_t stride_dst, ptrdiff_t stride_src,
>> //                      int16_t *sao_offset_val, int sao_left_class,
>> @@ -56,6 +60,7 @@ function ff_hevc_sao_band_filter_8x8_8_neon, export=1
>>         // |xDE#xAD|xCA#xFE|xBE#xEF|xFE#xED|....
>>         // +----------------------------------->
>>         //    i-0     i-1     i-2     i-3
>> +        subs            w8, w8,  #8
>>         ld1             {v2.8b}, [x1], #8          // dst[x] = av_clip_pixel(src[x] + offset_table[src[x] >> shift]);
>
> For cases like this, it's usually better to place the subs after the ld1, because the ld1 will take a couple cycles before the result is available (which the next instruction needs).
>
>>         uxtl            v0.8h,  v2.8b              // load src[x]
>>         ushr            v2.8h,  v0.8h, #3          // >> BIT_DEPTH - 3
>> @@ -66,7 +71,7 @@ function ff_hevc_sao_band_filter_8x8_8_neon, export=1
>>         add             v1.8h,  v0.8h, v2.8h       // src[x] + table
>>         sqxtun          v4.8b,  v1.8h              // clip + narrow
>>         st1             {v4.8b}, [x0], #8          // store
>> -        subs            w8, w8,  #8                // done 8 pixels
>> +        // done 8 pixels
>>         bne             2b
>>         subs            w7, w7,  #1                // finished line, prep. new
>>         add             x0, x0,  x2                // dst += stride_dst
>> @@ -75,12 +80,11 @@ function ff_hevc_sao_band_filter_8x8_8_neon, export=1
>>         ret
>> endfunc
>>
>> -// ASSUMES STRIDE_SRC = 192
>> .Lsao_edge_pos:
>> .word 1 // horizontal
>> -.word 192 // vertical
>> -.word 192 + 1 // 45 degree
>> -.word 192 - 1 // 135 degree
>> +.word SAO_STRIDE // vertical
>> +.word SAO_STRIDE + 1 // 45 degree
>> +.word SAO_STRIDE - 1 // 135 degree
>>
>> // ff_hevc_sao_edge_filter_16x16_8_neon(char *dst, char *src, ptrdiff stride_dst,
>> //                                      int16 *sao_offset_val, int eo, int width, int height)
>> @@ -98,7 +102,7 @@ function ff_hevc_sao_edge_filter_16x16_8_neon, export=1
>>         uzp2            v1.16b, v3.16b, v3.16b     // sao_offset_val -> upper
>>         uzp1            v0.16b, v3.16b, v3.16b     // sao_offset_val -> lower
>>         movi            v2.16b, #2
>> -        mov             x15, #192
>> +        mov             x15, #SAO_STRIDE
>>         // strides between end of line and next src/dst
>>         sub             x15, x15, x5               // stride_src - width
>>         sub             x16, x2, x5                // stride_dst - width
>> @@ -108,6 +112,7 @@ function ff_hevc_sao_edge_filter_16x16_8_neon, export=1
>>         sub             x12, x11, x4               // src_a (prev) = src - sao_edge_pos
>>         add             x13, x11, x4               // src_b (next) = src + sao_edge_pos
>> 2:      // process 16 bytes
>> +        subs            x14, x14, #16
>>         ld1             {v3.16b}, [x11], #16       // load src
>>         ld1             {v4.16b}, [x12], #16       // load src_a (prev)
>>         ld1             {v5.16b}, [x13], #16       // load src_b (next)
>
> Same thing here, it's better to do the subs after firing off all loads.
>
>> @@ -130,12 +135,12 @@ function ff_hevc_sao_edge_filter_16x16_8_neon, export=1
>>         sqxtun          v3.8b, v20.8h
>>         sqxtun2         v3.16b, v21.8h
>>         st1             {v3.16b}, [x0], #16
>> -        subs            x14, x14, #16              // filtered 16 bytes
>> +        // filtered 16 bytes
>>         b.ne            2b                         // do we have width to filter?
>>         // no width to filter, setup next line
>> +        subs            w6, w6, #1                 // filtered line
>>         add             x11, x11, x15              // stride src to next line
>>         add             x0, x0, x16                // stride dst to next line
>> -        subs            w6, w6, #1                 // filtered line
>>         b.ne            1b                         // do we have lines to process?
>
> This looks good!
>
>>         // no lines to filter
>>         ret
>> @@ -156,12 +161,12 @@ function ff_hevc_sao_edge_filter_8x8_8_neon, export=1
>>         movi            v2.16b, #2
>>         add             x16, x0, x2
>>         lsl             x2,  x2, #1
>> -        mov             x15, #192
>> +        mov             x15, #SAO_STRIDE
>>         mov             x8,  x1
>>         sub             x9,  x1, x4
>>         add             x10, x1, x4
>> -        lsr             w17, w6, #1
>> -1:      ld1             {v3.d}[0], [ x8], x15
>> +1:      subs            w6, w6, #2
>> +        ld1             {v3.d}[0], [ x8], x15
>>         ld1             {v4.d}[0], [ x9], x15
>>         ld1             {v5.d}[0], [x10], x15
>>         ld1             {v3.d}[1], [ x8], x15
>
> Ditto, move the decrement after the loads.
>

Thanks, pushed with changes.

-- 
J. Dekker