[FFmpeg-devel] [PATCH] [v4] avcodec/vaapi_encode: add customized surface alignment

[FFmpeg-devel] [PATCH] [v4] avcodec/vaapi_encode: add customized surface alignment

[Araz Iusubov]

This commit fixes issues with AMD HEVC encoding. 
By default AMD hevc encoder asks for the alignment 64x16, while FFMPEG VAAPI has 16x16. 
Adding support for customized surface size from VASurfaceAttribAlignmentSize in VAAPI version 1.21.0

Signed-off-by: Araz Iusubov <Primeadvice@gmail.com>
---
 libavcodec/vaapi_encode.c   | 11 +++++++++++
 libavutil/hwcontext.h       |  7 +++++++
 libavutil/hwcontext_vaapi.c |  5 +++++
 3 files changed, 23 insertions(+)

diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
index 940f0678a5..2a74db23b1 100644
--- a/libavcodec/vaapi_encode.c
+++ b/libavcodec/vaapi_encode.c
@@ -2711,6 +2711,17 @@ static av_cold int vaapi_encode_create_recon_frames(AVCodecContext *avctx)
     av_log(avctx, AV_LOG_DEBUG, "Using %s as format of "
            "reconstructed frames.\n", av_get_pix_fmt_name(recon_format));
 
+    if (constraints->log2_alignment) {
+        ctx->surface_width = FFALIGN(avctx->width,
+                              1 << (constraints->log2_alignment & 0xf));
+        ctx->surface_height = FFALIGN(avctx->height,
+                              1 << ((constraints->log2_alignment & 0xf0) >> 4));
+        av_log(avctx, AV_LOG_VERBOSE, "Using customized alignment size "
+                "[%dx%d].\n",
+                (1 << (constraints->log2_alignment & 0xf)),
+                (1 << ((constraints->log2_alignment & 0xf0) >> 4)));
+    }
+
     if (ctx->surface_width  < constraints->min_width  ||
         ctx->surface_height < constraints->min_height ||
         ctx->surface_width  > constraints->max_width ||
diff --git a/libavutil/hwcontext.h b/libavutil/hwcontext.h
index bac30debae..1eb56aff78 100644
--- a/libavutil/hwcontext.h
+++ b/libavutil/hwcontext.h
@@ -465,6 +465,13 @@ typedef struct AVHWFramesConstraints {
      */
     int max_width;
     int max_height;
+
+    /**
+     * The frame width/height log2 alignment when available
+     * the lower 4 bits, width; another 4 bits, height
+     * (Zero is not applied, use the default value)
+     */
+    int log2_alignment;
 } AVHWFramesConstraints;
 
 /**
diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c
index 56d03aa4cd..6cda0fd811 100644
--- a/libavutil/hwcontext_vaapi.c
+++ b/libavutil/hwcontext_vaapi.c
@@ -294,6 +294,11 @@ static int vaapi_frames_get_constraints(AVHWDeviceContext *hwdev,
             case VASurfaceAttribMaxHeight:
                 constraints->max_height = attr_list[i].value.value.i;
                 break;
+#if VA_CHECK_VERSION(1, 21, 0)
+            case VASurfaceAttribAlignmentSize:
+                constraints->log2_alignment = attr_list[i].value.value.i;
+                break;
+#endif
             }
         }
         if (pix_fmt_count == 0) {
-- 
2.43.0.windows.1

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

Re: [FFmpeg-devel] [PATCH] [v4] avcodec/vaapi_encode: add customized surface alignment

[Anton Khirnov]

Quoting Araz Iusubov (2024-03-21 18:02:19)
> diff --git a/libavutil/hwcontext.h b/libavutil/hwcontext.h
> index bac30debae..1eb56aff78 100644
> --- a/libavutil/hwcontext.h
> +++ b/libavutil/hwcontext.h
> @@ -465,6 +465,13 @@ typedef struct AVHWFramesConstraints {
>       */
>      int max_width;
>      int max_height;
> +
> +    /**
> +     * The frame width/height log2 alignment when available
> +     * the lower 4 bits, width; another 4 bits, height
> +     * (Zero is not applied, use the default value)

Why this unnecessary complication? Just use two ints, or two uint8_t's
if you really need to save space, though that also seems unncessary as
this struct is allocated rarely and usually does not live long.

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

Re: [FFmpeg-devel] [PATCH] [v4] avcodec/vaapi_encode: add customized surface alignment

[Mark Thompson]

On 21/03/2024 17:02, Araz Iusubov wrote:
> This commit fixes issues with AMD HEVC encoding.
> By default AMD hevc encoder asks for the alignment 64x16, while FFMPEG VAAPI has 16x16.
> Adding support for customized surface size from VASurfaceAttribAlignmentSize in VAAPI version 1.21.0
> 
> Signed-off-by: Araz Iusubov <Primeadvice@gmail.com>
> ---
>   libavcodec/vaapi_encode.c   | 11 +++++++++++
>   libavutil/hwcontext.h       |  7 +++++++
>   libavutil/hwcontext_vaapi.c |  5 +++++
>   3 files changed, 23 insertions(+)
> 
> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
> index 940f0678a5..2a74db23b1 100644
> --- a/libavcodec/vaapi_encode.c
> +++ b/libavcodec/vaapi_encode.c
> @@ -2711,6 +2711,17 @@ static av_cold int vaapi_encode_create_recon_frames(AVCodecContext *avctx)
>       av_log(avctx, AV_LOG_DEBUG, "Using %s as format of "
>              "reconstructed frames.\n", av_get_pix_fmt_name(recon_format));
>   
> +    if (constraints->log2_alignment) {
> +        ctx->surface_width = FFALIGN(avctx->width,
> +                              1 << (constraints->log2_alignment & 0xf));
> +        ctx->surface_height = FFALIGN(avctx->height,
> +                              1 << ((constraints->log2_alignment & 0xf0) >> 4));
> +        av_log(avctx, AV_LOG_VERBOSE, "Using customized alignment size "
> +                "[%dx%d].\n",
> +                (1 << (constraints->log2_alignment & 0xf)),
> +                (1 << ((constraints->log2_alignment & 0xf0) >> 4)));
> +    }
> +
>       if (ctx->surface_width  < constraints->min_width  ||
>           ctx->surface_height < constraints->min_height ||
>           ctx->surface_width  > constraints->max_width ||
> diff --git a/libavutil/hwcontext.h b/libavutil/hwcontext.h
> index bac30debae..1eb56aff78 100644
> --- a/libavutil/hwcontext.h
> +++ b/libavutil/hwcontext.h
> @@ -465,6 +465,13 @@ typedef struct AVHWFramesConstraints {
>        */
>       int max_width;
>       int max_height;
> +
> +    /**
> +     * The frame width/height log2 alignment when available
> +     * the lower 4 bits, width; another 4 bits, height
> +     * (Zero is not applied, use the default value)
> +     */
> +    int log2_alignment;

What other users do you have in mind for this?  (Are you expecting hwupload to use it as well, say?)

If this is only used in VAAPI encode then I would suggest putting the query there rather than adding new library API for it.

Also agree with Anton that this form is unnecessarily confusing - just make it a normal field, don't pack it like this.  (And possibly drop the log2 as well?  That doesn't seem like it adds anything useful.)

>   } AVHWFramesConstraints;
>   
>   /**
> diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c
> index 56d03aa4cd..6cda0fd811 100644
> --- a/libavutil/hwcontext_vaapi.c
> +++ b/libavutil/hwcontext_vaapi.c
> @@ -294,6 +294,11 @@ static int vaapi_frames_get_constraints(AVHWDeviceContext *hwdev,
>               case VASurfaceAttribMaxHeight:
>                   constraints->max_height = attr_list[i].value.value.i;
>                   break;
> +#if VA_CHECK_VERSION(1, 21, 0)
> +            case VASurfaceAttribAlignmentSize:
> +                constraints->log2_alignment = attr_list[i].value.value.i;
> +                break;
> +#endif
>               }
>           }
>           if (pix_fmt_count == 0) {

Thanks,

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

Re: [FFmpeg-devel] [PATCH] [v4] avcodec/vaapi_encode: add customized surface alignment

[Araz]

>>On Fri, Mar 22, 2024 at 11:35 AM Anton Khirnov <anton@khirnov.net> wrote:
>>Why this unnecessary complication? Just use two ints, or two uint8_t's
>>if you really need to save space, though that also seems unncessary as
>>this struct is allocated rarely and usually does not live long.

https://github.com/intel/libva/pull/794
The alignment range for log2_size of 4bit is in range of
[2**0, 2**15] = [1, 32768] this is large enough, or other way is to
directly use the alignment value, however I would want it to be in
powers of 2 and not any number, to force this condition, the common
way is to use log2_size(), which is using less bits and forcing
the alignment to be in powers of 2.

>>On Sun, Mar 24, 2024 at 5:38 PM Mark Thompson <sw@jkqxz.net> wrote:
>>What other users do you have in mind for this?  (Are you expecting
hwupload to use it as well, say?)
>>
>>If this is only used in VAAPI encode then I would suggest putting the
query there rather than adding new library API for it.
>>
>>Also agree with Anton that this form is unnecessarily confusing - just
make it a normal field, don't pack it like this.  (And possibly drop the
log2 as well?  That doesn't seem like it adds anything useful.)

The limitation is for AMD HW, and it is not limited to VAAPI, and for other
HW encoding protocols this limitation also exists.
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] [v4] avcodec/vaapi_encode: add customized surface alignment

[Mark Thompson]

On 27/03/2024 11:14, Araz wrote:
>>> On Fri, Mar 22, 2024 at 11:35 AM Anton Khirnov <anton@khirnov.net> wrote:
>>> Why this unnecessary complication? Just use two ints, or two uint8_t's
>>> if you really need to save space, though that also seems unncessary as
>>> this struct is allocated rarely and usually does not live long.
> 
> https://github.com/intel/libva/pull/794
> The alignment range for log2_size of 4bit is in range of
> [2**0, 2**15] = [1, 32768] this is large enough, or other way is to
> directly use the alignment value, however I would want it to be in
> powers of 2 and not any number, to force this condition, the common
> way is to use log2_size(), which is using less bits and forcing
> the alignment to be in powers of 2.

It is not necessary to copy exactly the same field layout.

Are you sure that there is never a meaningful non-power-of-two-bytes case?

Given that this is defining new public API to libavutil we don't want to be artificially constrained to precisely what happens to be needed in this case.

>>> On Sun, Mar 24, 2024 at 5:38 PM Mark Thompson <sw@jkqxz.net> wrote:
>>> What other users do you have in mind for this?  (Are you expecting
> hwupload to use it as well, say?)
>>>
>>> If this is only used in VAAPI encode then I would suggest putting the
> query there rather than adding new library API for it.
>>>
>>> Also agree with Anton that this form is unnecessarily confusing - just
> make it a normal field, don't pack it like this.  (And possibly drop the
> log2 as well?  That doesn't seem like it adds anything useful.)
> 
> The limitation is for AMD HW, and it is not limited to VAAPI, and for other
> HW encoding protocols this limitation also exists.

Does that mean we need this in hwupload and other similar surface-creation places or not?

If this is only inside the VAAPI encoder for reconstructed frames then it doesn't seem useful to push the information into the public API.

Thanks,

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

Re: [FFmpeg-devel] [PATCH] [v4] avcodec/vaapi_encode: add customized surface alignment

[Araz]

> On Mon, Apr 1, 2024 at 9:33 PM Mark Thompson <sw@jkqxz.net> wrote:
> It is not necessary to copy exactly the same field layout.
> Are you sure that there is never a meaningful non-power-of-two-bytes case?
> Given that this is defining new public API to libavutil we don't want to
be artificially constrained to precisely what happens to be needed in this
case.
If you think copying the field is not preferred,  any other ways in your
mind? How to use the VA interface to query the surface alignment data?
_______________________________________________
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".