OpenCV fix 14774 breaks cudacodec #2180

cudawarped · 2019-07-02T10:07:51Z

In opencv/opencv#14774 cap_ffmpeg_api.hpp was replaced with cap_ffmpeg_legacy_api.hpp and all of the cuda encoder decoder parts were removed from cap_ffmpeg_legacy_api.hpp, e.g.

/*
 * For CUDA decoder
 */

OPENCV_FFMPEG_API struct InputMediaStream_FFMPEG* create_InputMediaStream_FFMPEG(const char* fileName, int* codec, int* chroma_format, int* width, int* height);
OPENCV_FFMPEG_API void release_InputMediaStream_FFMPEG(struct InputMediaStream_FFMPEG* stream);
OPENCV_FFMPEG_API int read_InputMediaStream_FFMPEG(struct InputMediaStream_FFMPEG* stream, unsigned char** data, int* size, int* endOfFile);

typedef struct InputMediaStream_FFMPEG* (*Create_InputMediaStream_FFMPEG_Plugin)(const char* fileName, int* codec, int* chroma_format, int* width, int* height);
typedef void (*Release_InputMediaStream_FFMPEG_Plugin)(struct InputMediaStream_FFMPEG* stream);
typedef int (*Read_InputMediaStream_FFMPEG_Plugin)(struct InputMediaStream_FFMPEG* stream, unsigned char** data, int* size, int* endOfFile);

additionaly the CUDA encoder/decoder implementation was removed from cap_ffmpeg_impl.hpp
meaning that simply updating the include in precomp.hpp fails when compiling cudacodec.

I don't have a deep understanding of the intention of the changes in #14774 or the recommended way that all the pieces of OpenCV fit together so I have submitted this hack which I used to get things working on my system as a PR to further explain the problem, and get some guidance as to the correct solution.

opencv=ffmpeg_raw_retrieve
force_builders=Custom
buildworker:Custom=linux-4
docker_image:Custom=ubuntu-cuda:16.04

alalek

Thank you for contribution.

Please ensure that this can be built without FFmpeg (cmake -DWITH_FFMPEG=OFF ...).

alalek · 2019-07-02T10:44:01Z

modules/cudacodec/src/cuvid_ffmpeg_api.hpp

@@ -0,0 +1,49 @@
+#ifndef __OPENCV_CUVID_FFMPEG_API_H__


Please add short license header (here and for other files): https://github.com/opencv/opencv/wiki/Coding_Style_Guide#file-structure

// This file is part of OpenCV project. // It is subject to the license terms in the LICENSE file found in the top-level directory // of this distribution and at http://opencv.org/license.html.

alalek · 2019-07-02T10:44:37Z

modules/cudacodec/src/cuvid_ffmpeg_impl.hpp

+struct OutputMediaStream_FFMPEG
+{
+	bool open(const char* fileName, int width, int height, double fps);
+	void close();


Please replace tabs => 4 spaces

alalek · 2019-07-02T10:45:07Z

modules/cudacodec/src/cuvid_ffmpeg_api.hpp

+
+	OPENCV_FFMPEG_API struct OutputMediaStream_FFMPEG* create_OutputMediaStream_FFMPEG(const char* fileName, int width, int height, double fps);
+	OPENCV_FFMPEG_API void release_OutputMediaStream_FFMPEG(struct OutputMediaStream_FFMPEG* stream);
+	OPENCV_FFMPEG_API void write_OutputMediaStream_FFMPEG(struct OutputMediaStream_FFMPEG* stream, unsigned char* data, int size, int keyFrame);


Please avoid indentation in namespace/extern "C" blocks.

alalek · 2019-07-02T10:45:52Z

modules/cudacodec/src/cuvid_ffmpeg_api.hpp

+//#elif defined __GNUC__ && __GNUC__ >= 4
+//#   define OPENCV_FFMPEG_API __attribute__ ((visibility ("default")))
+//#else
+//#   define OPENCV_FFMPEG_API


I believe OPENCV_FFMPEG_API modifier can be removed.

alalek · 2019-07-02T10:47:41Z

modules/cudacodec/CMakeLists.txt

@@ -13,6 +13,10 @@ ocv_glob_module_sources()

 set(extra_libs "")

+if(HAVE_FFMPEG)


Perhaps this should not work with HAVE_FFMPEG_WRAPPER (Windows)

alalek · 2019-07-02T10:50:27Z

modules/cudacodec/src/precomp.hpp

@@ -81,7 +81,7 @@
    #include "video_decoder.hpp"
    #include "video_parser.hpp"

-    #include "../src/cap_ffmpeg_api.hpp"
+    #include "cuvid_ffmpeg_api.hpp"


We need guard: #if defined(HAVE_FFMPEG) here and in places where this API is used.

cudawarped · 2019-07-03T17:19:54Z

I have added the changes but I am still unsure if I have used the correct preprocessor. What is the difference in purpose between WITH_FFMPEG and WITH_FFMPEG_WRAPPER?

I have also added a guard on the tests as the smaller video requires ffmpeg to demux the file. The guards are not properly implemented yet because I can't work out how to include HAVE_FFMPEG_WRAPPER, should it be as a preprocessor or cvconfig.h, or should I remove the guard and let the tests fail?

alalek · 2019-07-03T19:11:20Z

WITH_FFMPEG_WRAPPER

There is no such option/variable.

HAVE_FFMPEG - OpenCV can use FFmpeg SDK (headers/libs)
HAVE_FFMPEG_WRAPPER - OpenCV can't use FFmpeg, but it can load wrapper dynamically. Implies HAVE_FFMPEG option.

So your condition should be HAVE_FFMPEG && !HAVE_FFMPEG_WRAPPER (similar to that).
But, HAVE_FFMPEG_WRAPPER is not exposed via CMake to modules outside of videoio.

Also to use FFmpeg header/libs your should use proper dependency target (ocv.3rdparty.ffmpeg).
Are you sure that your cuvid_ffmpeg_impl.hpp code has been compiled on your side? I don't see any FFmpeg #include.
You mention file cap_ffmpeg_legacy_api.hpp, but it is missing in the patch.

cudawarped · 2019-07-04T07:16:46Z

Your correct, I wasn't correctly including cuvid_ffmpeg_impl.hpp and now I have, I realize everything depends on FFmpeg headers/libs.

I still am unsure as to why the cudacodec routines were removed in #14774 so I'm not entirely sure if I should be directly using FFmpeg headers/libs in cudacodec or somehow using the dynamic wrapper? If I can use the FFmpeg headers/libs what is the recommended way to include them in cudacodec? If not I will have to leave this and it may be an idea to just disable the FFmpeg fallback in cudacodec.

alalek · 2019-07-04T08:03:14Z

Dynamic wrapper doesn't contain CUDA-related code anymore.
Used classes should be bundled into cudacodec sources itself (and they depend/use FFmpeg SDK).

cudawarped · 2019-07-05T06:58:38Z

On reading the FFmpeg readme and this I have a clearer idea of what is going on.

Now the only issue is that the shared library on windows has changed from opencv_ffmpeg411_64.dll to opencv_videoio_ffmpeg411_64.dll and because the CUDA encoder/decoder implementation was removed from cap_ffmpeg_impl.hpp the symbols which allow direct access to the bit stream

create_InputMediaStream_FFMPEG
release_InputMediaStream_FFMPEG
read_InputMediaStream_FFMPEG

have been removed. Will these be included in the opencv_videoio_ffmpeg411_64.dll again now that I have re-included them in cuvid_ffmpeg_impl.hpp? Without them there is no fall back when cuvidCreateVideoSource() fails.

mshabunin · 2019-07-11T09:06:50Z

@cudawarped , current CUDA codec design is flawed, because it uses private methods and definitions of videoio module. It is better to add raw stream output support to VideoCapture to make it work instead of duplicating the code. Or, even better, new class: VideoContainer.

cudawarped · 2019-07-11T09:22:32Z

@mshabunin that sounds reasonable, I may not have time to implement this for a while, it was not my intention to implement a fix just to highlight the changes which broke cudacodec after 4.1.0, if someone else wants to take over so that cudacodec is not broken in 4.1.1 then feel free.

…raw encoded bitstream, from the videoio module instead of its own implementation.

cudawarped · 2019-10-14T08:22:07Z

@mshabunin everything should be working as well as it was before the break.

Ideally cudacodec should be updated to mirror the AppDec sample from Nvidia, adding support for a wider variety of color space formats etc. Unfortunately because cudacodec needs to work with all CUDA API's from the last 5 years mirroring the sample code and maintaining compatibility would be very difficult.

alalek · 2019-11-14T16:13:27Z

modules/cudacodec/include/opencv2/cudacodec.hpp

@@ -320,7 +323,7 @@ class CV_EXPORTS_W RawVideoSource
    @param size Size in bytes of current frame.
    @param endOfFile Indicates that it is end of stream.
     */
-    virtual bool getNextPacket(unsigned char** data, int* size, bool* endOfFile) = 0;
+    virtual bool getNextPacket(unsigned char** data, size_t* size, bool* endOfFile) = 0;


endOfFile is probably duplicated by return value. It is better to avoid behavior confusion and remove endOfFile.

I agree should I change this or are you happy to?

It would be nice if you can add commit for that.

commit added

alalek · 2019-11-14T16:18:43Z

modules/cudacodec/src/ffmpeg_video_source.cpp

+        chromaFormat = YUV420;
+        nBitDepthMinus8 = 0;
+        break;
+    default:


Feel free to add more cases (if you have corresponding videos)

I think your change makes everything clearer because I am pretty sure the historic version of AppDec which cudacodec is based on cannot deal with any formats other than YUV420. Therefore any other cases would be redundant.

alalek

Well done! Thank you 👍

Example to describe comilation issue

00e60f0

alalek reviewed Jul 2, 2019

View reviewed changes

Added required changes, builds with -DWITH_FFMPEG=ON|OFF

78518a1

Working with standard ffmpeg cap.

d1765e5

cudawarped mentioned this pull request Aug 13, 2019

Add retrieve encoded frame to VideoCapture opencv/opencv#15290

Merged

cudawarped added 5 commits August 13, 2019 11:52

Changed cudacodec to use new retrieveRaw() function, to retrieve the …

27171f6

…raw encoded bitstream, from the videoio module instead of its own implementation.

Fix cv::cudacodec::VideoWriter

455b913

Update to use VideoContainer

dea65cc

Remove header used in testing

1ea3306

Remove warning

f0b9ccd

cudawarped and others added 3 commits October 22, 2019 18:00

remove dependence on redundant ffmpeg codecs

413b4f6

cudacodec: use .set(CAP_PROP_FORMAT, -1) to extract RAW streams

3029009

whitespace

1660566

alalek reviewed Nov 14, 2019

View reviewed changes

addressed alalek's comment

f372b4b

alalek approved these changes Nov 18, 2019

View reviewed changes

alalek merged commit 53b9880 into opencv:master Nov 18, 2019

alalek mentioned this pull request Sep 5, 2020

is createVideoWriter not working on nvcuvid? opencv/opencv#18271

Closed

		@@ -13,6 +13,10 @@ ocv_glob_module_sources()

		set(extra_libs "")

		if(HAVE_FFMPEG)

OpenCV fix 14774 breaks cudacodec #2180

OpenCV fix 14774 breaks cudacodec #2180

Uh oh!

Conversation

cudawarped commented Jul 2, 2019 • edited by alalek Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alalek left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cudawarped commented Jul 3, 2019

Uh oh!

alalek commented Jul 3, 2019

Uh oh!

cudawarped commented Jul 4, 2019

Uh oh!

alalek commented Jul 4, 2019

Uh oh!

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

Uh oh!

Uh oh!

mshabunin commented Jul 11, 2019

Uh oh!

cudawarped commented Jul 11, 2019

Uh oh!

cudawarped commented Oct 14, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alalek left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cudawarped commented Jul 2, 2019 •

edited by alalek

Loading

cudawarped commented Jul 5, 2019 •

edited

Loading