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 VAConfigAttribEncMaxTileRows and VAConfigAttribEncMaxTileCols #757

Merged
merged 1 commit into from
Oct 4, 2023

Conversation

sivileri
Copy link
Contributor

@sivileri sivileri commented Sep 22, 2023

Currently there is no way of exposing the driver tile_rows and tile_cols limitations for encoding with tiles. For codecs like AV1 there is a cap specifying the max number of tiles, but without differentiating cols/rows.

Different hardware may have restrictions which may need to be taken into account. More specifically, D3D12 encode exposes the maximum rows/cols and adding VAConfigAttribEncMaxTileRows/Cols allows for mapping that information to the VAOn12 driver.

Related vainfo change: intel/libva-utils#344

@dvrogozh @XinfengZhang

va/va.h Outdated Show resolved Hide resolved
@sivileri sivileri changed the title va: Add VAConfigAttribEncMaxTileRowsCols va: Add VAConfigAttribEncMaxTileRows and VAConfigAttribEncMaxTileCols Sep 27, 2023
va/va.h Outdated Show resolved Hide resolved
va/va.h Outdated Show resolved Hide resolved
sivileri added a commit to sivileri/libva-utils that referenced this pull request Sep 27, 2023
Copy link
Contributor

@dvrogozh dvrogozh left a comment

Choose a reason for hiding this comment

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

looks good

Currently there is no way of exposing the driver tile_rows and tile_cols limitations
for encoding with tiles. For codecs like AV1 there is a cap specifying the max number
of tiles, but without differentiating cols/rows.

Different hardware may have restrictions which may need to be taken into account.
More specifically, D3D12 encode exposes the maximum rows/cols and adding
VAConfigAttribEncMaxTileRows/Cols allows for mapping that information to the VAOn12 driver.

Signed-off-by: Sil Vilerino <[email protected]>
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.

one value is enough? 16 bit for each?

@sivileri
Copy link
Contributor Author

one value is enough? 16 bit for each?

@XinfengZhang thanks for taking a look. We've discussed this previously with @dvrogozh . It may be easier for the user to have separate values instead of having to deal with bit manipulation, there are other caps like VASurfaceAttribMaxWidth and VASurfaceAttribMaxHeight that are separate (but could be combined as low/high 16 bit too with [0..65536] each) or VAConfigAttribEncMaxRefFrames that is combined for L0/L1 in low/hight 16 bits, so not clear on any specific style/trend for these.

Do you think keeping the separate values would be okay for VAConfigAttribEncMaxTileRows and VAConfigAttribEncMaxTileCols in this PR and the related intel/libva-utils#344?

image

@dvrogozh
Copy link
Contributor

These attributes are not directly involved in bitstream packing. In such cases I think it's better to favor usability over compactness.

@sivileri
Copy link
Contributor Author

sivileri commented Oct 2, 2023

@XinfengZhang kindly pinging on this MR :)

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.

LGTM, VA_ATTRIB_NOT_SUPPORTED represent " no limitation" "comply with codec spec" right?

@XinfengZhang XinfengZhang merged commit 71ef7e0 into intel:master Oct 4, 2023
14 checks passed
sivileri added a commit to sivileri/libva-utils that referenced this pull request Oct 4, 2023
XinfengZhang pushed a commit to intel/libva-utils that referenced this pull request Oct 4, 2023
@sivileri sivileri deleted the add_enc_max_tile_cols_rows branch March 27, 2024 12:54
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.

3 participants