`cudacodec::VideoReader` add reconfigure decoder functionality by cudawarped · Pull Request #3453 · opencv/opencv_contrib

cudawarped · 2023-03-03T10:21:05Z

Add decoder reconfiguration functionality on resolution change, see Nvidia Docs.

PR test is dependent on multi-resolution test video from opencv/opencv_extra#1047

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

👍 Well done! Tested with Ubuntu 18.04 and CUDA 10.2.

asmorkalov · 2023-03-20T07:11:36Z

@cudawarped I made experiments with cv::VideoCapture and ported your test. VideoCapture class behaves differently and changes frame resolution on the go. What if make the behavior optional?

cudawarped · 2023-03-20T09:41:12Z

@asmorkalov I just ran a quick test and VideoCapure maintains the same frame size and places the new smaller frame in a ROI starting at the top left corner, shown below.

As you can see VideoCapture copies the new smaller frame to this smaller ROI leaving the rest of the outpur frame the same as it was. To me this looks buggy.

Personally I like the resize option for the reasons mentioned in the Nvidia docs,

The API can be used in scenarios where the bitstream undergoes changes in resolution, for e.g. when the encoder (on server side) changes image resolution frequently to adhere to Quality of Service(QoS) constraints.

but I understand the importants of having the same behaviour accross OpenCV's video reading interfaces.

Therefore I can add default the behaviour to do the same thing as VideoCapture but first zeroing out the image (so it looks less buggy) or I could place the image in the centre and letterbox it, which would you prefer?

I am happy either way, the main reason for this PR was to avoid VideoReader hanging when there is a new call to cv::cudacodec::detail::VideoParser::HandleVideoSequence which is the case currently because the decoder is destroyed while it is being used in another thread.

asmorkalov · 2023-03-20T10:38:42Z

Let's do the following then:

You'll add option to CUDA branch to make the behavior controlled.
I'll take a look on VideoCapture back-ends. We need to ensure consistent behaviour across platforms.
I'll put the issue to our core meeting agenda to discuss the cases with other team members and define fault behaviour.

cudawarped · 2023-03-21T11:55:26Z

You'll add option to CUDA branch to make the behavior controlled.

Added a ResolutionChangeMode enum with default behaviour to produce the same as output as cv::VideoCapture + reduced the number of frames in the test file to 40, 20 at the higher and 20 at the lower resolution.

asmorkalov · 2023-06-06T15:31:33Z

modules/cudacodec/src/video_decoder.cpp

+    if (!decodeResChange)
+        return 1;
+


It should be before block with autolock.

I will take a look at this next week, I need to find the test footage I was using where the number of decode surfaces changed but the resolution didn't to check if this can be moved.

asmorkalov · 2023-06-06T15:37:25Z

modules/cudacodec/include/opencv2/cudacodec.hpp

 };

+/** @brief Output format for a decoded frame when the resolution of the source is reduced by the encoder.  In all cases the size of the output frame remains the same.
+* @param Default Use the approach adopted by cv::VideoCapture, i.e. maintain the same frame size by placing the smaller output in the top left corner.


I made experiment with your test stream and VideoCapture follows frame resolution according to video:

Frame size: 700x400 Resolution: 700x400 Frame size: 700x400 Resolution: 700x400 Frame size: 700x400 [swscaler @ 0x55a1797812e0] Slice parameters 0, 400 are invalid Resolution: 672x384 Frame size: 672x384 [swscaler @ 0x55a1797812e0] Slice parameters 0, 400 are invalid Resolution: 672x384 Frame size: 672x384 Resolution: 672x384 Frame size: 672x384 Resolution: 672x384 Frame size: 672x384 Resolution: 672x384

Frame size: cols & rows for Mat. Resolution: CAP_PROP_FRAME_WIDTH and CAP_PROP_FRAME_HEIGHT. IMHO, it should be default behavior for consistency.

This appears to depend on the version of FFMpeg. If I use the prebuilt windows plugin

-- FFMPEG: YES (prebuilt binaries)
-- avcodec: YES (58.134.100)
-- avformat: YES (58.76.100)
-- avutil: YES (56.70.100)
-- swscale: YES (5.9.100)
-- avresample: YES (4.0.0)

then I get the output you described, shown below

avcodec: 58.134.100

[700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [swscaler @ 00000220c64f4140] Slice parameters 0, 400 are invalid [672 x 384] [swscaler @ 00000220c64f4140] Slice parameters 0, 400 are invalid [672 x 384] [672 x 384] [672 x 384] [672 x 384] [672 x 384] [672 x 384] [672 x 384] [672 x 384] [672 x 384] [672 x 384] [672 x 384] [672 x 384] [672 x 384] [672 x 384] [672 x 384] [672 x 384] [672 x 384] [672 x 384] [672 x 384] [672 x 384] [672 x 384]

however when I link to a more recent version of FFmpeg, e.g.

-- FFMPEG: YES (find_package)
-- avcodec: YES (59.18.100)
-- avformat: YES (59.16.100)
-- avutil: YES (57.17.100)
-- swscale: YES (6.4.100)

I get the previous output I showed in the screen shot with consistent sizes and no

[swscaler @ 00000220c64f4140] Slice parameters 0, 400 are invalid

error message.

avcodec: 59.18.100

[700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400] [700 x 400]

IMHO, it should be default behavior for consistency

Unfortunately the output from the Nvidia decoder will always be the same resolution as the original/first decoded frame (700x400) because the reconfigure functionality is designed for QOS use where you would want to maintain the same output resolution.

What would you like me to do?

I propose to rename enum items and remove reference to cv::VideoCature as soon as it behaves slightly different depending on backend.

"default" -> "copy"

"QoS" -> "scale" or "upscale"
It makes behaviour more obvious even without documentation.

OK, just a thought would it be cleaner to remove ResolutionChangeMode completely and just default to QOS as the copy behaviour may not be a sensible default anymore (FFmpeg could change again so why copy them)?

Ok, works for me then.

asmorkalov

👍 Thanks a lot for the contribution!

asmorkalov · 2023-06-09T07:29:16Z

Tested manually with Ubuntu 18.04, CUDA 10.2 and GF 1080.

cudawarped mentioned this pull request Mar 3, 2023

cudacodec::VideoReader add multi resolution test video opencv/opencv_extra#1047

Merged

asmorkalov added the category: cuda label Mar 15, 2023

asmorkalov self-requested a review March 15, 2023 12:33

asmorkalov self-assigned this Mar 15, 2023

asmorkalov approved these changes Mar 15, 2023

View reviewed changes

asmorkalov approved these changes Mar 18, 2023

View reviewed changes

cudawarped force-pushed the cudacodec_add_reconfigure_decoder branch from 2c2e39e to f0fa5bb Compare March 21, 2023 11:45

cudawarped force-pushed the cudacodec_add_reconfigure_decoder branch from f0fa5bb to ea1a5be Compare April 17, 2023 09:54

asmorkalov requested changes Jun 6, 2023

View reviewed changes

cudacodec: add capacity to reconfigure decoder on resolution change

045a9b0

cudawarped force-pushed the cudacodec_add_reconfigure_decoder branch from ea1a5be to 045a9b0 Compare June 9, 2023 06:19

asmorkalov approved these changes Jun 9, 2023

View reviewed changes

asmorkalov merged commit 353e37e into opencv:4.x Jun 9, 2023

asmorkalov mentioned this pull request Jul 12, 2023

5.x merge 4.x #3527

Merged

Conversation

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

Uh oh!

Pull Request Readiness Checklist

Uh oh!

asmorkalov left a comment

Choose a reason for hiding this comment

Uh oh!

asmorkalov commented Mar 20, 2023

Uh oh!

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

Uh oh!

Uh oh!

asmorkalov commented Mar 20, 2023

Uh oh!

cudawarped commented Mar 21, 2023

Uh oh!

asmorkalov Jun 6, 2023

Choose a reason for hiding this comment

Uh oh!

cudawarped Jun 8, 2023

Choose a reason for hiding this comment

Uh oh!

asmorkalov Jun 6, 2023

Choose a reason for hiding this comment

Uh oh!

cudawarped Jun 8, 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 Jun 8, 2023

Choose a reason for hiding this comment

Uh oh!

cudawarped Jun 8, 2023

Choose a reason for hiding this comment

Uh oh!

asmorkalov Jun 8, 2023

Choose a reason for hiding this comment

Uh oh!

asmorkalov left a comment

Choose a reason for hiding this comment

Uh oh!

asmorkalov commented Jun 9, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cudawarped commented Mar 3, 2023 •

edited

Loading

cudawarped commented Mar 20, 2023 •

edited

Loading

cudawarped Jun 8, 2023 •

edited

Loading