Skip to content

Fallback to vaCreateImage + vaPutImage when vaDeriveImage fails#21538

Merged
opencv-pushbot merged 1 commit intoopencv:4.xfrom
edman007:fixup-21536
Feb 1, 2022
Merged

Fallback to vaCreateImage + vaPutImage when vaDeriveImage fails#21538
opencv-pushbot merged 1 commit intoopencv:4.xfrom
edman007:fixup-21536

Conversation

@edman007
Copy link
Copy Markdown
Contributor

Per intel docs for libva, when vaDeriveImage fails vaCreateImage +
vaPutImage should be tried. This is important as mesa with AMD HW
will always fail because the image is interlaced, but the HW
capability still exists

Fixes #21536

Pull Request Readiness Checklist

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

  • [ x ] I agree to contribute to the project under Apache 2 License.
  • [ x ] 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
  • [ x ] The PR is proposed to the proper branch
  • [ x ] 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.
  • [ x ] The feature is well documented and sample code can be built with the project CMake

@edman007 edman007 force-pushed the fixup-21536 branch 2 times, most recently from 4b99d7f to 038afdf Compare January 29, 2022 14:13
@edman007
Copy link
Copy Markdown
Contributor Author

Fixed looping through the array of formats to use the num_formats returned by libva, and I have tested this on mine, works just fine

Copy link
Copy Markdown
Member

@alalek alalek left a comment

Choose a reason for hiding this comment

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

Thank you for contribution!

convertToVASurface() workaround doesn't look valid.
We need to update data of the surface. Updating image is not enough as it doesn't update content of the original surface.

//try vaCreateImage + vaPutImage
//pick a format
int num_formats = vaMaxNumImageFormats(display);
VAImageFormat fmt_list[num_formats];
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you please to use std::vector<VAImageFormat> or cv::AutoBuffer<VAImageFormat> instead of dynamic allocated arrays?

Comment on lines +619 to +634
VAImageFormat *selected_format = &fmt_list[0];
for (int i = 0; i < num_formats; i++){
if (fmt_list[i].fourcc == VA_FOURCC_NV12 || fmt_list[i].fourcc == VA_FOURCC_YV12){
selected_format = &fmt_list[i];
break;
}
}

status = vaCreateImage(display, selected_format, size.width, size.height, &image);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What if selected_format is not "found"? Why do we fallback to the first one?

I believe we should emit error message as fast as possible.

@edman007 edman007 marked this pull request as draft January 31, 2022 18:34
…fails

Per intel docs for libva, when vaDeriveImage fails vaCreateImage +
  vaPutImage should be tried. This is important as mesa with AMD HW
  will always fail because the image is interlaced so a indirect
  method must be used to get the surface to/from and image

Fixes opencv#21536
@edman007
Copy link
Copy Markdown
Contributor Author

@alalek Updated it with the changes you suggested

  • Throw an error if num_formats <= 0
  • Use an std::vector for the formats
  • Throw an error if none of the supported formats are found

convertToVASurface() workaround doesn't look valid.
We need to update data of the surface. Updating image is not enough as it doesn't update content of the original surface.

That should be fixed now! It now uses vaPutImage or vaGetImage, and vaPutImage is after we write the data to the image. I have no idea why vaPutImage worked for me in my testing when it should have been vaGetImage, but I tested it in both directions and verified that I can modify it as a UMat and write it back and encode it with VA-API/FFmpeg

@edman007 edman007 marked this pull request as ready for review January 31, 2022 22:18
Copy link
Copy Markdown
Member

@alalek alalek left a comment

Choose a reason for hiding this comment

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

Thank you for contribution 👍

@opencv-pushbot opencv-pushbot merged commit a351e05 into opencv:4.x Feb 1, 2022
@alalek alalek mentioned this pull request Feb 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cv::va_intel::convertFromVASurface always fails on AMD devices using mesa

3 participants