[vaPutSurface] do not reuse previous vpp context #944
Conversation
|
seems it could not avoid race condition, why not create temp vpp context and destory it after execution ? if you want to avoid create/destroy frequently , you should not store it in mediaCtx |
you mean we should protect PutSurfaceContext ? |
|
@XinfengZhang do you think need to protect entire vaPutSurface call with some lock? |
There was a problem hiding this comment.
@xuguangxin For race condition, we have PutSurfaceRenderMutex/PutSurfaceSwapBufferMutex to protect it. Is that still not enough, did you meet any issue currently?
|
@Xiaogangli-intel , yes, it will reuse the previous vpp context. image following usecase: |
We usally use vaPutSurface in a renderer thread. It's different than vpp thread. Reuse the vpp context will introduce a thread race condition issue.
|
@XinfengZhang @Xiaogang-Li , protectted the vp context creation. please help review it again. thanks |
|
@XinfengZhang @Xiaogang-Li , are we ready to merge this? |
|
@XinfengZhang are we ready to merge this? |
|
@XinfengZhang , any feedback for this? |
|
@XinfengZhang @Xiaogang-Li any more suggestion on this? |
|
@XinfengZhang any more comment for this? |
| } | ||
| vpCtx = (PDDI_VP_CONTEXT)DdiMedia_GetContextFromContextID(ctx, mediaCtx->PutSurfaceContext, &ctxType); |
There was a problem hiding this comment.
Could we move these lines into mutex protection part considering here vphal is used? Thanks.
DdiMediaUtil_LockMutex(&mediaCtx->PutSurfaceRenderMutex);
eStatus = vpHal->Render(&renderParams);
if (MOS_FAILED(eStatus))
{
DdiMediaUtil_UnLockMutex(&mediaCtx->PutSurfaceRenderMutex);
mos_bo_unreference(drawable_bo);
return VA_STATUS_ERROR_OPERATION_FAILED;
}
DdiMediaUtil_UnLockMutex(&mediaCtx->PutSurfaceRenderMutex);
There was a problem hiding this comment.
Hi Furong,
Thanks for the review. It may not needed.
Above lock just protect us for context creation. Once it created, it only destroy at uninitialized time.
The DdiMedia_GetContextFromContextID will hold a lock, and later operation like "vpHal->Render(&renderParams);" is protected by lock too.
So the entire process is thread safe.
We usally use vaPutSurface in a renderer thread. It's different than vpp thread.
Reuse the vpp context will introduce a thread race condition issue.