[ffmpeg] Update dependency support for recent changes. #21980

Sibras · 2021-12-11T05:56:19Z

Updates for changes in commits:
7bb175e [aom/libavif] Add support for ARM and UWP
a8204d9 [fribidi] Support new platform

Updates for changes in commits: 7bb175e [aom/libavif] Add support for ARM and UWP a8204d9 [fribidi] Support new platform

PhoebeHui

LGTM, thanks for your PR!

BillyONeal

I think these messages should just be deleted; they are implied by the dependencies and vcpkg will already print such messages long before attempting to build ffmpeg:

PS C:\Dev\vcpkg> .\vcpkg.exe install ffmpeg[aom]:arm-windows
Computing installation plan...
Error: aom[core] is only supported on '!(windows & arm & !uwp)'
PS C:\Dev\vcpkg>

Similarly, I believe supports expressions on features that exist only to accommodate dependencies should be removed.

Thanks for noticing the duplicate message!

Sibras · 2021-12-14T03:29:01Z

The supports 'platform' values in vcpkg.json are there to enable the 'all' feature to work correctly and the error messages were added to the portfile because at the time vcpkg wasn't generating error messages when unsupported features were enabled (#17153) and im not entirely sure if that situation has been resolved.

I should also point out that some of the errors in the portfile and not just due to the dependencies support requirements but also additional requirements of ffmpeg (i.e. vcpkg may be able to build a dependency with configuration x but ffmpeg doesnt support using it)

github-actions

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

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

Error: Local changes detected for ffmpeg but no changes to version or port version.
-- Version: 4.4.1#3
-- Old SHA: 00480edd2a451f2a3a55452779f524709ee52585
-- New SHA: ae9a4b22462b3aea1423c43cc24b4de746df8ab9
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

github-actions

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 772fe6cbce530cb3a5f0fee67b57e9861676e5d0 -- versions
./vcpkg x-add-version --all

Diff

diff --git a/versions/f-/ffmpeg.json b/versions/f-/ffmpeg.json
index e15e945..01ca59e 100644
--- a/versions/f-/ffmpeg.json
+++ b/versions/f-/ffmpeg.json
@@ -6,7 +6,7 @@
       "port-version": 4
     },
     {
-      "git-tree": "ae9a4b22462b3aea1423c43cc24b4de746df8ab9",
+      "git-tree": "00480edd2a451f2a3a55452779f524709ee52585",
       "version": "4.4.1",
       "port-version": 3
     },

PhoebeHui · 2021-12-24T09:23:12Z

ports/ffmpeg/portfile.cmake

 if("aom" IN_LIST FEATURES)
-    if (VCPKG_TARGET_ARCHITECTURE STREQUAL "arm" OR VCPKG_TARGET_ARCHITECTURE STREQUAL "arm64" OR VCPKG_TARGET_IS_UWP)
-        message(FATAL_ERROR "Feature 'aom' does not support 'uwp | arm'")
+    if ((VCPKG_TARGET_ARCHITECTURE STREQUAL "arm" OR VCPKG_TARGET_ARCHITECTURE STREQUAL "arm64") AND NOT VCPKG_TARGET_IS_UWP)
+        message(FATAL_ERROR "Feature 'aom' does not support 'windows & arm & !uwp'")
    endif()
 endif()


I think Billy means these messages should just be deleted, it looks doesn't support arm or arm64, which the dependency port aom already have the message.

PS F:\vcpkg\21980\vcpkg> ./vcpkg install ffmpeg[aom]:arm-windows Computing installation plan... Error: aom[core] is only supported on '!(windows & arm & !uwp)'

github-actions

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 772fe6cbce530cb3a5f0fee67b57e9861676e5d0 -- versions
./vcpkg x-add-version --all

Diff

diff --git a/versions/f-/ffmpeg.json b/versions/f-/ffmpeg.json
index e15e945..f51e031 100644
--- a/versions/f-/ffmpeg.json
+++ b/versions/f-/ffmpeg.json
@@ -1,12 +1,12 @@
 {
   "versions": [
     {
-      "git-tree": "f61b69b7b8d727e4caf45d7f39d9dcf77e620090",
+      "git-tree": "705505c43d32e7d62a0df59e506b8a3a81d742bd",
       "version": "4.4.1",
       "port-version": 4
     },
     {
-      "git-tree": "ae9a4b22462b3aea1423c43cc24b4de746df8ab9",
+      "git-tree": "00480edd2a451f2a3a55452779f524709ee52585",
       "version": "4.4.1",
       "port-version": 3
     },

BillyONeal · 2021-12-29T02:58:54Z

The supports 'platform' values in vcpkg.json are there to enable the 'all' feature to work correctly

That's not correct; feature resolution does not inspect "supports"

I should also point out that some of the errors in the portfile and not just due to the dependencies support requirements but also additional requirements of ffmpeg (i.e. vcpkg may be able to build a dependency with configuration x but ffmpeg doesnt support using it)

In such cases we can leave the message :). I just don't want ffmpeg to need to be updated every time whether one of its dependencies supports a given platform changes since it will inevitably create out-of-date messages.

Sibras · 2021-12-29T03:56:28Z

The supports 'platform' values in vcpkg.json are there to enable the 'all' feature to work correctly

That's not correct; feature resolution does not inspect "supports"

No, but it does check "platform" which is what I was referring to

I should also point out that some of the errors in the portfile and not just due to the dependencies support requirements but also additional requirements of ffmpeg (i.e. vcpkg may be able to build a dependency with configuration x but ffmpeg doesnt support using it)

In such cases we can leave the message :). I just don't want ffmpeg to need to be updated every time whether one of its dependencies supports a given platform changes since it will inevitably create out-of-date messages.

I can go through and delete everything thats not ffmpeg specific. I wasnt a big fan of adding that in the first place for just the reasons you mentioned.

BillyONeal · 2021-12-30T17:42:11Z

Thanks for the updated reporting and working with us on code review feedback!

In microsoft#21980 , we removed all the blocks for tensorflow which were merely replicating that upstream's "supports" expression. That is the correct behavior: if upstream adds support for a thing, it should start being tested downstream. However, the "all" and "all-nonfree" features of ffmpeg attempt to turn on what is really "all supported" rather than "all", so the feature-dependency needs to be guarded. Note that the ffmpeg[tensorflow] feature remains unguarded! It is ffmpeg[all]'s *dependency* on ffmpeg[tensorflow] that is guarded.

…64. (#22984) * [ffmpeg] Block "tensorflow" for the "all" feature on non-x64. In #21980 , we removed all the blocks for tensorflow which were merely replicating that upstream's "supports" expression. That is the correct behavior: if upstream adds support for a thing, it should start being tested downstream. However, the "all" and "all-nonfree" features of ffmpeg attempt to turn on what is really "all supported" rather than "all", so the feature-dependency needs to be guarded. Note that the ffmpeg[tensorflow] feature remains unguarded! It is ffmpeg[all]'s *dependency* on ffmpeg[tensorflow] that is guarded. * Also guard ass, tensorflow, and fontconfig for uwp

…64. (#22984) * [ffmpeg] Block "tensorflow" for the "all" feature on non-x64. In microsoft/vcpkg#21980 , we removed all the blocks for tensorflow which were merely replicating that upstream's "supports" expression. That is the correct behavior: if upstream adds support for a thing, it should start being tested downstream. However, the "all" and "all-nonfree" features of ffmpeg attempt to turn on what is really "all supported" rather than "all", so the feature-dependency needs to be guarded. Note that the ffmpeg[tensorflow] feature remains unguarded! It is ffmpeg[all]'s *dependency* on ffmpeg[tensorflow] that is guarded. * Also guard ass, tensorflow, and fontconfig for uwp

[ffmpeg] Update dependency support for recent changes.

b09025f

Updates for changes in commits: 7bb175e [aom/libavif] Add support for ARM and UWP a8204d9 [fribidi] Support new platform

github-actions bot approved these changes Dec 11, 2021

View reviewed changes

JonLiu1993 assigned PhoebeHui Dec 13, 2021

PhoebeHui added the category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist label Dec 13, 2021

PhoebeHui approved these changes Dec 13, 2021

View reviewed changes

PhoebeHui added the info:reviewed Pull Request changes follow basic guidelines label Dec 13, 2021

BillyONeal requested changes Dec 13, 2021

View reviewed changes

BillyONeal added requires:author-response and removed info:reviewed Pull Request changes follow basic guidelines labels Dec 13, 2021

PhoebeHui requested a review from BillyONeal December 17, 2021 02:11

Merge branch 'master' into master

2c885cf

github-actions bot requested changes Dec 23, 2021

View reviewed changes

update

0ee6a49

github-actions bot requested changes Dec 23, 2021

View reviewed changes

PhoebeHui reviewed Dec 24, 2021

View reviewed changes

[ffmpeg] update x264 supports.

0893063

github-actions bot requested changes Dec 29, 2021

View reviewed changes

BillyONeal added 2 commits December 28, 2021 18:55

Merge remote-tracking branch 'origin/master' into HEAD

1d256b8

Fix version database.

2afed80

github-actions bot approved these changes Dec 29, 2021

View reviewed changes

Sibras added 2 commits December 29, 2021 14:40

update

664bec2

Merge branch 'master' of https://github.com/Sibras/vcpkg

e7e10a2

github-actions bot approved these changes Dec 29, 2021

View reviewed changes

Sibras added 3 commits December 29, 2021 19:02

Update all platform checks and error messages

649184a

Update vcpkg.json

7d996ab

Update ffmpeg.json

e46507f

github-actions bot approved these changes Dec 29, 2021

View reviewed changes

fixup osx.

05c7b0f

github-actions bot approved these changes Dec 29, 2021

View reviewed changes

another osx fixup

9422f0a

github-actions bot approved these changes Dec 30, 2021

View reviewed changes

PhoebeHui removed the requires:author-response label Dec 30, 2021

PhoebeHui approved these changes Dec 30, 2021

View reviewed changes

PhoebeHui added the info:reviewed Pull Request changes follow basic guidelines label Dec 30, 2021

BillyONeal merged commit 25a3fb2 into microsoft:master Dec 30, 2021

dg0yt mentioned this pull request Feb 7, 2022

[OpenCV] upgrade to v4.5.5 #22801

Merged

BillyONeal mentioned this pull request Feb 7, 2022

[ffmpeg] Block "tensorflow" and others for the "all" feature on non-x64. #22984

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ffmpeg] Update dependency support for recent changes. #21980

[ffmpeg] Update dependency support for recent changes. #21980

Sibras commented Dec 11, 2021

PhoebeHui left a comment

BillyONeal left a comment

Sibras commented Dec 14, 2021

github-actions bot left a comment

github-actions bot left a comment

PhoebeHui Dec 24, 2021

github-actions bot left a comment

BillyONeal commented Dec 29, 2021

Sibras commented Dec 29, 2021

BillyONeal commented Dec 30, 2021

[ffmpeg] Update dependency support for recent changes. #21980

[ffmpeg] Update dependency support for recent changes. #21980

Conversation

Sibras commented Dec 11, 2021

PhoebeHui left a comment

Choose a reason for hiding this comment

BillyONeal left a comment

Choose a reason for hiding this comment

Sibras commented Dec 14, 2021

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

PhoebeHui Dec 24, 2021

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

BillyONeal commented Dec 29, 2021

Sibras commented Dec 29, 2021

BillyONeal commented Dec 30, 2021