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

Add support for resolution alignment during encoding #85

Merged
merged 1 commit into from
Nov 9, 2023

Conversation

DanielNovak
Copy link
Contributor

@DanielNovak DanielNovak commented Nov 7, 2023

This PR adds two new encoder factories:

  • SimulcastAlignedVideoEncoderFactory
  • DefaultAlignedVideoEncoderFactory

These should be generally always preferred to be used instead of the default SimulcastVideoEncoderFactory and DefaultVideoEncoderFactory. The difference is that the new factories allow to specify a resolution alignment requirement (resolutionAdjustmentin the constructor). HW encoders for VP8 (and H264) require a 16x16 aligned resolutions.

This code is based on the original source from:
https://github.com/shiguredo/sora-android-sdk/blob/3cc88e806ab2f2327bf3042072e98d6da9df4408/sora-android-sdk/src/main/kotlin/jp/shiguredo/sora/sdk/codec/HardwareVideoEncoderWrapperFactory.kt
Credit and thanks go to Shiguredo, Inc.

This is especially a problem while simulcasting - webrtc computes the resolutions based on the scaleResolutionDownBy defined in RtpParameters.Encodings and these resolutions are not 16x16 and this leads to various hard to debug problems with the encoder. WebRTC allows you to override the getEncoderInfo() in HardwareVideoEncoder.java and define the alignment but in most cases it will not crop the resolution and instead it will just adjust the scaleResolutionDownBy you defined. This behaviour is defined here. So from 1, 2, 4 (full, half, quarter resolution) it can create 1, 1, 1 to match the alignment and this is not a correct approach. In our tests the encoder would not work correctly and would drop frames in simulcast scenarios (1280x720, 3 simulcast layers - 1, 2, 4. This breaks if you are stream with phone oriented to left-orientation on Pixel devices).

Relevant issues:
https://bugs.chromium.org/p/chromium/issues/detail?id=1084702
https://bugs.chromium.org/p/chromium/issues/detail?id=1351371
https://bugs.chromium.org/p/webrtc/issues/detail?id=12328

@DanielNovak DanielNovak requested a review from skydoves as a code owner November 7, 2023 07:47
@DanielNovak DanielNovak force-pushed the aligned_encoder branch 5 times, most recently from 1114502 to f80536a Compare November 7, 2023 08:39
Copy link
Member

@skydoves skydoves left a comment

Choose a reason for hiding this comment

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

Overall looks good to me. Nicely done!

@skydoves skydoves merged commit 28adca5 into main Nov 9, 2023
1 check passed
@skydoves skydoves deleted the aligned_encoder branch November 9, 2023 05:00
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.

2 participants