-
Notifications
You must be signed in to change notification settings - Fork 5.8k
Fix opencv issue #22721 #3380
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix opencv issue #22721 #3380
Conversation
61ccc36
to
9b87bed
Compare
@@ -61,12 +61,6 @@ GUID EncodingProfileGuid(const EncodeProfile encodingProfile); | |||
GUID EncodingPresetGuid(const EncodePreset nvPreset); | |||
bool Equal(const GUID& g1, const GUID& g2); | |||
|
|||
EncoderParams::EncoderParams() : nvPreset(ENC_PRESET_P3), tuningInfo(ENC_TUNING_INFO_HIGH_QUALITY), encodingProfile(ENC_CODEC_PROFILE_AUTOSELECT), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are should be implementation in the #else
branch:
#else // !defined HAVE_NVCUVENC
... CV_Error(...NotImplemented...) implementation stubs for public API ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
createVideoWriter
has this else branch. With the changes parameters structure could be created correctly without linker issue, but createVideoWriter
raises the issue. I propose to merge the PR as is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved it to the header to mirror the way cudacodec::FormatInfo() is implemented.
Both structs can exist without the Nvidia Video Coded SDK being installed whether they should or not is a different matter. Do you want me to move the implementation of both into the .cpp and prevent their use when HAVE_NVCUVENC
and/or HAVE_NVCUVID
are not defined?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer to have config structures always available how it's done in PR.
Fix opencv/opencv#22721 (comment)
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.