Add description of the return status for vaSyncSurface() #774

pengxin99 · 2023-11-27T10:03:02Z

this will indicate APP behavior when meet VA_STATUS_ERROR_DECODING_ERROR from vaSyncSurface

va/va.h

pengxin99 · 2023-12-15T07:26:05Z

Hi, @XinfengZhang @Jexu, have add more detail description, could you pls help to review this PR again?

XinfengZhang · 2024-01-09T07:52:14Z

va/va.h

+ *   and verify its validity and integrity.
+ * - \ref VA_STATUS_ERROR_OPERATION_FAILED: When the attempt to retrieve the status report fails or related work
+ *   on the current surface fails, this error is generated. In such cases, the client can proceed with subsequent
+ *   frames by ignore the error, but it may involve incorrect result like corruption.


it means "operation failed" could be ignored by application

VA_STATUS_ERROR_OPERATION_FAILED is caused by status report fail which cannot be used to determine whether the result is correct. So declare here that APP can ignore this error but may meet incorrect result.

And will add more detail like "To init new context to avoid protentional incorrect result."

XinfengZhang · 2024-01-09T07:52:47Z

va/va.h

+ *   on the current surface fails, this error is generated. In such cases, the client can proceed with subsequent
+ *   frames by ignore the error, but it may involve incorrect result like corruption.
+ * - \ref VA_STATUS_ERROR_HW_BUSY: This error indicates that synchronization is still in progress or GPU hang happens.
+ *   The client could call the function again within a period of time to wait hw execution completion.


same surfaceID again?

Yes, will add it

XinfengZhang · 2024-01-09T07:56:39Z

va/va.h

+ * - \ref VA_STATUS_ERROR_HW_BUSY: This error indicates that synchronization is still in progress or GPU hang happens.
+ *   The client could call the function again within a period of time to wait hw execution completion.
+ *   If hw execution could not complete successfully, gpu hang and reset may occur, in this case,
+ *   application could try to recover media context by initialing new one.


must recreate context? could application just ignore it?

I think the recreate context is must, VA_STATUS_ERROR_HW_BUSY indicate that HW reset happened, this need to recover media context. Media reset will not return this error.

XinfengZhang · 2024-01-09T07:57:46Z

va/va.h

+ * - \ref VA_STATUS_ERROR_DECODING_ERROR: This error is encountered when macroblock (MB) errors with
+ *   non-conformance input clips are detected during the decoding process. The application can proceed with
+ *   subsequent frames, but it is advised to be aware of potential corruption in the output.
+ *   Call vaQuerySurfaceError could get more details of mb error information.


difference comparing with VA_STATUS_ERROR_OPERATION_FAILED

XinfengZhang · 2024-01-09T08:06:44Z

I agree with the direction, provide hint to application , then application could handle correlated errors , ignore , or reset the context etc. .
but I suggest to clarify the difference. currently:
VA_STATUS_ERROR_DECODING_ERROR may caused by a bistream corruption. related with syntax
VA_STATUS_ERROR_HW_BUSY is about the HW reset , but looks could not distinguish , media reset or gpu reset. if media reset , it could be ignored directly. it also related with #714?

but operation failed is unclear.

pengxin99 · 2024-01-12T04:42:27Z

@XinfengZhang Thanks for the comments. and I will refine this PR for below two point:

VA_STATUS_ERROR_HW_BUSY is about the HW reset , but looks could not distinguish , media reset or gpu reset. if media reset , it could be ignored directly. it also related with #714?

The VA_STATUS_ERROR_HW_BUSY is only for HW reset, so cannot ignore this error, vaSyncSurface will return success if media reset happended.

but operation failed is unclear.

can see the below comment: #774 (comment)

dvrogozh · 2024-01-24T23:49:22Z

va/va.h

+ * - \ref VA_STATUS_ERROR_OPERATION_FAILED: When the attempt to retrieve the status report fails or related work
+ *   on the current surface fails, this error is generated. In such cases, the client can proceed with subsequent
+ *   frames by ignore the error, but it may involve incorrect result like corruption, MD5 mismatch or HW hang.
+ *   Initialize new context can avoid the potential issues.


Suggested change

* Initialize new context can avoid the potential issues.

* Initializing new context can avoid the potential issues.

dvrogozh · 2024-01-24T23:50:35Z

va/va.h

+ * - \ref VA_STATUS_ERROR_HW_BUSY: This error indicates that synchronization is still in progress or GPU hang happens.
+ *   The client could call the function again with the same render_target within a period of time to wait hw execution completion.
+ *   If hw execution could not complete successfully, gpu hang and reset may occur, in this case,
+ *   application could try to recover media context by initialing new one.


Suggested change

* application could try to recover media context by initialing new one.

* application could try to recover media context by initializing new one.

dvrogozh · 2024-01-24T23:51:54Z

va/va.h

+ * - \ref VA_STATUS_ERROR_DECODING_ERROR: This error is encountered when macroblock (MB) errors with
+ *   non-conformance input clips are detected during the decoding process. The application can proceed with
+ *   subsequent frames, but it is advised to be aware of potential corruption in the output.
+ *   Call vaQuerySurfaceError could get more details of mb error information.


Suggested change

* Call vaQuerySurfaceError could get more details of mb error information.

* Call vaQuerySurfaceError to get more details on mb error.

dvrogozh · 2024-01-24T23:55:17Z

va/va.h

@@ -4187,6 +4187,26 @@ Synchronization
 * This function blocks until all pending operations on the render target
 * have been completed.  Upon return it is safe to use the render target for a
 * different picture.
+ *
+ * Possible errors:
+ * - \ref VA_STATUS_ERROR_INVALID_CONTEXT: This error occurs when the function is provided with an invalid context.


This error occurs when the function is provided with an invalid context.

Function prototype does not have context at all. So, this error is inadequate ( VA_STATUS_ERROR_INVALID_DISPLAY seems more reasonable) or at least incorrectly described. VAStatus vaSyncSurface(VADisplay dpy, VASurfaceID render_target).

thanks for this advice, the media-driver will return VA_STATUS_ERROR_INVALID_CONTEXT instead of VA_STATUS_ERROR_INVALID_DISPLAY , so I only correct the comments here to make sure the API compatible with the old version.

dvrogozh · 2024-01-25T00:00:36Z

Also, please, squash commits. For this change there should be just 1 commit in the PR.

this will indicate APP behavior when meet VA_STATUS_ERROR_DECODING_ERROR from vaSyncSurface Signed-off-by: Pengxin Yuan <[email protected]>

pengxin99 · 2024-01-26T02:36:13Z

Also, please, squash commits. For this change there should be just 1 commit in the PR.

@dvrogozh Thanks for your review, and have refined this PR, could pls have a look again?

pengxin99 · 2024-02-20T02:24:18Z

Hi, @dvrogozh @XinfengZhang , Can you take another look at this PR if you have time?

dvrogozh

looks good

pengxin99 · 2024-03-06T08:34:32Z

Hi, @XinfengZhang , Could you pls take another look at this PR when you have time?

pengxin99 force-pushed the pengxin/add_comments_for_vasyncsurface branch from 79c3709 to 8ba1827 Compare November 27, 2023 10:06

pengxin99 changed the title ~~Add explaination of decoding error status for vaSyncSurfac~~ Add description of decoding error status for vaSyncSurface() Nov 27, 2023

pengxin99 force-pushed the pengxin/add_comments_for_vasyncsurface branch from 8ba1827 to 36407ae Compare November 27, 2023 11:08

Jexu reviewed Nov 29, 2023

View reviewed changes

va/va.h Outdated Show resolved Hide resolved

pengxin99 requested a review from Jexu December 11, 2023 10:23

pengxin99 force-pushed the pengxin/add_comments_for_vasyncsurface branch from 8b94cd0 to 35d5b95 Compare December 15, 2023 07:23

Jexu approved these changes Dec 18, 2023

View reviewed changes

pengxin99 changed the title ~~Add description of decoding error status for vaSyncSurface()~~ Add description of the return status for vaSyncSurface() Dec 21, 2023

XinfengZhang reviewed Jan 9, 2024

View reviewed changes

pengxin99 requested a review from XinfengZhang January 24, 2024 03:02

dvrogozh requested changes Jan 24, 2024

View reviewed changes

Add description of decoding error status for vaSyncSurface()

b0a8b63

this will indicate APP behavior when meet VA_STATUS_ERROR_DECODING_ERROR from vaSyncSurface Signed-off-by: Pengxin Yuan <[email protected]>

pengxin99 force-pushed the pengxin/add_comments_for_vasyncsurface branch from 3a201cd to b0a8b63 Compare January 26, 2024 02:32

pengxin99 requested a review from dvrogozh January 26, 2024 02:36

dvrogozh approved these changes Feb 27, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add description of the return status for vaSyncSurface() #774

Add description of the return status for vaSyncSurface() #774

pengxin99 commented Nov 27, 2023

pengxin99 commented Dec 15, 2023

XinfengZhang Jan 9, 2024

pengxin99 Jan 12, 2024

XinfengZhang Jan 9, 2024

pengxin99 Jan 12, 2024

XinfengZhang Jan 9, 2024

pengxin99 Jan 12, 2024

XinfengZhang Jan 9, 2024

XinfengZhang commented Jan 9, 2024

pengxin99 commented Jan 12, 2024 •

edited

Loading

dvrogozh Jan 24, 2024

pengxin99 Jan 26, 2024

dvrogozh Jan 24, 2024

pengxin99 Jan 26, 2024

dvrogozh Jan 24, 2024

pengxin99 Jan 26, 2024

dvrogozh Jan 24, 2024

pengxin99 Jan 26, 2024

dvrogozh commented Jan 25, 2024

pengxin99 commented Jan 26, 2024 •

edited

Loading

pengxin99 commented Feb 20, 2024

dvrogozh left a comment

pengxin99 commented Mar 6, 2024

	* Initialize new context can avoid the potential issues.
	* Initializing new context can avoid the potential issues.

	* application could try to recover media context by initialing new one.
	* application could try to recover media context by initializing new one.

	* Call vaQuerySurfaceError could get more details of mb error information.
	* Call vaQuerySurfaceError to get more details on mb error.

Add description of the return status for vaSyncSurface() #774

Are you sure you want to change the base?

Add description of the return status for vaSyncSurface() #774

Conversation

pengxin99 commented Nov 27, 2023

pengxin99 commented Dec 15, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

XinfengZhang commented Jan 9, 2024

pengxin99 commented Jan 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dvrogozh commented Jan 25, 2024

pengxin99 commented Jan 26, 2024 • edited Loading

pengxin99 commented Feb 20, 2024

dvrogozh left a comment

Choose a reason for hiding this comment

pengxin99 commented Mar 6, 2024

pengxin99 commented Jan 12, 2024 •

edited

Loading

pengxin99 commented Jan 26, 2024 •

edited

Loading