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

refine encode segment id map buffer #756

Merged
merged 3 commits into from
Jul 3, 2024
Merged

Conversation

XinfengZhang
Copy link
Contributor

attention: these change will break backward compatibility, the pre-assumption is no one is using them besides intel media driver and intel oneVPL runtime, if there are other customer, it should be risky
so, please help to check whether there are somebody are using segment id map buffer for avc or vp9 encoding.

va/va.h Show resolved Hide resolved
va/va_enc_av1.h Outdated
/**
* \brief AV1 Block Segmentation ID Buffer
*
* The application provides a buffer of VAEncMacroblockMapBufferType containing

Choose a reason for hiding this comment

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

Do we need to set size and num_elements in vaCreateBuffer to width and height as well as in vp9, respectively?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is a kind of backward combability issue, I want to unify them to a linear buffer. so, the question is: is there other user beside intel media driver , is there any application is using the older interface?

Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be a better solution if we keep the original behavior when calling vaCreateBuffer with VAEncMacroblockMapBufferType, and we define a new BufferType (e.g. named VAEncUnifiedSegmentationMapBufferType).
There are some benefits but libva will a little bit complicated.
We can keep all the backward compability and suggest/guide app-developers to switch to the new/unified interface.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so, we support both for VP9?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I would prefer that.
It would benefit the application maintenance effort.
However, it will make libva more complicated......

Choose a reason for hiding this comment

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

Can we make a decision here?

va/va_enc_av1.h Show resolved Hide resolved
@@ -670,7 +672,6 @@ typedef struct _VAEncPictureParameterBufferAV1 {
* 0: 16x16 block size, default value;
* 1: 32x32 block size;
* 2: 64x64 block size;
* 3: 8x8 block size.

Choose a reason for hiding this comment

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

Why is this removed?
For the consisency to vp9, should this be VA_SEGID_BLOCKXXX?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated.

va/va.h Show resolved Hide resolved
@XinfengZhang
Copy link
Contributor Author

@rosetta-jpn , updated. how about this version?

@XinfengZhang XinfengZhang force-pushed the seg branch 2 times, most recently from e7f68e0 to 8265d88 Compare June 20, 2024 12:46
va/va_enc_vp9.h Outdated Show resolved Hide resolved
va/va_enc_vp9.h Outdated Show resolved Hide resolved
/** \brief each segmentID represent a 64x64 block */
#define VA_SEGID_BLOCK_64X64 2
/** \brief each segmentID represent a 8x8 block */
#define VA_SEGID_BLOCK_8X8 3

Choose a reason for hiding this comment

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

nit: Shall we add VA_SEGID_BLOCK_4x4? AV1 is 4x4 seg map.

@rosetta-jpn
Copy link

Sorry for so delayed review.
LGTM % nit. Feel free to submit the patches.

XinfengZhang and others added 3 commits July 3, 2024 11:08
Signed-off-by: Yao, Leyu <[email protected]>
Signed-off-by: Zhang,Carl <[email protected]>
previous description limited the segment id block 8x8
it is not accurate and should support different block size which
could be set by picture parameter.

attention: this change will impact backward compatiblility,
considering no one is using it. we could ignore this side effect

Signed-off-by: Carl Zhang <[email protected]>
@XinfengZhang XinfengZhang merged commit 438398b into intel:master Jul 3, 2024
14 checks passed
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