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

feat: port media/codecs to CMake #1143

Merged
merged 1 commit into from
Dec 16, 2022

Conversation

cadubentzen
Copy link
Contributor

@cadubentzen cadubentzen commented Nov 21, 2022

  • Compile media/codecs with CMake
  • Compile tests for media/codecs with CMake
  • Pass tests for media/codecs with CMake

Also:

  • Use the same clang-format version as the CI here

Related to #1047

@google-cla
Copy link

google-cla bot commented Nov 21, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@cadubentzen cadubentzen changed the title WIP: port media/codecs to CMake feat: port media/codecs to CMake Nov 21, 2022
packager/media/codecs/CMakeLists.txt Outdated Show resolved Hide resolved
packager/utils/bytes_to_string_view.h Outdated Show resolved Hide resolved
packager/utils/bytes_to_string_view.h Show resolved Hide resolved
@zdanek
Copy link
Contributor

zdanek commented Dec 8, 2022

@cadubentzen can we go with this PR? What is blocking us?
I depend on it in two PRs :)

@cadubentzen
Copy link
Contributor Author

@cadubentzen can we go with this PR? What is blocking us? I depend on it in two PRs :)

Hi @zdanek sorry for the delay here. At this point, only the corporate CLA is blocking it. I have already pinged folks internally to double-check it but unfortunately, it's not solved yet and I can't really speed it up :(

@zdanek
Copy link
Contributor

zdanek commented Dec 9, 2022

only the corporate CLA is blocking it

that's why I'm here as an individual playing at my own rules in spare time :)

But, yeah, I do understand. I still have a lot to do in mpd.
Cheers.

@joeyparrish
Copy link
Member

@cadubentzen, the CLA bot is happy with you, so that's good enough for me. Can you fix the copyright headers (or let me know that you don't mind me doing it)? I'm ready to merge this once that is resolved.

@cadubentzen
Copy link
Contributor Author

cadubentzen commented Dec 15, 2022

@cadubentzen, the CLA bot is happy with you, so that's good enough for me. Can you fix the copyright headers (or let me know that you don't mind me doing it)? I'm ready to merge this once that is resolved.

@joeyparrish Thanks, I just fixed the copyright headers. Could you please re-trigger CI here?

@cadubentzen cadubentzen marked this pull request as ready for review December 15, 2022 17:01
@joeyparrish
Copy link
Member

Looks like there are some build errors. At least one was a missing header. Can you please take a look?

@joeyparrish
Copy link
Member

D:\a\shaka-packager\shaka-packager\packager\media\codecs\h264_parser_unittest.cc(192,17): error C2220: the following warning is treated as an error [D:\a\shaka-packager\shaka-packager\build\packager\media\codecs\codecs_unittest.vcxproj]
D:\a\shaka-packager\shaka-packager\packager\media\codecs\h264_parser_unittest.cc(192,17): warning C4459: declaration of 'kSps' hides global declaration [D:\a\shaka-packager\shaka-packager\build\packager\media\codecs\codecs_unittest.vcxproj]
D:\a\shaka-packager\shaka-packager\packager\media\codecs\h264_parser_unittest.cc(17,15): message : see declaration of 'shaka::media::`anonymous-namespace'::kSps' [D:\a\shaka-packager\shaka-packager\build\packager\media\codecs\codecs_unittest.vcxproj]
D:\a\shaka-packager\shaka-packager\packager\media\codecs\h264_parser_unittest.cc(212,17): warning C4459: declaration of 'kSps' hides global declaration [D:\a\shaka-packager\shaka-packager\build\packager\media\codecs\codecs_unittest.vcxproj]
D:\a\shaka-packager\shaka-packager\packager\media\codecs\h264_parser_unittest.cc(17,15): message : see declaration of 'shaka::media::`anonymous-namespace'::kSps' [D:\a\shaka-packager\shaka-packager\build\packager\media\codecs\codecs_unittest.vcxproj]
D:\a\shaka-packager\shaka-packager\packager\media\codecs\h264_parser_unittest.cc(233,17): warning C4459: declaration of 'kSps' hides global declaration [D:\a\shaka-packager\shaka-packager\build\packager\media\codecs\codecs_unittest.vcxproj]
D:\a\shaka-packager\shaka-packager\packager\media\codecs\h264_parser_unittest.cc(17,15): message : see declaration of 'shaka::media::`anonymous-namespace'::kSps' [D:\a\shaka-packager\shaka-packager\build\packager\media\codecs\codecs_unittest.vcxproj]
D:\a\shaka-packager\shaka-packager\packager\media\codecs\h264_parser_unittest.cc(259,17): warning C4459: declaration of 'kSps' hides global declaration [D:\a\shaka-packager\shaka-packager\build\packager\media\codecs\codecs_unittest.vcxproj]
D:\a\shaka-packager\shaka-packager\packager\media\codecs\h264_parser_unittest.cc(17,15): message : see declaration of 'shaka::media::`anonymous-namespace'::kSps' [D:\a\shaka-packager\shaka-packager\build\packager\media\codecs\codecs_unittest.vcxproj]
  h265_byte_to_unit_stream_converter_unittest.cc
  h265_parser_unittest.cc
  h26x_bit_reader_unittest.cc
  hevc_decoder_configuration_record_unittest.cc
  hls_audio_util_unittest.cc
  nal_unit_to_byte_stream_converter_unittest.cc
D:\a\shaka-packager\shaka-packager\.\packager/media/formats/mp4/box_definitions_comparison.h(350,51): error C2220: the following warning is treated as an error [D:\a\shaka-packager\shaka-packager\build\packager\media\codecs\codecs_unittest.vcxproj]
D:\a\shaka-packager\shaka-packager\.\packager/media/formats/mp4/box_definitions_comparison.h(350,51): warning C4100: 'rhs': unreferenced formal parameter [D:\a\shaka-packager\shaka-packager\build\packager\media\codecs\codecs_unittest.vcxproj]
D:\a\shaka-packager\shaka-packager\.\packager/media/formats/mp4/box_definitions_comparison.h(349,51): warning C4100: 'lhs': unreferenced formal parameter [D:\a\shaka-packager\shaka-packager\build\packager\media\codecs\codecs_unittest.vcxproj]
D:\a\shaka-packager\shaka-packager\.\packager/media/formats/mp4/box_definitions_comparison.h(491,73): warning C4100: 'rhs': unreferenced formal parameter [D:\a\shaka-packager\shaka-packager\build\packager\media\codecs\codecs_unittest.vcxproj]
D:\a\shaka-packager\shaka-packager\.\packager/media/formats/mp4/box_definitions_comparison.h(491,46): warning C4100: 'lhs': unreferenced formal parameter [D:\a\shaka-packager\shaka-packager\build\packager\media\codecs\codecs_unittest.vcxproj]

@cadubentzen cadubentzen marked this pull request as draft December 15, 2022 21:22
@cadubentzen
Copy link
Contributor Author

@joeyparrish I'm also now checking in parallel with the CI builds at my fork. Will ping you once the build is green there so you can retry it here 🙂

@joeyparrish
Copy link
Member

See also #1157, which fixes the Linux release builds. (If you'll review it, I'll go ahead and merge it today.)

@cadubentzen cadubentzen marked this pull request as ready for review December 15, 2022 22:31
@cadubentzen
Copy link
Contributor Author

@joeyparrish it should be good to go now 🙂 Got a green CI here.

@joeyparrish
Copy link
Member

Running it again now. BTW, I am working on ways to speed up CI builds so you can have a tighter feedback loop in situations like this. It might take some time for me to get it right, but I'm working on it.

@joeyparrish joeyparrish merged commit e9bf0c6 into shaka-project:cmake Dec 16, 2022
@cadubentzen cadubentzen deleted the cmake-media-codecs branch December 16, 2022 11:03
@github-actions github-actions bot added the status: archived Archived and locked; will not be updated label Jul 24, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: archived Archived and locked; will not be updated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants