Redesign of CL/VA interop initialization #22780

kallaballa · 2022-11-09T14:48:33Z

The pertaining issue: #22779

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

I agree to contribute to the project under Apache 2 License.
To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
The PR is proposed to the proper branch
There is a reference to the original bug report and related work
There is accuracy test, performance test and test data in opencv_extra repository, if applicable
Patch to opencv_extra has the same branch name.
The feature is well documented and sample code can be built with the project CMake

alalek

Thank you for the contribution!

I need a more time for launching of your code with reproducer (no working configuration with VA at this moment).

BTW, could you try to check your reproducer with OpenCV without PR #19755 (e.g. commit bb92eb5 )

modules/videoio/src/cap_ffmpeg_hw.hpp

modules/core/src/va_intel.cpp

kallaballa · 2022-11-17T09:54:40Z

I need a more time for launching of your code with reproducer (no working configuration with VA at this moment).

BTW, could you try to check your reproducer with OpenCV without PR #19755 (e.g. commit bb92eb5 )

Alright

kallaballa · 2022-11-19T21:01:35Z

I don't know what exactly happened. I force pushed to my own branch.. and then this

kallaballa · 2022-11-19T21:04:24Z

I pushed an empty branch which made it auto close. now things should be back to fine

kallaballa · 2022-11-19T21:05:53Z

I drafted a fallback-less version of VA initialization that does not work with my reproducer. 2875b0b
Same error. Trying to analyze the situation.

kallaballa · 2022-11-19T21:54:55Z

It after all still boils down to

opencv/modules/videoio/src/cap_ffmpeg_hw.hpp

Line 478 in 64aad34

va_intel::ocl::initializeContextFromVA(va_display);

being called twice and failing.

kallaballa · 2022-11-20T01:00:03Z

Apart from fixing an error in my system setup, I think i have the behavior like you wanted. That also required changes to my reproducer which now works (but is code-wise a little cumbersome). This needs a better design, especially in cases where different interop-systems are used at the same time.

Reproducer before (fails) : vaapi-decode-encode.cpp

Reproducer now (succeeds) : vaapi-decode-encode.cpp

Anyway as I mentioned above i have a proposal for how this could work, based on my 4.x fork: https://github.com/kallaballa/GCV/blob/main/src/video/video-demo.cpp

I isolated the dirty hack that made my demo work in a separate branch

Now... how to do it properly?

…ze the context automatically if it isn't already

kallaballa · 2022-11-22T09:13:54Z

Apart from fixing an error in my system setup, I think i have the behavior like you wanted. That also required changes to my reproducer which now works (but is code-wise a little cumbersome). This needs a better design, especially in cases where different interop-systems are used at the same time.

~~Reproducer before (fails) : vaapi-decode-encode.cpp~~

~~Reproducer now (succeeds) : vaapi-decode-encode.cpp~~

Reproducer works now as initially proposed, except now there isn't an interop fallback anymore: vaapi-decode-encode.cpp

Anyway as I mentioned above i have a proposal for how this could work, based on my 4.x fork: https://github.com/kallaballa/GCV/blob/main/src/video/video-demo.cpp

Also the demo would now work like that (except it requires #22704)

I isolated the dirty hack that made my demo work in a separate branch

Now... how to do it properly?

I moved cv::va_intel::VAAPIInterop into its own header so i can query for it like that and only initialize automatically if it isn't present (a254612): https://github.com/kallaballa/opencv/blob/a254612d65e385bc1f3882a576e9f39cf96d791e/modules/videoio/src/cap_ffmpeg_hw.hpp#L478-L487

On the other hand if the user decides to manually call initializeContextFromVA() multiple times to have different contexts, that is possible. But only if it doesn't use VideoCapture/VideoWriter

What do you think?

kallaballa · 2022-11-22T11:46:06Z

Simplified the reproducer

kallaballa · 2022-11-22T12:01:00Z

Updated the proposal/demo which demonstrates how CL/GL and CL/VA interop could be used together by copying and binding ocl_contexts as needed.

kallaballa · 2022-11-24T10:44:24Z

On the other hand if the user decides to manually call initializeContextFromVA() multiple times to have different contexts, that is possible. But only if it doesn't use VideoCapture/VideoWriter

Ensured by throwing an error on VADisplay mismatch: 497a536

alalek

Thank you for the updates!

Sorry for delay. I'm a busy till the end of this week.

modules/core/include/opencv2/core/va_intel_interop.hpp

modules/core/src/precomp.hpp

alalek · 2022-11-24T11:44:34Z

modules/core/src/va_intel.cpp

-    {
+    VAAPIInterop* interop = ocl_context.getContext().getUserContext<VAAPIInterop>().get();
+    if(!context_display || context_display != display)
+        CV_Error_(cv::Error::StsBadArg, ("Can't interop with current OpenCL context - VA display mismatch: %p (context) vs %p (surface).\ndid you call initializeContextFromVA before using VideoCapture/VideoWriter?", context_display, (void*)display));


VideoCapture/VideoWriter

?
We should not have this in generic functions.

Why is CV_Error()?

I think you are right about this. Should be debug? I'll test.

alalek · 2022-11-24T11:48:49Z

modules/videoio/src/cap_ffmpeg_hw.hpp

+            va_intel::ocl::initializeContextFromVA(va_display);
+        }
+    } else {
+        CV_LOG_DEBUG(NULL, "OpenCL/VA_INTEL: CL/VA interop already initialized. ")


If there is HW_ACCELERATION_USE_OPENCL=1 then we should initialize context anyway (as this is explicitly requested by user through dedicated property).

Is there a way to check if OpenCL context could serve for compatible (but different) VADisplays?

I try to find one but initializing anyway would lead to the situation we had with different VADisplays and no VA-capability

Also, when the user sets HW_ACCELERATION_USE_OPENCL=1 on both VideoCapture and VideoWriter in the same program tha would lead again to the situation with two different VADisplays.

So this wouldn't benefit from VA:

cv::VideoCapture capture("in.mkv", cv::CAP_FFMPEG, { cv::CAP_PROP_HW_DEVICE, VA_HW_DEVICE_INDEX, cv::CAP_PROP_HW_ACCELERATION, cv::VIDEO_ACCELERATION_VAAPI, cv::CAP_PROP_HW_ACCELERATION_USE_OPENCL, 1 }); cv::VideoWriter writer("out.mkv", cv::CAP_FFMPEG, cv::VideoWriter::fourcc('V', 'P', '9', '0'), fps, cv::Size(width, height), { cv::VIDEOWRITER_PROP_HW_ACCELERATION, cv::VIDEO_ACCELERATION_VAAPI, cv::VIDEOWRITER_PROP_HW_ACCELERATION_USE_OPENCL, 1 }); cv::UMat frame; capture >> frame; writer << frame;

full example: https://github.com/kallaballa/OpenCV-Issues/blob/main/src/vaapi-decode-encode/vaapi-decode-encode.cpp

kallaballa · 2023-01-14T10:17:15Z

I've experimented with partly parallel implementations of the WASM versions of my demos and the current VA initialization pattern is really in the way because the native implementation differs greatly from the WASM implementation and suffers from the above mentioned defects. It would be great if we could outline requirements on an initialization pattern so i can create a proper pull request and resolve this issue.

asmorkalov requested a review from alalek November 14, 2022 06:50

alalek reviewed Nov 15, 2022

View reviewed changes

modules/videoio/src/cap_ffmpeg_hw.hpp Outdated Show resolved Hide resolved

modules/core/src/va_intel.cpp Outdated Show resolved Hide resolved

kallaballa closed this Nov 19, 2022

kallaballa force-pushed the vaapi_encode_decode_at_the_same_time branch from d57a32a to 1ba0984 Compare November 19, 2022 21:00

no fallback paths for vaapi in initialization

2875b0b

kallaballa reopened this Nov 19, 2022

throw an error if VA-interop context can't be created

e538e3f

kallaballa added 2 commits November 20, 2022 02:05

fix doxygen warning

736afbb

move cv::va_intel::VAAPIInterop into its own header and only initiali…

a254612

…ze the context automatically if it isn't already

kallaballa added 2 commits November 22, 2022 10:42

Merge branch 'opencv:4.x' into vaapi_encode_decode_at_the_same_time

895bded

refactoring of if clauses

6eac43c

fixed indentation

da08802

kallaballa changed the title ~~Allow for diverging VADisplays~~ Redesign of CL/VA interop intitialization Nov 22, 2022

kallaballa changed the title ~~Redesign of CL/VA interop intitialization~~ Redesign of CL/VA interop initialization Nov 22, 2022

asmorkalov added this to the 4.7.0 milestone Nov 23, 2022

asmorkalov mentioned this pull request Nov 24, 2022

Implemented optional EGL based GL/CL interop #22704

Open

6 tasks

kallaballa force-pushed the vaapi_encode_decode_at_the_same_time branch from 7b723c5 to f161534 Compare November 24, 2022 10:42

throw an error on VADisplay mismatch

497a536

kallaballa force-pushed the vaapi_encode_decode_at_the_same_time branch from f161534 to 497a536 Compare November 24, 2022 10:57

alalek reviewed Nov 24, 2022

View reviewed changes

kallaballa added 4 commits November 26, 2022 23:32

move va_intel_interop.hpp out of precomp.hpp and into detail directory

b56bc32

move va_intel_interop.hpp out of precomp.hpp and into detail directory

5d8cc5d

silence shadowing warning

2457393

Merge branch 'opencv:4.x' into vaapi_encode_decode_at_the_same_time

86c012a

kallaballa mentioned this pull request Dec 6, 2022

V4D - A High performance visualization module with GUI #22923

Open

asmorkalov modified the milestones: 4.8.0, 4.9.0 Jun 23, 2023

asmorkalov modified the milestones: 4.9.0, 4.10.0 Dec 19, 2023

asmorkalov modified the milestones: 4.10.0, 4.11.0 Apr 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Redesign of CL/VA interop initialization #22780

Redesign of CL/VA interop initialization #22780

kallaballa commented Nov 9, 2022

alalek left a comment

kallaballa commented Nov 17, 2022

kallaballa commented Nov 19, 2022 •

edited

kallaballa commented Nov 19, 2022

kallaballa commented Nov 19, 2022 •

edited

kallaballa commented Nov 19, 2022

kallaballa commented Nov 20, 2022 •

edited

kallaballa commented Nov 22, 2022 •

edited

kallaballa commented Nov 22, 2022

kallaballa commented Nov 22, 2022 •

edited

kallaballa commented Nov 24, 2022 •

edited

alalek left a comment

alalek Nov 24, 2022

kallaballa Nov 26, 2022

alalek Nov 24, 2022

kallaballa Nov 26, 2022

kallaballa Nov 26, 2022

kallaballa Nov 26, 2022 •

edited

kallaballa commented Jan 14, 2023 •

edited

Redesign of CL/VA interop initialization #22780

Are you sure you want to change the base?

Redesign of CL/VA interop initialization #22780

Conversation

kallaballa commented Nov 9, 2022

Pull Request Readiness Checklist

alalek left a comment

Choose a reason for hiding this comment

kallaballa commented Nov 17, 2022

kallaballa commented Nov 19, 2022 • edited

kallaballa commented Nov 19, 2022

kallaballa commented Nov 19, 2022 • edited

kallaballa commented Nov 19, 2022

kallaballa commented Nov 20, 2022 • edited

kallaballa commented Nov 22, 2022 • edited

kallaballa commented Nov 22, 2022

kallaballa commented Nov 22, 2022 • edited

kallaballa commented Nov 24, 2022 • edited

alalek left a comment

Choose a reason for hiding this comment

alalek Nov 24, 2022

Choose a reason for hiding this comment

kallaballa Nov 26, 2022

Choose a reason for hiding this comment

alalek Nov 24, 2022

Choose a reason for hiding this comment

kallaballa Nov 26, 2022

Choose a reason for hiding this comment

kallaballa Nov 26, 2022

Choose a reason for hiding this comment

kallaballa Nov 26, 2022 • edited

Choose a reason for hiding this comment

kallaballa commented Jan 14, 2023 • edited

kallaballa commented Nov 19, 2022 •

edited

kallaballa commented Nov 19, 2022 •

edited

kallaballa commented Nov 20, 2022 •

edited

kallaballa commented Nov 22, 2022 •

edited

kallaballa commented Nov 22, 2022 •

edited

kallaballa commented Nov 24, 2022 •

edited

kallaballa Nov 26, 2022 •

edited

kallaballa commented Jan 14, 2023 •

edited