Repository navigation
Fix portrait video quality detection and fallback logging - #620
Open
davidjiagoogle wants to merge 5 commits into
Open
davidjiagoogle wants to merge 5 commits into
davidjiagoogle wants to merge 5 commits into
Conversation
Contributor
There was a problem hiding this comment.
Code Review
This pull request makes video quality resolution mapping orientation-invariant by using the short edge of the resolution, adds corresponding unit tests, and prevents crashes by allowing fallback video qualities. The review feedback suggests simplifying a log statement's string concatenation into a single Kotlin string template and highlights a gap in the QUALITY_RANGE_MAP classification ranges that could cause 1440p resolutions to be incorrectly classified as unspecified.
temcguir
requested changes
Oct 28, 2026
temcguir
left a comment
Collaborator
There was a problem hiding this comment.
Nice fix, looks good overall! Just a few small things inline. Could you also mention the FHD range change in the PR description?
- Document short-edge matching and 1440p FHD range in QUALITY_RANGE_MAP - Skip video quality fallback log when resolutionInfo is null and downgrade to Log.w - Make getWidthFromCropRect/getHeightFromCropRect rotation-aware and format width x height in DebugOverlayComponents - Update 1440p test resolutions to 2560x1440 / 1440x2560 and add range boundary and crop-rect rotation unit tests
This file contains hidden or 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
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.
Summary
getVideoQualityFromResolution):VideoCaptureis wrapped inStreamSharing,VirtualCameraAdapterpre-rotates the child stream resolution into portrait (e.g.,720x1280for HD or1080x1920for FHD).getVideoQualityFromResolution()checkedres.heightagainstQUALITY_RANGE_MAP, which misclassified portrait720x1280(height = 1280) asVideoQuality.FHDand portrait1080x1920(height = 1920) asVideoQuality.UNSPECIFIED.minOf(res.width, res.height)so quality detection uses the short edge regardless of orientation, and extend theFHDrange inQUALITY_RANGE_MAPup to2159so intermediate resolutions like 1440p (QHD) map toFHDinstead ofUNSPECIFIED.getWidthFromCropRect/getHeightFromCropRect):ResolutionInfo.rotationDegreeswhen computing upright video width and height fromcropRect, refresh dimensions when device rotation updates, and formatwidth x heightinDebugOverlayComponentsso dimensions display accurately both with and withoutStreamSharing.runSingleCameraSession):resolutionInfo != nullandsessionSettings.videoQuality != VideoQuality.UNSPECIFIED, fix string interpolation, and log atLog.w.createVideoUseCase):QualitySelector.from(quality, FallbackStrategy.lowerQualityOrHigherThan(quality))to accurately reflect the fallback behavior.CameraSessionTestcoveringgetVideoQualityFromResolutionacross landscape, portrait, 1440p (2560x1440/1440x2560), range boundaries, andnullresolutions, as well asgetWidthFromCropRectandgetHeightFromCropRectacross rotated and unrotated crop rects.