`cudacodec::VideoReader`: fix nv12 to bgr/bgra/grey conversion #3468

cudawarped · 2023-04-05T14:50:25Z

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

asmorkalov · 2023-04-13T06:52:54Z

I see build issue with my CUDA 10.2 on Ubuntu:

      ^~~~~~~~~~~
In file included from /home/alexander/Projects/OpenCV/opencv_contrib/modules/cudacodec/src/precomp.hpp:57:0,
                 from /home/alexander/Projects/OpenCV/opencv_contrib/modules/cudacodec/src/video_reader.cpp:43:
/home/alexander/Projects/OpenCV/opencv_contrib/modules/cudacodec/src/video_reader.cpp:76:25: error: ‘nppiNV12ToBGR_709CSC_8u_P2C3R_Ctx’ was not declared in this scope
             nppSafeCall(nppiNV12ToBGR_709CSC_8u_P2C3R_Ctx(pSrc, decodedFrame.step, outFrame.data, outFrame.step, oSizeROI, nppStreamCtx));
                         ^
/home/alexander/Projects/OpenCV/opencv-master/modules/core/include/opencv2/core/private.cuda.hpp:162:52: note: in definition of macro ‘nppSafeCall’
 #define nppSafeCall(expr)  cv::cuda::checkNppError(expr, __FILE__, __LINE__, CV_Func)
                                                    ^~~~
/home/alexander/Projects/OpenCV/opencv_contrib/modules/cudacodec/src/video_reader.cpp:76:25: note: suggested alternative: ‘nppiNV12ToBGR_709HDTV_8u_P2C3R_Ctx’
             nppSafeCall(nppiNV12ToBGR_709CSC_8u_P2C3R_Ctx(pSrc, decodedFrame.step, outFrame.data, outFrame.step, oSizeROI, nppStreamCtx));

asmorkalov · 2023-04-13T07:04:32Z

modules/cudacodec/include/opencv2/cudacodec.hpp

@@ -329,6 +329,7 @@ struct CV_EXPORTS_W_SIMPLE FormatInfo
    CV_PROP_RW cv::Size targetSz;//!< Post-processed size of the output frame.
    CV_PROP_RW cv::Rect srcRoi;//!< Region of interest decoded from video source.
    CV_PROP_RW cv::Rect targetRoi;//!< Region of interest in the output frame containing the decoded frame.
+    CV_PROP_RW bool videoFullRangeFlag;//!< indicates if the black level, luma and chroma are using the full range 0-255.


The flag corresponds to BGR 709 HDTV (true) / RGB 709 CSC (false) conversion. I propose to use it naming or at least to define it in documentation string. Also, it makes sense to mention, that the flag is set automatically from stream parser.

The flag corresponds to BGR 709 HDTV (true) / RGB 709 CSC (false) conversion. I propose to use it naming or at least to define it in documentation string.

The flag indicates whether the luma and chroma samples are represented using the full range of 8 bit values (0-255) or a smaller range. From my understanding the limited range is the most common for video. This is applicable to BGRA, BGR and Gray conversion not just BGRA.

The name is to be consistent with the definition in Annex E of the ITU-T Specification (video_full_range_flag) and the name given internally in the Nvidia Video Codec SDK (videoFullRangeFlag). I can add these details to the documentation string but I think the name is valid?

Also, it makes sense to mention, that the flag is set automatically from stream parser.

Agreed.

Ok, let's presume naming, but add reference to BGR 709 HDTV / RGB 709 color spaces.

cudawarped · 2023-04-13T08:40:18Z

I see build issue with my CUDA 10.2 on Ubuntu:

      ^~~~~~~~~~~
In file included from /home/alexander/Projects/OpenCV/opencv_contrib/modules/cudacodec/src/precomp.hpp:57:0,
                 from /home/alexander/Projects/OpenCV/opencv_contrib/modules/cudacodec/src/video_reader.cpp:43:
/home/alexander/Projects/OpenCV/opencv_contrib/modules/cudacodec/src/video_reader.cpp:76:25: error: ‘nppiNV12ToBGR_709CSC_8u_P2C3R_Ctx’ was not declared in this scope
             nppSafeCall(nppiNV12ToBGR_709CSC_8u_P2C3R_Ctx(pSrc, decodedFrame.step, outFrame.data, outFrame.step, oSizeROI, nppStreamCtx));
                         ^
/home/alexander/Projects/OpenCV/opencv-master/modules/core/include/opencv2/core/private.cuda.hpp:162:52: note: in definition of macro ‘nppSafeCall’
 #define nppSafeCall(expr)  cv::cuda::checkNppError(expr, __FILE__, __LINE__, CV_Func)
                                                    ^~~~
/home/alexander/Projects/OpenCV/opencv_contrib/modules/cudacodec/src/video_reader.cpp:76:25: note: suggested alternative: ‘nppiNV12ToBGR_709HDTV_8u_P2C3R_Ctx’
             nppSafeCall(nppiNV12ToBGR_709CSC_8u_P2C3R_Ctx(pSrc, decodedFrame.step, outFrame.data, outFrame.step, oSizeROI, nppStreamCtx));

Apologies, it looks like it wasn't introduced until CUDA 11.0, I will add build guards for this. In my defence Nvidia's documentation doesn't mention this anywhere in the release notes, you have to manually go through the NPP documentation to see that it wasn't in CUDA 10.2 and was in CUDA 11.0, come on Nvidia!! I promise I will be more rigorous next time.

cudawarped · 2023-04-13T11:50:58Z

Updated and tested on Windows 11 built with VS2019 against CUDA toolkit 10.2 and the latest version of Nvidia Video Codec SDK (12.0). All cudacodec tests passing on RTX 3070 Ti.

asmorkalov · 2023-04-13T14:49:05Z

Works for me with CUDA 10.2 and Ubuntu 18.04.

asmorkalov · 2023-04-13T14:29:11Z

modules/cudacodec/src/video_reader.cpp

+            nppSafeCall(nppiNV12ToBGR_709HDTV_8u_P2C3R_Ctx(pSrc, decodedFrame.step, outFrame.data, outFrame.step, oSizeROI, nppStreamCtx));
+        else {
+#if (CUDART_VERSION < 11000)
+            CV_LOG_DEBUG(NULL, "Color reproduction may be inaccurate due CUDA version <= 11.0, for better results upgrade CUDA runtime or try ColorFormat::BGRA.");


I propose to make the check in constructor, and use LOG_INFO to not generate spam. M.b. could you also add own conversion kernel for BGR in the same way as BGRA?

I propose to make the check in constructor, and use LOG_INFO to not generate spam.

Unfortunately that information isn't available until a frame has been run through the decoder. This is something I've wanted to change for a while and was going to add in another PR. Should I add that change to this PR or simply remove the debug output which is not really necessary?

M.b. could you also add own conversion kernel for BGR in the same way as BGRA?

Actually I would prefer to remove the BGRA kernel. Much as I am not a fan of the NPP libs (buggy, performance oscillates depending on the CUDA version etc.), they should be the best option for performance on future hardware + they should get more user testing than the OpenCV CUDA modules.

asmorkalov · 2023-04-13T15:30:04Z

Let's stay it as is then. Let's merge things as is for now and tune the behavior later.

cudawarped changed the title ~~cudacodec: fix nv12 to bgr/bgra/grey conversion~~ cudacodec::VideoReader: fix nv12 to bgr/bgra/grey conversion Apr 5, 2023

cudawarped mentioned this pull request Apr 5, 2023

cudacodec::VideoReader: add full coded colour range test file opencv/opencv_extra#1051

Merged

cudawarped force-pushed the cudacodec_fix_colour_conversion branch from 2ea6f4a to 2b476dd Compare April 5, 2023 17:35

asmorkalov self-requested a review April 6, 2023 19:18

asmorkalov reviewed Apr 13, 2023

View reviewed changes

asmorkalov self-assigned this Apr 13, 2023

asmorkalov added the category: cuda label Apr 13, 2023

cudacodec: fix nv12 to bgr/bgra conversion

d996792

cudawarped force-pushed the cudacodec_fix_colour_conversion branch from 2b476dd to d996792 Compare April 13, 2023 11:48

asmorkalov reviewed Apr 13, 2023

View reviewed changes

asmorkalov approved these changes Apr 13, 2023

View reviewed changes

asmorkalov merged commit bd546fa into opencv:4.x Apr 17, 2023

cudawarped mentioned this pull request Apr 28, 2023

cudacodec::VideoReader - ensure FormatInfo.valid == true on return from constructor #3486

Merged

6 tasks

asmorkalov mentioned this pull request May 31, 2023

(5.x) Merge 4.x #3501

Merged

cudawarped mentioned this pull request Nov 22, 2024

cudacodec::Videoreader fix YUV color conversion #3828

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

`cudacodec::VideoReader`: fix nv12 to bgr/bgra/grey conversion #3468

`cudacodec::VideoReader`: fix nv12 to bgr/bgra/grey conversion #3468

Uh oh!

cudawarped commented Apr 5, 2023 •

edited

Loading

Uh oh!

asmorkalov commented Apr 13, 2023

Uh oh!

asmorkalov Apr 13, 2023

Uh oh!

cudawarped Apr 13, 2023 •

edited

Loading

Uh oh!

asmorkalov Apr 13, 2023

Uh oh!

cudawarped commented Apr 13, 2023

Uh oh!

cudawarped commented Apr 13, 2023

Uh oh!

asmorkalov commented Apr 13, 2023

Uh oh!

asmorkalov Apr 13, 2023

Uh oh!

cudawarped Apr 13, 2023

Uh oh!

asmorkalov commented Apr 13, 2023

Uh oh!

Uh oh!

cudacodec::VideoReader: fix nv12 to bgr/bgra/grey conversion #3468

cudacodec::VideoReader: fix nv12 to bgr/bgra/grey conversion #3468

Uh oh!

Conversation

cudawarped commented Apr 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Readiness Checklist

Uh oh!

asmorkalov commented Apr 13, 2023

Uh oh!

asmorkalov Apr 13, 2023

Choose a reason for hiding this comment

Uh oh!

cudawarped Apr 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

asmorkalov Apr 13, 2023

Choose a reason for hiding this comment

Uh oh!

cudawarped commented Apr 13, 2023

Uh oh!

cudawarped commented Apr 13, 2023

Uh oh!

asmorkalov commented Apr 13, 2023

Uh oh!

asmorkalov Apr 13, 2023

Choose a reason for hiding this comment

Uh oh!

cudawarped Apr 13, 2023

Choose a reason for hiding this comment

Uh oh!

asmorkalov commented Apr 13, 2023

Uh oh!

Uh oh!

`cudacodec::VideoReader`: fix nv12 to bgr/bgra/grey conversion #3468

`cudacodec::VideoReader`: fix nv12 to bgr/bgra/grey conversion #3468

cudawarped commented Apr 5, 2023 •

edited

Loading

cudawarped Apr 13, 2023 •

edited

Loading