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

replace the reconstruct surface to be variant #915

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

XinfengZhang
Copy link
Contributor

replace the reconstruct surface to be variant surface for HEVC REXT encoding
it could make the difference between recon/raw to be transparent to application
pay attention , the surface should be not read or write.

Signed-off-by: XinfengZhang carl.zhang@intel.com

Copy link
Contributor

@fulinjie fulinjie left a comment

Choose a reason for hiding this comment

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

Hi @XinfengZhang, thanks for working this out.
Verified with AYUV and Y410 encoding, this recon methods for me without resize on application level.

However, previous concern about Y210 still existed, and we'd better handle Y210 as well.
(Verified, garbage still existed.)

@XinfengZhang
Copy link
Contributor Author

updated for Y210

DdiMediaUtil_LockMutex(&mediaCtx->SurfaceMutex);
uint32_t i;
//get current element heap and index
for(i = 0; i < mediaCtx->pSurfaceHeap->uiAllocatedHeapElements; i ++)
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can replace this segment with DdiMedia_GetVASurfaceIDFromSurface.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes

surfaceElement ++;
}
//if cant find
if(i == surface->pMediaCtx->pSurfaceHeap->uiAllocatedHeapElements)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it will be great if we move this condition checking to the beginning of the function, then we will no need to allocate the dstSurface.

@@ -298,6 +298,8 @@ typedef struct _DDI_MEDIA_SURFACE
uint8_t *pSystemShadow; // Shadow surface in system memory

uint32_t uiMapFlag;

uint32_t uiVariantFlag;
Copy link
Contributor

Choose a reason for hiding this comment

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

Are there any reserved purposes for this flag? If no, maybe bool is enough for it.

@fulinjie
Copy link
Contributor

@XinfengZhang Note that MSDK needs some related change for this, otherwise it'll break the current encoding path since the surface reallocation is handled inside driver now.

@XinfengZhang
Copy link
Contributor Author

@fulinjie updated the patch, could you help to have a try?
@onabiull could you help to check from MSDK side about this change?

@fulinjie
Copy link
Contributor

Verified with this patch applied on 7620ba2, works for me for both Y210 and AYUV/Y410.

DDI_CHK_RET(RegisterRTSurfaces(&(m_encodeCtx->RTtbl), DdiMedia_GetSurfaceFromVASurfaceID(mediaCtx, picParams->decoded_curr_pic.picture_id)), "RegisterRTSurfaces failed!");
surface = DdiMedia_GetSurfaceFromVASurfaceID(mediaCtx, picParams->decoded_curr_pic.picture_id);

if(m_encodeCtx->vaProfile == VAProfileHEVCMain444
Copy link
Contributor

Choose a reason for hiding this comment

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

Why no 420 10 bit here?

@XinfengZhang XinfengZhang force-pushed the recon_format branch 2 times, most recently from 44a9782 to 2608509 Compare May 28, 2020 08:51
replace the reconstruct surface to be variant surface for HEVC REXT encoding
it could make the difference between recon/raw to be transparent to application
pay attention , the surface should be not read or write.

Signed-off-by: XinfengZhang <carl.zhang@intel.com>
Signed-off-by: XinfengZhang <carl.zhang@intel.com>
@XinfengZhang XinfengZhang added Encode video encode related verifying PR: fix ready and verifying with build/test labels Jun 4, 2020
@saosipov
Copy link
Contributor

saosipov commented Jun 10, 2020

corresponding msdk change Intel-Media-SDK/MediaSDK#2151

@Sherry-Lin Sherry-Lin removed the verifying PR: fix ready and verifying with build/test label Aug 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Encode video encode related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants