-
Notifications
You must be signed in to change notification settings - Fork 99
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
Improve H264 encoder #70
Conversation
hiroshihorie
commented
May 18, 2023
•
edited
Loading
edited
- Improve bitrate
- Turn on Apple's optimizations when available
- maxQP for screen share mode
- Some code clean up
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good! (for the parts I was able to understand). I'm still not quite sure what the bug was that prevented it from maintaining bitrate previously.
_encoderFrameRate = MIN(settings.maxFramerate, _maxAllowedFrameRate); | ||
if (_encodeMode == Constant) { | ||
_targetBitrateBps = _maxBitrate; | ||
// _bitrateAdjuster->SetTargetBitrateBps(_targetBitrateBps); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: this can be removed?
|
||
CMTime presentationTimeStamp = CMTimeMake(frame.timeStampNs / rtc::kNumNanosecsPerMillisec, 1000); | ||
if (CMTimeCompare(presentationTimeStamp, _previousPresentationTimeStamp) == 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when does this happen? Is it an optimization for screenshare?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is extra safety to avoid this issue:
https://developer.apple.com/forums/thread/702891
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks! looks like this is part of reason why it's unable to keep up.
@@ -811,7 +910,12 @@ - (void)frameWasEncoded:(OSStatus)status | |||
RTC_LOG(LS_ERROR) << "Encode callback failed"; | |||
return; | |||
} | |||
_bitrateAdjuster->Update(frame.buffer.length); | |||
|
|||
// if (_encodeMode == Constant) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this because we've deprecated bitrateAdjuster? good to remove then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested two days ago only modifying kLimitToAverageBitRateFactor = 1.5f; to
kLimitToAverageBitRateFactor = 5.0f; also seems to get good results (keep bitrateAdjuster). will do a comparison test soon
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hiroshihorie , can you try just modifying kLimitToAverageBitRateFactor = 5.0f
based on the original code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@davidzhao Yes I think it's ok to remove _bitrateAdjuster.
@cloudwebrtc kLimitToAverageBitRateFactor 5~8 seems fine but, I think chrome was using 10.0f if my memory was correct. Should we just follow what chrome uses ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cloudwebrtc kLimitToAverageBitRateFactor 5~8 seems fine but, I think chrome was using 10.0f if my memory was correct. Should we just follow what chrome uses ?
following the chrome should be fine, and test results are as expected, so great work @hiroshihorie
@@ -54,14 +54,42 @@ - (void)frameWasEncoded : (OSStatus)status flags : (VTEncodeInfoFlags)infoFlags | |||
// The ratio between kVTCompressionPropertyKey_DataRateLimits and | |||
// kVTCompressionPropertyKey_AverageBitRate. The data rate limit is set higher | |||
// than the average bit rate to avoid undershooting the target. | |||
const float kLimitToAverageBitRateFactor = 1.5f; | |||
const float kLimitToAverageBitRateFactor = 10.0f; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any idea why this value had to be set so high compared to what it was before? it seems that 10x average bitrate would seem quite high, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just did some tests for camera capture, and the bit rate will be close to the target value (but not exceeding the limit). the moving picture is smooth and clear, and the bit rate of the static picture can be reduced to 1/5 of the target bit rate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The game screen may be due to complex textures, at least 4~6mbps needs to be set to achieve smooth and clear 720p/30fps, and the camera only needs 2mbps, so this PR is good for me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@davidzhao This is for computing data rate limits and not the average bitrate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
got it, I don't really understand this so feel free to ignore :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Chrome has both CBR mode and VBR(ABR) mode, not sure in what condition that switches. Will take a quick look. |
* progress * enable low-latency video encoding * fix compile * prioritize speed * fix required os versions * maxQP for screensharing * cbr * Update RTCVideoEncoderH264.mm * Update RTCVideoEncoderH264.mm * format * clean up * resolution alignment * revert resolution alignment
[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) Co-authored-by: Hiroshi Horie <548776+hiroshihorie@users.noreply.github.com>
[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). Co-authored-by: Hiroshi Horie <548776+hiroshihorie@users.noreply.github.com>
[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). Co-authored-by: Hiroshi Horie <548776+hiroshihorie@users.noreply.github.com>
[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). Co-authored-by: Hiroshi Horie <548776+hiroshihorie@users.noreply.github.com>
[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). Co-authored-by: Hiroshi Horie <548776+hiroshihorie@users.noreply.github.com>
[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) Co-authored-by: Hiroshi Horie <548776+hiroshihorie@users.noreply.github.com>
[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). Fix camera rotation (#92) 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 Co-authored-by: Hiroshi Horie <548776+hiroshihorie@users.noreply.github.com>
[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). Fix camera rotation (#92) 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 Co-authored-by: Hiroshi Horie <548776+hiroshihorie@users.noreply.github.com>
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>
[Mac/iOS] feat: Add RTCYUVHelper for darwin. (webrtc-sdk#28) Cross-platform `RTCMTLVideoView` for both iOS / macOS (webrtc-sdk#40) rotationOverride should not be assign (webrtc-sdk#44) [ObjC] Expose properties / methods required for AV1 codec support (webrtc-sdk#60) Workaround: Render PixelBuffer in RTCMTLVideoView (webrtc-sdk#58) Improve iOS/macOS H264 encoder (webrtc-sdk#70) fix: fix video encoder not resuming correctly upon foregrounding (webrtc-sdk#75). Co-authored-by: Hiroshi Horie <548776+hiroshihorie@users.noreply.github.com>