Skip to content
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

[ffmpeg/opencv4] Update opencv4 feature dependency and remove ffmpeg feature 'postproc' from default feature #19358

Merged
merged 16 commits into from
Aug 13, 2021

Conversation

PhoebeHui
Copy link
Contributor

@PhoebeHui PhoebeHui commented Aug 4, 2021

Fixes #19334

Changes:

  1. Opencv4, opencv3 and opencv2 only requires feature ffmpeg[avcodec,avdevice,avformat,swresample,swscale] by default see https://github.com/opencv/opencv/blob/master/CMakeLists.txt#L1369, so we can update opencv4 to depend on these features instead
  2. Remove the 'postproc' from ffmpeg default feature: ffmpeg default feature 'postproc' depends on ffmpeg[gpl], so the gpl would be installed, that cause ffmpeg is configured to be with GPL license by default, See details in issue current opencv[ffmpeg] feature makes the whole opencv library to be GPL licensed? #19334

@PhoebeHui PhoebeHui added category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist info:internal This PR or Issue was filed by the vcpkg team. labels Aug 4, 2021
@PhoebeHui PhoebeHui marked this pull request as ready for review August 4, 2021 10:31
@JackBoosY JackBoosY added the info:reviewed Pull Request changes follow basic guidelines label Aug 5, 2021
@cenit
Copy link
Contributor

cenit commented Aug 5, 2021

why not also to opencv2?

@JackBoosY JackBoosY added requires:author-response and removed info:reviewed Pull Request changes follow basic guidelines labels Aug 6, 2021
@PhoebeHui
Copy link
Contributor Author

@cenit, thanks for your review! The opencv2 has been updated.

@PhoebeHui
Copy link
Contributor Author

PhoebeHui commented Aug 10, 2021

opecv4:x64-uwp and opencv4:arm-uwp failed with following failures, same bug reported by #19313, and this could be reproed on master locally.

D:\buildtrees\opencv4\src\4.5.2-755f235ba0.clean\modules\imgcodecs\src\grfmt_tiff.cpp(266,9): error C4996: 'tiff_dummy_namespace::uint32': libtiff type deprecated; please use corresponding C99 stdint.h type [D:\buildtrees\opencv4\x64-uwp-dbg\modules\imgcodecs\opencv_imgcodecs.vcxproj]
         D:\buildtrees\opencv4\src\4.5.2-755f235ba0.clean\modules\imgcodecs\src\grfmt_tiff.cpp(267,9): error C4996: 'tiff_dummy_namespace::uint16': libtiff type deprecated; please use corresponding C99 stdint.h type [D:\buildtrees\opencv4\x64-uwp-dbg\modules\imgcodecs\opencv_imgcodecs.vcxproj]
         D:\buildtrees\opencv4\src\4.5.2-755f235ba0.clean\modules\imgcodecs\src\grfmt_tiff.cpp(275,13): error C4996: 'tiff_dummy_namespace::uint16': libtiff type deprecated; please use corresponding C99 stdint.h type [D:\buildtrees\opencv4\x64-uwp-dbg\modules\imgcodecs\opencv_imgcodecs.vcxproj]
         D:\buildtrees\opencv4\src\4.5.2-755f235ba0.clean\modules\imgcodecs\src\grfmt_tiff.cpp(341,45): error C4996: 'tiff_dummy_namespace::uint16': libtiff type deprecated; please use corresponding C99 stdint.h type [D:\buildtrees\opencv4\x64-uwp-dbg\modules\imgcodecs\opencv_imgcodecs.vcxproj]
         D:\buildtrees\opencv4\src\4.5.2-755f235ba0.clean\modules\imgcodecs\src\grfmt_tiff.cpp(397,38): error C4996: 'tiff_dummy_namespace::uint16': libtiff type deprecated; please use corresponding C99 stdint.h type [D:\buildtrees\opencv4\x64-uwp-dbg\modules\imgcodecs\opencv_imgcodecs.vcxproj]
         D:\buildtrees\opencv4\src\4.5.2-755f235ba0.clean\modules\imgcodecs\src\grfmt_tiff.cpp(420,5): error C4996: 'tiff_dummy_namespace::uint16': libtiff type deprecated; please use corresponding C99 stdint.h type [D:\buildtrees\opencv4\x64-uwp-dbg\modules\imgcodecs\opencv_imgcodecs.vcxproj]
         D:\buildtrees\opencv4\src\4.5.2-755f235ba0.clean\modules\imgcodecs\src\grfmt_tiff.cpp(420,27): error C4996: 'tiff_dummy_namespace::uint16': libtiff type deprecated; please use corresponding C99 stdint.h type [D:\buildtrees\opencv4\x64-uwp-dbg\modules\imgcodecs\opencv_imgcodecs.vcxproj]
         D:\buildtrees\opencv4\src\4.5.2-755f235ba0.clean\modules\imgcodecs\src\grfmt_tiff.cpp(436,9): error C4996: 'tiff_dummy_namespace::uint16': libtiff type deprecated; please use corresponding C99 stdint.h type [D:\buildtrees\opencv4\x64-uwp-dbg\modules\imgcodecs\opencv_imgcodecs.vcxproj]
         D:\buildtrees\opencv4\src\4.5.2-755f235ba0.clean\modules\imgcodecs\src\grfmt_tiff.cpp(443,9): error C4996: 'tiff_dummy_namespace::uint16': libtiff type deprecated; please use corresponding C99 stdint.h type [D:\buildtrees\opencv4\x64-uwp-dbg\modules\imgcodecs\opencv_imgcodecs.vcxproj]
         D:\buildtrees\opencv4\src\4.5.2-755f235ba0.clean\modules\imgcodecs\src\grfmt_tiff.cpp(463,9): error C4996: 'tiff_dummy_namespace::uint32': libtiff type deprecated; please use corresponding C99 stdint.h type [D:\buildtrees\opencv4\x64-uwp-dbg\modules\imgcodecs\opencv_imgcodecs.vcxproj]
         D:\buildtrees\opencv4\src\4.5.2-755f235ba0.clean\modules\imgcodecs\src\grfmt_tiff.cpp(481,71): error C4996: 'tiff_dummy_namespace::uint32': libtiff type deprecated; please use corresponding C99 stdint.h type [D:\buildtrees\opencv4\x64-uwp-dbg\modules\imgcodecs\opencv_imgcodecs.vcxproj]
         D:\buildtrees\opencv4\src\4.5.2-755f235ba0.clean\modules\imgcodecs\src\grfmt_tiff.cpp(527,33): error C4996: 'tiff_dummy_namespace::uint32': libtiff type deprecated; please use corresponding C99 stdint.h type [D:\buildtrees\opencv4\x64-uwp-dbg\modules\imgcodecs\opencv_imgcodecs.vcxproj]
         D:\buildtrees\opencv4\src\4.5.2-755f235ba0.clean\modules\imgcodecs\src\grfmt_tiff.cpp(531,33): error C4996: 'tiff_dummy_namespace::uint32': libtiff type deprecated; please use corresponding C99 stdint.h type [D:\buildtrees\opencv4\x64-uwp-dbg\modules\imgcodecs\opencv_imgcodecs.vcxproj]
         D:\buildtrees\opencv4\src\4.5.2-755f235ba0.clean\modules\imgcodecs\src\grfmt_tiff.cpp(569,33): error C4996: 'tiff_dummy_namespace::uint32': libtiff type deprecated; please use corresponding C99 stdint.h type [D:\buildtrees\opencv4\x64-uwp-dbg\modules\imgcodecs\opencv_imgcodecs.vcxproj]
         D:\buildtrees\opencv4\src\4.5.2-755f235ba0.clean\modules\imgcodecs\src\grfmt_tiff.cpp(573,33): error C4996: 'tiff_dummy_namespace::uint32': libtiff type deprecated; please use corresponding C99 stdint.h type [D:\buildtrees\opencv4\x64-uwp-dbg\modules\imgcodecs\opencv_imgcodecs.vcxproj]

@cenit
Copy link
Contributor

cenit commented Aug 10, 2021

@PhoebeHui is this valid also for #18827? if so, it might be a tiff recent update which broke downstream ports using it, because it was not a problem when that pr was completed

@dg0yt
Copy link
Contributor

dg0yt commented Aug 10, 2021

Tiff changed its type system with 4.3.0 (to standard types!). I explicitly stated this in the update (#18187). That PR also include a fix to the single port which had the same issue: https://github.com/microsoft/vcpkg/pull/18187/files#diff-9693267cc3a0c0a25b5e19b706db8f4eee2ad2758d8645e5f3e3438ca98105bd

Summary:

+    if(MSVC)
+        target_compile_options(foo PRIVATE /wd4996)
+    endif()

@PhoebeHui
Copy link
Contributor Author

@cenit @dg0yt, thanks for your help!

@PhoebeHui PhoebeHui requested a review from JackBoosY August 11, 2021 06:49
ports/opencv2/vcpkg.json Outdated Show resolved Hide resolved
versions/baseline.json Outdated Show resolved Hide resolved
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

After committing all other changes, the version database must be updated
git add -u && git commit
git checkout be456649fd835f6d2d676b81886fb6a58d5b52da -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/o-/opencv2.json b/versions/o-/opencv2.json
index 36121fb..d983d36 100644
--- a/versions/o-/opencv2.json
+++ b/versions/o-/opencv2.json
@@ -5,11 +5,6 @@
       "version": "2.4.13.7",
       "port-version": 6
     },
-    {
-      "git-tree": "425a3fc01ea3038e0ecea76415545ce30c75453f",
-      "version": "2.4.13.4",
-      "port-version": 6
-    },
     {
       "git-tree": "50a5602ecab15fd1c36f0619a5fdbd4792eb1b23",
       "version-string": "2.4.13.7",

@JackBoosY JackBoosY added the info:reviewed Pull Request changes follow basic guidelines label Aug 11, 2021
@cenit
Copy link
Contributor

cenit commented Aug 11, 2021

again, just to be sure, isn't the same patch for tiff required also by opencv2 and not just opencv3 and opencv4?

@PhoebeHui
Copy link
Contributor Author

opencv2 doesn't support uwp, so I think we don't need to add that patch as well, @cenit, what do you think?

@cenit
Copy link
Contributor

cenit commented Aug 11, 2021

yes, sorry you're right. This is a problem specific to uwp (which i still have to investigate because it looks suspicious it is found only on uwp).
Might just be that warnings are treated as errors in uwp, and the patch in fact just disables that warning, now that i think about it...

@PhoebeHui
Copy link
Contributor Author

Yes. from https://github.com/microsoft/vcpkg/blob/master/ports/opencv2/portfile.cmake#L9, it looks opencv2 has other problems.

@cenit
Copy link
Contributor

cenit commented Aug 11, 2021

i was referring to the tiff uwp patch, but yes, opencv2 has other problems too on uwp, so no reason to patch it for now (and also, i don't know anyone using opencv2 on uwp, so no need to make it work in future too)

@PhoebeHui
Copy link
Contributor Author

@BillyONeal, could you help merge this PR?

@cenit
Copy link
Contributor

cenit commented Aug 12, 2021

can you please add the fix in this comment to this PR? @PhoebeHui

#19497 (comment)

# Conflicts:
#	versions/f-/ffmpeg.json
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

After committing all other changes, the version database must be updated
git add -u && git commit
git checkout d306ab43c77c28d2f1d002093db6d4386cd7b333 -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/f-/ffmpeg.json b/versions/f-/ffmpeg.json
index 83fe529..4fb6cbd 100644
--- a/versions/f-/ffmpeg.json
+++ b/versions/f-/ffmpeg.json
@@ -6,7 +6,7 @@
       "port-version": 14
     },
     {
-      "git-tree": "64bd86208925adf6521e3cf926734be757e2737d",
+      "git-tree": "26e3d87d6b3049b45355f36a34402b938d5b486d",
       "version": "4.4",
       "port-version": 13
     },

@BillyONeal
Copy link
Member

can you please add the fix in this comment to this PR? @PhoebeHui

#19497 (comment)

I'm not sure I understand, that comment seems to indicate that there's a broken hash somewhere but I tried this locally without asset caching and it passed (as in the download succeeded and hash check passed)

@cenit Do you want to block this change until that is applied? Thanks!

@cenit
Copy link
Contributor

cenit commented Aug 13, 2021

cuda feature had a hash change, it would be nice to archive opencv 4.5.2 with a proper hash before going to 4.5.3

@PhoebeHui
Copy link
Contributor Author

Updated! thanks @BillyONeal @dg0yt @cenit for your review!

@BillyONeal BillyONeal merged commit eb9cb29 into microsoft:master Aug 13, 2021
@BillyONeal
Copy link
Member

Thanks for your help folks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist info:internal This PR or Issue was filed by the vcpkg team. info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

current opencv[ffmpeg] feature makes the whole opencv library to be GPL licensed?
5 participants