Fix simulcast using hardware encoder on Android #48
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks for the PR! We'll take a look. |
|
Seems like the Would simply setting |
Yes, the checks simply stops encoding if frame size is not aligned. I don't see |
|
Ah, I think I ran into an old issue with this now: Attempting to initEncode with odd dimensions will crash; this wasn't an issue before since alignment fixed this, but without alignment on simulcast layers, that becomes an issue again. I don't think this route will be viable without someway to at least aligning to even numbers. I wonder if each simulcast layer can be aligned independently; that way so we don't have to deal with the cropping issues as well as not being limited to powers of 2 scales. |
|
I am wondering how other people use H264 hardware encoder as it clearly doesn't work as expected 🙁 |
|
I tested this on my device some odd resolutions like 1279x719 or 1111x1111 and it works for me. Could you tell us:
Some time ago I considered that too but it would be too many changes if it's possible at all. |
|
Here, the issue is that the base layer gets aligned, but the scaled layers are scaled without any further alignment (since the previous math dictated that no further alignment was needed). I have the base layer being 600x800, with a separate simulcast layer downscaled to 360x480 (0.6x). Since 600 isn't divisible by 16, it gets aligned to 592. The simulcast layer's width becomes 592*0.6 = 355. I reproduced this issue on my Pixel 4, but the last time I saw this issue was around m97ish, and that pretty reliably crashed on all devices IIRC. Crash: |
|
@davidliu thanks, I managed to reproduce it on my device too. So, if the problem is that all layers aren't aligned to 2, then my idea is to set them to be aligned to 2, instead of 16 like now. Turns out it's already configurable on the Java side here: https://github.com/livekit/client-sdk-android/blob/main/livekit-android-sdk/src/main/java/io/livekit/android/webrtc/SimulcastVideoEncoderFactoryWrapper.kt#L193 and all layers are aligned to 2 and with I updated the PR accordingly, now all it does is removing the crash if the resolutions are not aligned to 16x16. |
|
Getting some weird outputs now: For a base layer of 600x800, with scaleResolutionDowns of [1, 1.66667, 3.33333], the alignment adjuster changes the scales like so: The 1 -> 1.08333 seems a little weird, though it still results in an even number for the dimens. The 1.625 results in an odd number though (600 / 1.625 = 369). I'll take a look into alignment adjuster to see what went wrong here. |
|
I think I understand what's going on here now; the issue is that no realignment seems to be done after the alignment is changed from the alignment adjuster. In the case above, the alignment changes to 13, with scales of [13/12, 13/8, 13/4]. This works as long as the resolution is a multiple of 13, as the scales will turn that multiple of 13 into multiples of 12, 8, 4 respectively, which are all divisible by the original required alignment of 2. However, no alignment seems to be taking place, and the scales are applied directly to the original resolution, resulting in the odd number I was see. I wonder if there's something we need to do to trigger an alignment when needed. @graszka22 would you happen to know where the alignment takes place? |
|
Ok, with a little more digging, I think I have a solution to this issue; in HardwareVideoEncoder, put back the width/height checks, but reduce With this in place (and |
|
Alternatively, changing the catch clause when configuring the codec in |
|
@davidliu Agreed, I changed |
davidliu
approved these changes
Nov 21, 2022
|
Seems good. Any other changes you think this will need? Or ready to merge? |
|
It's ready to merge 👍 |
|
@davidliu Thank you 😄 |
cloudwebrtc
pushed a commit
that referenced
this pull request
Jan 18, 2023
* Fix simulcast using hardware encoder on Android * Revert disabling apply_alignment_to_all_simulcast_layers * Update HardwareVideoEncoder.java
cloudwebrtc
pushed a commit
that referenced
this pull request
Jun 6, 2023
* Fix simulcast using hardware encoder on Android * Revert disabling apply_alignment_to_all_simulcast_layers * Update HardwareVideoEncoder.java
cloudwebrtc
added a commit
that referenced
this pull request
Jun 12, 2023
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) Co-authored-by: Hiroshi Horie <548776+hiroshihorie@users.noreply.github.com> Co-authored-by: Angelika Serwa <angelika.serwa@gmail.com>
Merged
cloudwebrtc
added a commit
that referenced
this pull request
Jul 12, 2023
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) Co-authored-by: Hiroshi Horie <548776+hiroshihorie@users.noreply.github.com> Co-authored-by: Angelika Serwa <angelika.serwa@gmail.com>
cloudwebrtc
added a commit
that referenced
this pull request
Jul 13, 2023
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) Co-authored-by: Hiroshi Horie <548776+hiroshihorie@users.noreply.github.com> Co-authored-by: Angelika Serwa <angelika.serwa@gmail.com>
cloudwebrtc
added a commit
that referenced
this pull request
May 21, 2024
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) Co-authored-by: Hiroshi Horie <548776+hiroshihorie@users.noreply.github.com> Co-authored-by: Angelika Serwa <angelika.serwa@gmail.com>
Merged
cloudwebrtc
added a commit
that referenced
this pull request
May 21, 2024
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) Co-authored-by: Hiroshi Horie <548776+hiroshihorie@users.noreply.github.com> Co-authored-by: Angelika Serwa <angelika.serwa@gmail.com>
cloudwebrtc
added a commit
that referenced
this pull request
May 21, 2024
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) Co-authored-by: Hiroshi Horie <548776+hiroshihorie@users.noreply.github.com> Co-authored-by: Angelika Serwa <angelika.serwa@gmail.com>
cloudwebrtc
added a commit
that referenced
this pull request
May 22, 2024
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) Co-authored-by: Hiroshi Horie <548776+hiroshihorie@users.noreply.github.com> Co-authored-by: Angelika Serwa <angelika.serwa@gmail.com>
cloudwebrtc
added a commit
that referenced
this pull request
May 22, 2024
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) Co-authored-by: Hiroshi Horie <548776+hiroshihorie@users.noreply.github.com> Co-authored-by: Angelika Serwa <angelika.serwa@gmail.com>
cloudwebrtc
added a commit
that referenced
this pull request
Jun 12, 2024
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 <548776+hiroshihorie@users.noreply.github.com> Co-authored-by: David Zhao <dz@livekit.io> Co-authored-by: davidliu <davidliu@deviange.net> Co-authored-by: Angelika Serwa <angelika.serwa@gmail.com> Co-authored-by: Théo Monnom <theo.monnom@outlook.com>
npazkevich
pushed a commit
to npazkevich/webrtc
that referenced
this pull request
Jun 24, 2024
Simulcast support for iOS SDK (webrtc-sdk#4) Support for simulcast in Android SDK (webrtc-sdk#3) include simulcast headers for mac also (webrtc-sdk#10) Fix simulcast using hardware encoder on Android (webrtc-sdk#48) Co-authored-by: Hiroshi Horie <548776+hiroshihorie@users.noreply.github.com> Co-authored-by: Angelika Serwa <angelika.serwa@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi! When using simulcast on Android with hardware encoder we noticed that all simulcast layers have the same resolution and switching between them is broken (video stops encoding when there is only one layer etc.). That's not only our problem, other people noticed it as well, there is a discussion here: https://bugs.chromium.org/p/webrtc/issues/detail?id=12328
We know what causes the problem. Some phones have weird hardware encoders that accept only resolutions aligned to 16x16. Webrtc developers added
apply_alignment_to_all_simulcast_layersoption to apply 16x16 alignment to all simulcast layers. There is a code inalignment_adjuster.ccthat tries to change the scales to apply the alignment but for 16x16 alignment it will always set the scales to {1, 1, 1}. I added a description why it happens in the discussion linked above.There are basically two solutions to this problem:
kMaxAlignmentconstant. Right now it's hardcoded to16. WhenkMaxAlignment = 64the scales can be adjusted to{1, 2, 4}. However, this comes with a cost that the video can be cropped (even more than 64px) and aspect ratio of the video might be changed a bit. That's what we tried at first here https://webrtc-review.googlesource.com/c/src/+/277440 but the reviewer told us about cropping.apply_alignment_to_all_simulcast_layers, that's what the reviewer suggested to us and we think it would be a better idea. It fixes the simulcast issues and there will be no cropping. However right nowapply_alignment_to_all_simulcast_layersis set in the cpp code and we can't disable it from java code. This PR changes that. It can break the encoding on some devices though. From the discussion here: https://bugs.chromium.org/p/chromium/issues/detail?id=1084702 apparently it affects only Pixel 3 and 3a with Android < 12. If that's a problem we can enableapply_alignment_to_all_simulcast_layersonly on those affected devices.Please let us know what do you think about those solutions. We're open for ideas on how to fix that. Thanks!