Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

va: add VASurfaceAttribAlignmentSize #794

Merged
merged 1 commit into from
Feb 18, 2024

Conversation

ruijingdong
Copy link
Contributor

Due to different HW implementation, the
surface could require a different alignment size
rather than the default ones, here introduces
a new VASurfaceAttribute, alignment size, which
contains two variables log2_width_alignment
and log2_height_alignment, each has 4 bits,
and the alignment needs to be left shifted
2**size from the application side.

The alignment is in the powers of 2 and
range in [20, ... 215] = [1, 2, 4, 8, ... 32768]

And this alignment should be met when creating
context as an add-on requirement. If not
implemented, the existing/default alignment
logic will be used.

@XinfengZhang
Copy link
Contributor

I am wondering what's the usage of this surface alignments usage:
currently, when you call vaCreateSurfaces, you will get a surface with default alignment--- this default alignment should already met the alignment request. right?
in another words, when you query the alignment request, you get a value 16, then when you create surface (480x270), you create it as (480x272). is it the expected behavior.
but current design, when you create a 480x270, it will give a 480x272 to you by default, what's the difference of these two behaviors?

@ruijingdong
Copy link
Contributor Author

I am wondering what's the usage of this surface alignments usage: currently, when you call vaCreateSurfaces, you will get a surface with default alignment--- this default alignment should already met the alignment request. right? in another words, when you query the alignment request, you get a value 16, then when you create surface (480x270), you create it as (480x272). is it the expected behavior. but current design, when you create a 480x270, it will give a 480x272 to you by default, what's the difference of these two behaviors?

The original issue has been discussed in
https://gitlab.freedesktop.org/mesa/mesa/-/issues/9196

This HEVC problem can be resolved by hacking below code in 
vaapi_encode_h265.c in FFmpeg.

from

ctx->surface_width  = FFALIGN(avctx->width, priv->min_cb_size);
ctx->surface_height = FFALIGN(avctx->height, priv->min_cb_size);

to

ctx->surface_width  = FFALIGN(avctx->width, 64);
ctx->surface_height = FFALIGN(avctx->height, 16);

That is due to the AMD HEVC encoder HW implementation requires 
the image_input surface dimension to be aligned with 64x16. And the surface_width 
and surface_height alignment seems nowhere to get. Considering this could be a 
general issue of different HW implementation constrains, we propose to have such
an interface so that this alignment information is more flexible to different
applications and HW implementations.

Thanks,
Ruijing

Copy link
Contributor

@XinfengZhang XinfengZhang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the change itself looks good to me, but do not touch the micro_version, thanks
it will be a part of 2.21.0, not 2.21.1

meson.build Outdated Show resolved Hide resolved
@ruijingdong ruijingdong force-pushed the surface_attribute_alignment branch from 1bccaf7 to b8e450f Compare February 7, 2024 15:15
@ruijingdong
Copy link
Contributor Author

Thanks for reviewing, anything I need to do from my side?

Due to different HW implementation, the
surface could require a different alignment size
rather than the default ones, here introduces
a new VASurfaceAttribute, alignment size, which
contains two variables log2_width_alignment
and log2_height_alignment, each has 4 bits,
and the alignment needs to be left shifted
2**size from the application side.

The alignment is in the powers of 2 and
range in [2**0, ... 2**15] = [1, 2, 4, 8, ... 32768]

And this alignment should be met when creating
context as an add-on requirement. If not
implemented, the existing/default alignment
logic will be used.

Signed-off-by: Ruijing Dong <[email protected]>
@XinfengZhang XinfengZhang force-pushed the surface_attribute_alignment branch from b8e450f to 8da2f8d Compare February 17, 2024 13:25
@XinfengZhang XinfengZhang merged commit 0e4c0e5 into intel:master Feb 18, 2024
14 checks passed
@ruijingdong ruijingdong deleted the surface_attribute_alignment branch February 18, 2024 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants