Skip to content

Update to m137 #173

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

Open
wants to merge 44 commits into
base: m137_release
Choose a base branch
from
Open

Update to m137 #173

wants to merge 44 commits into from

Conversation

pblazej
Copy link

@pblazej pblazej commented Jun 12, 2025

I changed my strategy and merged old β†’ new, not as clear, but the amount of conflicts in the Obj-C++ part (especially around prefixing) made it unmanageable.

Status

Apple

  • 🟑 iOS - builds
  • 🟑 macOS - builds
  • 🟑 Catalyst - builds
  • 🟑 tvOS - builds
  • 🟑 xrOS - builds, requires 2.0
ld64.lld: error: ../../third_party/llvm-build/Release+Asserts/lib/clang/21/lib/darwin/libclang_rt.xros.a(addtf3.c.o) has version 2.2.0, which is newer than target minimum of 1.1.0

Android

  • πŸ”΄ trying to build, failing patches

Tests

  • unknown

fippo and others added 30 commits April 23, 2024 07:33
which had a 70 year offset (i.e. 2094 instead of 2024) which broke
the webrtc-internal stats graphs. A similar adjustment is done
for audio in audio/channel_receive.cc

BUG=webrtc:12529,chromium:336222282

(cherry picked from commit 7731380)

Change-Id: I0ce43cc8b451185bc056cf9e54757ef22d006c99
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/347780
Reviewed-by: Florent Castelli <[email protected]>
Commit-Queue: Philipp Hancke <[email protected]>
Reviewed-by: Harald Alvestrand <[email protected]>
Cr-Original-Commit-Position: refs/heads/main@{#42114}
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/348702
Commit-Queue: Harald Alvestrand <[email protected]>
Cr-Commit-Position: refs/branch-heads/6422@{#1}
Cr-Branched-From: b831eb8-refs/heads/main@{#42072}
This reverts commit 33cc835.

Reason for revert: Perf bots showed that this cl cause a change in metrics. It looks like it is for the better, but we want this to be behind a field trial.

Original change's description:
> Ignore allocated bitrate during initial exponential BWE.
>
> The reason why we want to do this is  because audio can allocate a needed bitrate before video when starting a call, which may lead to a race between the first probe result and updating the allocated bitrate.
> That is the, initial probe will try to probe up to the max configured bitrate.
>
> ProbeController::SetFirstProbeToMaxBitrate will allow the first probe to
> continue up to the max configured bitrate, regardless of of the max
> allocated bitrate.
>
> Bug: webrtc:14928
> Change-Id: I6e0ae90e21a78466527f3464951e6033dc846470
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/346760
> Reviewed-by: Diep Bui <[email protected]>
> Commit-Queue: Per Kjellander <[email protected]>
> Reviewed-by: Erik SprΓ₯ng <[email protected]>
> Reviewed-by: Per Kjellander <[email protected]>
> Cr-Commit-Position: refs/heads/main@{#42049}

(cherry picked from commit 501c4f3)

Bug: chromium:335337923,webrtc:14928
Change-Id: I56ba58560b6857b6069552c02df822691f7af64d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/347622
Bot-Commit: [email protected] <[email protected]>
Commit-Queue: Per Kjellander <[email protected]>
Reviewed-by: Diep Bui <[email protected]>
Owners-Override: Per Kjellander <[email protected]>
Cr-Original-Commit-Position: refs/heads/main@{#42081}
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/348722
Reviewed-by: Erik SprΓ₯ng <[email protected]>
Cr-Commit-Position: refs/branch-heads/6422@{#2}
Cr-Branched-From: b831eb8-refs/heads/main@{#42072}
Use M125 as the latest version and migrate historical patches to m125

Patches Group:

## 1. Update README.md
b6c65fc
* Add Apache-2.0 license and some note to README.md. (#9)
* Updated readme detailing changes from original (#42)
* Adding membrane framework (#51)
* Updated readme (#83)

## 2. Audio Device Optimization
7454824
* allow listen-only mode in AudioUnit, adjust when category changes
(#2)
* release mic when category changes
(#5)
* Change defaults to iOS defaults
(#7)
* Sync audio session config
(#8)
* feat: support bypass voice processing for iOS.
(#15)
* Remove MacBookPro audio pan right code
(#22)
* fix: Fix can't open mic alone when built-in AEC is enabled.
(#29)
* feat: add audio device changes detect for windows.
(#41)
* fix Linux compile (#47)
* AudioUnit: Don't rely on category switch for mic indicator to turn off
(#52)
* Stop recording on mute (turn off mic indicator)
(#55)
* Cherry pick audio selection from m97 release
(#35)
* [Mac] Allow audio device selection
(#21)
* RTCAudioDeviceModule.outputDevice / inputDevice getter and setter
(#80)
* Allow custom audio processing by exposing AudioProcessingModule
(#85)
* Expose audio sample buffers for Android
(#89)
* feat: add external audio processor for android.
(#103)
* android: make audio output attributes modifiable
(#118)
* Fix external audio processor sample rate calculation
(#108)
* Expose remote audio sample buffers on RTCAudioTrack
(#84)
* Fix memory leak when creating audio CMSampleBuffer
#86

## 3. Simulcast/SVC support for iOS/Android.
b0b9fe9
    
- Simulcast support for iOS SDK (#4)
- Support for simulcast in Android SDK (#3)
- include simulcast headers for mac also (#10)
- Fix simulcast using hardware encoder on Android (#48)
- Add scalabilityMode support for AV1/VP9. (#90)

## 4. Android improvements.
9aaaab5
- Start/Stop receiving stream method for VideoTrack (#25)
- Properly remove observer upon deconstruction (#26)
- feat: Expose setCodecPreferences/getCapabilities for android. (#61)
- fix: add WrappedVideoDecoderFactory.java. (#74)

## 5. Darwin improvements
a13ea17
- [Mac/iOS] feat: Add RTCYUVHelper for darwin. (#28)
- Cross-platform `RTCMTLVideoView` for both iOS / macOS (#40)
- rotationOverride should not be assign (#44)
- [ObjC] Expose properties / methods required for AV1 codec support
(#60)
- Workaround: Render PixelBuffer in RTCMTLVideoView (#58)
- Improve iOS/macOS H264 encoder (#70)
- fix: fix video encoder not resuming correctly upon foregrounding
(#75).
- add PrivacyInfo.xcprivacy to darwin frameworks. (#112)
- Add NSPrivacyCollectedDataTypes key to xcprivacy file (#114)
- Thread-safe `RTCInitFieldTrialDictionary` (#116)
- Set RTCCameraVideoCapturer initial zoom factor (#121)
- Unlock configuration before starting capture session (#122)

## 6. Desktop Capture for macOS.
841d78f
- [Mac] feat: Support screen capture for macOS. (#24) (#36)
- fix: Get thumbnails asynchronously. (#37)
- fix: Use CVPixelBuffer to build DesktopCapture Frame, fix the crash
caused by non-CVPixelBuffer frame in RTCVideoEncoderH264 that cannot be
cropped. (#63)
- Fix the crash when setting the fps of the virtual camera. (#62)

## 7. Frame Cryptor Support.
fc08745
- feat: Frame Cryptor (aes gcm/cbc). (#54)
- feat: key ratchet/derive. (#66)
- fix: skip invalid key when decryption failed. (#81)
- Improve e2ee, add setSharedKey to KeyProvider. (#88)
- add failure tolerance for framecryptor. (#91)
- fix h264 freeze. (#93)
- Fix/send frame cryptor events from signaling thread (#95)
- more improvements for E2EE. (#96)
- remove too verbose logs (#107)
- Add key ring size to keyProviderOptions. (#109)

## 8. Other improvements.
eed6c8a
- Added yuv_helper (#57)
- ABGRToI420, ARGBToI420 & ARGBToRGB24 (#65)
- more yuv wrappers (#87)
- Fix naming for yuv helper (#113)
- Fix missing `RTC_OBJC_TYPE` macros (#100)

---------

Co-authored-by: Hiroshi Horie <[email protected]>
Co-authored-by: David Zhao <[email protected]>
Co-authored-by: davidliu <[email protected]>
Co-authored-by: Angelika Serwa <[email protected]>
Co-authored-by: ThΓ©o Monnom <[email protected]>
Looks like this line was missed during the m125 update.


272127d#diff-56f5e0c459b287281ef3b0431d3f4129e8e4be4c6955d845bcb22210f08b7ba5R2289

Adding it back in so that mic is properly released when muted.
)

Pausing/stopping the audio track can lead to a race condition against
the AudioTrackThread due to this assert. Normally this is fine since
directly pausing/stopping isn't possible, but user is using reflection
to workaround another audio issue (muted participants still have a
sending audio stream which keeps the audio alive, affecting global sound
if in the background).

Not a full fix, as would like to manually control the audio track
directly (needs a bigger fix to handle proper synchronization before
allowing public access), but this will work through reflection (user
takes responsibility for usage).
Expose initializers to pass in capture session to RTCCameraVideoCapturer
so we can use AVCaptureMultiCamSession etc to capture front and back
simultaneously for iOS.
…135)

There is a race condition in NetworkMonitor where native observers may
be removed concurrently with a notification being dispatched, leading to
a dangling pointer dereference (trying to dispatch an observer that was
already removed and destroyed), and from there a crash with access
violation.

By ensuring dispatching to native observers is done within the
synchronization lock that guards additions/removals of native observers
protects against this race condition. Since native observers callbacks
are posted to the networking thread in the C++ side anyway, there should
be no risk of deadlock/starvation due to long-running observers.

Bug: webrtc:15837
Change-Id: Id2b788f102dbd25de76ceed434c4cd68aa9a569e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/338643
Reviewed-by: Taylor Brandstetter <[email protected]>
Commit-Queue: Harald Alvestrand <[email protected]>
Reviewed-by: Harald Alvestrand <[email protected]>
Cr-Commit-Position: refs/heads/main@{#42256}

Co-authored-by: Guy Hershenbaum <[email protected]>
TODO:
- [x]  fix compile for RTCCameraVideoCapturer
- [ ]  fix RTCMTLRenderer ?

---------

Co-authored-by: Hiroshi Horie <[email protected]>
TODO: 
- [x] Return `.systemPreferredCamera` for devices (visionOS only).
- [x] Use `AVCaptureMultiCamSession` only if `isMultiCamSupported` is
true.
- [x] Silence statusBarOrientation warning.

---------

Co-authored-by: [email protected] <[email protected]>
17.0+ only atm

---------

Co-authored-by: cloudwebrtc <[email protected]>
~Allow to use "googEchoCancellation" constraint for software AEC.
For devices "googEchoCancellation" should be false to use
VoiceProcessingIO.~
Instead of converting to Float, output original Int data without
conversion.
Output the raw format and convert when required.
Related issue: #148
Cherry-pick :
https://webrtc.googlesource.com/src/+/fea60ef8e72fb17b4f8a5363aff7e63ab8027b4f

Fixed issue with network interfaces due to a missing return value in the
"nw_path_enumerate_interfaces(...)" block. Exposed in iOS 18,
RTCNetworkMonitor::initWithObserver will only enumerate the first
interface, instead of all device interfaces

Bug: webrtc:359245764
Change-Id: Ifb9f28c33306c0096476a4afb0cdb4d734e87b2c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/359541
Auto-Submit: Corby <[email protected]>
Commit-Queue: Jonas Oreland <[email protected]>
Reviewed-by: KΓ‘ri Helgason <[email protected]>
Reviewed-by: Jonas Oreland <[email protected]>
Cr-Commit-Position: refs/heads/main@{#42818}

Co-authored-by: Corby Hoback <[email protected]>
…rtAsString()` (#156)

Justification for this change is that `std::to_string` should be avoided
as it uses the user's locale and calls to it get serialized, which is
bad for concurrency.

My actual motivation for this is quite bizarre. Before this change, with
Zed's use of the LiveKit Rust SDK, I was getting connection strings that
were not valid utf-8, instead having a port of
`3\u0000\u0000\u001c\u0000`. I have not figured out how that could
happen or why this change fixes it.
Continued from #100 also prefix
enums.
I think this shouldn't break compiling.
@pblazej
Copy link
Author

pblazej commented Jun 13, 2025

In some places I moved things to the deprecated namespaces, will fix that later on:

webrtc::ArrayView<char>(name, kAdmMaxDeviceNameSize));
rtc::ArrayView<char>(name, kAdmMaxDeviceNameSize),

@pblazej pblazej changed the title WIP: m137 patches WIP: Update to m137 Jun 13, 2025
@pblazej pblazej changed the title WIP: Update to m137 WIP: Port patches to m137 Jun 13, 2025
@pblazej pblazej changed the title WIP: Port patches to m137 WIP: Update to m137 Jun 13, 2025
rm -rf $OUT_DIR/*-lib $OUT_DIR/LiveKitWebRTC.*

mkdir -p $OUT_DIR/macOS-lib
cp -R $OUT_DIR/macOS-x64/LiveKitWebRTC.framework $OUT_DIR/macOS-lib/LiveKitWebRTC.framework
Copy link
Author

Choose a reason for hiding this comment

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

@hiroshihorie LiveKitWebRTC prefix is assigned somewhere at the ninja level here?

Copy link
Member

Choose a reason for hiding this comment

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

The prefixed version is in other branches, I will update those once this branch is merged πŸ™‚
I don't think we want to commit this prefixed ver build script to main.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, but it would be good to have a versioned one as well.

Copy link
Author

Choose a reason for hiding this comment

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

One more thing: does it mean that this is not working at all?

COMMON_ARGS="
      rtc_objc_prefix = \"LK\"

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure since in the prefix branch, I hard code the macro in RTCMacros.h file...

@davidliu
Copy link
Contributor

For android builds, we have build commands here: https://github.com/webrtc-sdk/webrtc-build/blob/main/docs/build.md

You can kick off a build manually through github actions here and passing in a commit hash to the workflow dispatch. I started building one off of 509ebe4. The prefixed build will probably not work since it's code-sensitive and usually requires extra manual changes separate from merging with upstream.

@@ -286,21 +287,6 @@ std::vector<ProbeClusterConfig> ProbeController::OnNetworkAvailability(
return std::vector<ProbeClusterConfig>();
}

void ProbeController::UpdateState(State new_state) {
Copy link
Author

Choose a reason for hiding this comment

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

Introduced in 8505a98 (merged later into m125), not sure if applicable.

Copy link
Author

Choose a reason for hiding this comment

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

This one generated the most conflicts, would be nice to take a deeper look at the initializer hierarchy here.

@@ -843,7 +903,7 @@ static void LogDeviceInfo() {

// If we're not initialized we don't need to do anything. Audio unit will
// be initialized on initialization.
if (!audio_is_initialized_) return;
if (!playout_is_initialized_ && !recording_is_initialized_) return;
Copy link
Author

Choose a reason for hiding this comment

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

Here and elsewhere in this file: check if this logic is still correct.

Copy link
Author

@pblazej pblazej Jun 18, 2025

Choose a reason for hiding this comment

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

@pblazej
Copy link
Author

pblazej commented Jun 18, 2025

@davidliu looks like Android is failing on these patches, started fixing them, not sure if the build succeeds.

@pblazej pblazej changed the title WIP: Update to m137 Update to m137 Jun 18, 2025
@pblazej pblazej marked this pull request as ready for review June 18, 2025 13:06
@pblazej pblazej requested review from davidliu and cloudwebrtc June 18, 2025 13:06
@pblazej
Copy link
Author

pblazej commented Jun 18, 2025

@cloudwebrtc any possibility to test other platforms with/without Docker on a purely macOS machine (CI is not enough I guess, especially the windows stuff commented out etc.)?

@cloudwebrtc
Copy link
Member

Apple Silicon macOS cannot test windows/linux code under X86 arch through VMware (only arm64). In the past, I compiled a libwebrtc.dll to upgrade flutter-webrtc after the patch was completed, and did a functional check of webrtc on e2e-flutter (on a x64 windows pc). But device-related tests, such as switching audio input/output, camera acquisition resolution fps, etc., may still require some manual testing.

@cloudwebrtc
Copy link
Member

In the previous versions (m104, m114, m125), I did two rounds of cherry-picking PRs. The first one was similar to what you are doing, merging the big commit of the previous PR and the PRs added later. The second cherry-pick will classify the PRs added later and compress them into larger commits, so as to avoid too many commits accumulating and causing tons of work in the next upgrade.

@cloudwebrtc
Copy link
Member

Since conflicts between similar versions are much smaller than those between versions with a larger span, compressing them into a large commit usually does not cause too many conflicts in the next upgrade (the workload of resolving conflicts will be within an acceptable range)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants