Merged
Conversation
6 tasks
asmorkalov
reviewed
Oct 11, 2022
Contributor
|
The new branch with encoder produces a lot of warnings during build. Unfortunately we do not have CI build with CUDA right now. My build log (Ubuntu 18.04, CUDA 10.2): |
…ws ffmpeg.dll in perf tests.
Contributor
Author
Do you see the same runtime errrors as detailed in #3362 on your local build? |
Contributor
|
No, I have codec library version and kernel driver version conflict. It's miss configuration on my side and I plan to fix it for testing. Andrey works on CI configuration for CUDA support. |
asmorkalov
requested changes
Oct 13, 2022
Comment on lines
80
to
105
| enum COLOR_FORMAT_CV { | ||
| UNDEFINED = 0, | ||
| BGR = 1, //!< Default OpenCV color format. | ||
| RGB = 2, | ||
| BGRA = 3, | ||
| RGBA = 4, | ||
| GRAY = 5 | ||
| }; | ||
|
|
||
| enum SurfaceFormat | ||
| /** @brief Nvidia Video Codec SDK surface formats. | ||
| */ | ||
| enum ENC_BUFFER_FORMAT | ||
| { | ||
| SF_UYVY = 0, | ||
| SF_YUY2, | ||
| SF_YV12, | ||
| SF_NV12, | ||
| SF_IYUV, | ||
| SF_BGR, | ||
| SF_GRAY = SF_BGR | ||
| BF_UNDEFINED = 0x00000000, //!< Undefined buffer format. | ||
| BF_NV12 = 0x00000001, //!< Semi-Planar YUV [Y plane followed by interleaved UV plane]. | ||
| BF_YV12 = 0x00000010, //!< Planar YUV [Y plane followed by V and U planes]. | ||
| BF_IYUV = 0x00000100, //!< Planar YUV [Y plane followed by U and V planes]. | ||
| BF_YUV444 = 0x00001000, //!< Planar YUV [Y plane followed by U and V planes]. | ||
| BF_YUV420_10BIT = 0x00010000, //!< 10 bit Semi-Planar YUV [Y plane followed by interleaved UV plane]. Each pixel of size 2 bytes. Most Significant 10 bits contain pixel data. | ||
| BF_YUV444_10BIT = 0x00100000, //!< 10 bit Planar YUV444 [Y plane followed by U and V planes]. Each pixel of size 2 bytes. Most Significant 10 bits contain pixel data. | ||
| BF_ARGB = 0x01000000, //!< 8 bit Packed A8R8G8B8. This is a word-ordered format where a pixel is represented by a 32-bit word with B in the lowest 8 bits, G in the next 8 bits, R in the 8 bits after that and A in the highest 8 bits. | ||
| BF_ARGB10 = 0x02000000, //!< 10 bit Packed A2R10G10B10. This is a word-ordered format where a pixel is represented by a 32-bit word with B in the lowest 10 bits, G in the next 10 bits, R in the 10 bits after that and A in the highest 2 bits. | ||
| BF_AYUV = 0x04000000, //!< 8 bit Packed A8Y8U8V8. This is a word-ordered format where a pixel is represented by a 32-bit word with V in the lowest 8 bits, U in the next 8 bits, Y in the 8 bits after that and A in the highest 8 bits. | ||
| BF_ABGR = 0x10000000, //!< 8 bit Packed A8B8G8R8. This is a word-ordered format where a pixel is represented by a 32-bit word with R in the lowest 8 bits, G in the next 8 bits, B in the 8 bits after that and A in the highest 8 bits. | ||
| BF_ABGR10 = 0x20000000, //!< 10 bit Packed A2B10G10R10. This is a word-ordered format where a pixel is represented by a 32-bit word with R in the lowest 10 bits, G in the next 10 bits, B in the 10 bits after that and A in the highest 2 bits. | ||
| }; |
Contributor
There was a problem hiding this comment.
If I understand correctly, COLOR_FORMAT_CV and ENC_BUFFER_FORMAT has strong correlation. In case if user define OpenCV style color then particular buffer format should be used. It means, that 2 options are redundant and only OpenCV style format should be presumed as API. Also ARGB10, ABGR10 and other 10 bit options are not achievable with OpenCV and I do not see a reason to expose them as API.
Contributor
Author
There was a problem hiding this comment.
I wanted to add surface types
- for users who are familiar with Nvidia's Video Coding SDK (maybe unecessary) and
- because apart from
BF_ARGBandBF_BGRAthey are not the same format as that produced bycv::cvtColor.
I could add the 8 bit versions to the existing COLOR_FORMAT_CV enum and rename it to COLOR_FORMAT as
enum COLOR_FORMAT {
UNDEFINED = 0,
BGR = 1, //!< Default OpenCV color format.
RGB = 2,
BGRA = 3,
RGBA = 4,
GRAY = 5,
NV_NV12,
NV_YV12,
etc.
};
which would reduce the number of factory methods while allowing users who have manually created say a YV12 Mat/GpuMat to use the encoder.
Do you see any problem/issues with this approach?
| self.assert_true(cap.isOpened()) | ||
| ret, blankFrameOut = cap.read() | ||
| self.assert_true(ret and blankFrameOut.shape == blankFrameIn.download().shape) | ||
| except cv.error as e: |
Contributor
There was a problem hiding this comment.
The test does not remove the temporary file.
|
|
||
| if (writer.empty()) { | ||
| frameSz = frame.size(); | ||
| writer = cv::cudacodec::createVideoWriter(outputFile, frameSz, codec, fps, surfaceFormatNv, stream); |
Contributor
There was a problem hiding this comment.
The test does not remove temporary files.
Contributor
|
@cudawarped Thanks a lot for the contribution! I updated CUDA driver and tests work on Ubuntu 18.04 + CUDA 10.2 and latest video codec sdk. Several thoughts on the PR:
|
Contributor
Author
In
|
Contributor
|
I still see a lot of .h265 and .hevc files in tmp after test execution. |
Contributor
Author
|
Updated, sorry I forgot to add to the accuracy test, updated now. |
Contributor
|
Thanks a lot for the patch. I reviewed the model API and have couple of ideas:
|
…fic ones and use camel-case for enum names.
Contributor
Author
|
Agreed. I have changed Unfortunately this will brake any code which currently sets the output color format for |
asmorkalov
approved these changes
Oct 27, 2022
| */ | ||
| void load(const String& configFile); | ||
| /** @brief Saves parameters to config file. | ||
| /** @brief ColorFormat for the frame returned by VideoReader::nextFrame()/VideoReader::retrieve() or used to initialize a VideoWriter. |
Contributor
There was a problem hiding this comment.
BuildBot warning on documentation:
/build/precommit-contrib_docs/4.x/opencv_contrib/modules/cudacodec/include/opencv2/cudacodec.hpp:96: warning: explicit link request to 'retrieve()' could not be resolved
Please do not use slash
4 tasks
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
cudacodec::VideoWriterhas been broken for a while (since hevc encoding support was added in 2015).This PR adds raw video wrting (.h264 and .hevc) support back to
cudacodec::VideoWriter. Additionaly the modules for video decoding (NVCUVID) and/or encoding (NVCUVENC) can now be included seperately using combinations of the below flagsThe functions within
NvEncoder.cppandNvEncoderCuda.cppare taken from Nvidia's Video Codec SDK.Dependant on opencv/opencv#22615
Pull Request Readiness Checklist
See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request
Patch to opencv_extra has the same branch name.