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.

Sign up for GitHub

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

Jump to bottom

[libflac] Update to 1.3.3-1 #11152

Merged
merged 1 commit into from
May 21, 2020
Merged

[libflac] Update to 1.3.3-1 #11152

merged 1 commit into from
May 21, 2020

Conversation

evpobr
Copy link
Contributor

@evpobr evpobr commented May 4, 2020

Describe the pull request

  • What does your PR fix? Fixes issue #

  • Which triplets are supported/not supported? Have you updated the CI baseline?

  • Does your PR follow the maintainer guide?

@LilyWangL LilyWangL requested a review from PhoebeHui May 6, 2020 01:59
PhoebeHui
PhoebeHui requested changes May 6, 2020
View reviewed changes
Copy link
Contributor

@PhoebeHui PhoebeHui left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

The following regressions caused by the changes, could you have a look?
libflac:arm-uwp
libflac:arm64-windows
aubio:x64-windows-static

@evpobr
Copy link
Contributor Author

evpobr commented May 6, 2020

Hmm, erros logs are not very informative. Can i have more info?

@BillyONeal
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@PhoebeHui
Copy link
Contributor

@evpobr, see the failure, you can also full logs in attachments.
libflac:arm64-windows:
C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.25.28610\include\tmmintrin.h(14): fatal error C1189: #error: This header is specific to X86 and X64 targets

libflac:x64-uwp and libflac:arm-uwp:
C:\agent_work\1\s\buildtrees\libflac\src\34fa315125-b626d87b3a\src\utils\flactimer\main.cpp(120,2): error C3861: 'GetStartupInfoA': identifier not found [C:\agent_work\1\s\buildtrees\libflac\x64-uwp-dbg\src\utils\flactimer\flactimer.vcxproj]

aubio:x64-windows-static:
libsndfile.lib(flac.c.obj) : error LNK2019: unresolved external symbol __imp_FLAC__stream_decoder_new referenced in function flac_open
libsndfile.lib(flac.c.obj) : error LNK2019: unresolved external symbol __imp_FLAC__stream_decoder_delete referenced in function flac_close
libsndfile.lib(flac.c.obj) : error LNK2019: unresolved external symbol __imp_FLAC__stream_decoder_set_metadata_respond_all referenced in function flac_open
libsndfile.lib(flac.c.obj) : error LNK2019: unresolved external symbol
...
install-x64-uwp-dbg-out.log
install-x64-windows-static-rel-out.log
install-arm64-windows-dbg-out.log

@evpobr evpobr force-pushed the update-port-libflac-v1.3.3-1 branch from f55291c to 759c2d8 Compare May 9, 2020 14:02
@evpobr evpobr marked this pull request as draft May 10, 2020 05:10
@evpobr
Copy link
Contributor Author

evpobr commented May 10, 2020

Bugs are fixed, waiting for xiph/flac#210.

@PhoebeHui PhoebeHui added the depends:upstream-changes Waiting on a change to the upstream project label May 14, 2020
@evpobr evpobr force-pushed the update-port-libflac-v1.3.3-1 branch from 0d3720f to b9962bf Compare May 17, 2020 07:04
@evpobr evpobr marked this pull request as ready for review May 17, 2020 09:00
@evpobr evpobr requested a review from PhoebeHui May 17, 2020 09:01
@evpobr
Copy link
Contributor Author

evpobr commented May 17, 2020

It is ready. Tests fail, don't know why.

@PhoebeHui PhoebeHui removed depends:upstream-changes Waiting on a change to the upstream project waiting for response labels May 21, 2020
@PhoebeHui
Copy link
Contributor

PhoebeHui commented May 21, 2020 •
edited
Loading

@evpobr, the previous failures in CI doesn't relate to your change, it's an infrastructure issue that fixed currently, I have rerun the PR, let's wait for the results.

Thanks for your updates!

PhoebeHui
PhoebeHui approved these changes May 21, 2020
View reviewed changes
Copy link
Contributor

@PhoebeHui PhoebeHui left a comment

Choose a reason for hiding this comment

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

The CI test pass!

@PhoebeHui PhoebeHui added the info:reviewed Pull Request changes follow basic guidelines label May 21, 2020
@strega-nil
Copy link
Contributor

Thanks @evpobr !

I just want to write this down for posterity: you may see that 1.3.3 and this commit are about 6 months apart, but the only changes made in between here are cleanups:

  • std::-ify a bunch of C library stuff
  • C-cast -> static_cast or reinterpret_cast
  • build system changes
  • unsigned -> uint32_t
  • removing UB in a shift
  • better support for powerPC and Windows
  • Change out cos(double) for cosf(float) in C
  • Correct %d to %u in printf

These are all completely benign, imo, so I'm okay with still calling this 1.3.3 with bugfixes.

@strega-nil strega-nil merged commit 875821c into microsoft:master May 21, 2020
@evpobr
Copy link
Contributor Author

evpobr commented May 22, 2020

@strega-nil

These are all completely benign, imo, so I'm okay with still calling this 1.3.3 with bugfixes.

The main goal was ce6dd6b - upstream CMakeLists.txt.

@strega-nil
Copy link
Contributor

@evpobr yeah, absolutely; just wanted to document the changes that made it in between 1.3.3 and ce6dd6b :)

@evpobr evpobr deleted the update-port-libflac-v1.3.3-1 branch May 23, 2020 08:11
@Christsnatcher Christsnatcher mentioned this pull request May 24, 2020
Closed
kevinlul added a commit to kevinlul/vcpkg that referenced this pull request Jul 11, 2020
@kevinlul
[sdl2-mixer] Fix FLAC symbol export when static linking
ee30a56 
Fix regression introduced by microsoft#11152
@kevinlul kevinlul mentioned this pull request Jul 11, 2020
Merged
strega-nil pushed a commit that referenced this pull request Jul 31, 2020
@kevinlul
[sdl2-mixer] Fix FLAC symbol export when static linking, add nativemi…
62d4320 
…di default feature (#12374)

* [sdl2-mixer] Fix FLAC symbol export when static linking

Fix regression introduced by #11152

* [sdl2-mixer] Add nativemidi default feature

Closes #10318

* [libsndfile] Revert to port version 8 as version 10

* [libflac] Use BUILD_SHARED_LIBS to properly export FLAC__NO_DLL via CMake

* [libflac] Force FLAC__NO_DLL in installed headers in static triplets

* [libflac] Modify headers on installation
hellozee pushed a commit to hellozee/vcpkg that referenced this pull request Sep 11, 2020
@kevinlul @hellozee
[sdl2-mixer] Fix FLAC symbol export when static linking, add nativemi…
8d90423 
…di default feature (microsoft#12374)

* [sdl2-mixer] Fix FLAC symbol export when static linking

Fix regression introduced by microsoft#11152

* [sdl2-mixer] Add nativemidi default feature

Closes microsoft#10318

* [libsndfile] Revert to port version 8 as version 10

* [libflac] Use BUILD_SHARED_LIBS to properly export FLAC__NO_DLL via CMake

* [libflac] Force FLAC__NO_DLL in installed headers in static triplets

* [libflac] Modify headers on installation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PhoebeHui PhoebeHui PhoebeHui approved these changes

Assignees

@PhoebeHui PhoebeHui

Labels
info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@evpobr @BillyONeal @PhoebeHui @strega-nil