Improve handling of unsupported resolutions#1804
Merged
JulianKast merged 2 commits into5.4.0_RCfrom Apr 11, 2022
Merged
Conversation
Adds a check in VirtualDisplayEncoder for unsupported resolutions that makes the virtual display encoder fail to start, prints a more helpful error message, and propagates an exception to the video stream manager Changes in VideoStreamManager now kill the video stream service when the encoder fails to start, allowing for subsequent streams to start after an encoder error
Codecov Report
@@ Coverage Diff @@
## 5.4.0_RC #1804 +/- ##
==============================================
- Coverage 54.05% 54.05% -0.01%
- Complexity 5519 5522 +3
==============================================
Files 562 562
Lines 25723 25742 +19
Branches 3372 3376 +4
==============================================
+ Hits 13904 13914 +10
- Misses 10562 10570 +8
- Partials 1257 1258 +1
|
JulianKast
approved these changes
Apr 11, 2022
Contributor
JulianKast
left a comment
JulianKast
left a comment
There was a problem hiding this comment.
When sdl_hmi ask if you would like to start video streaming, you have to hit cancel if the stream is not supported, to be able to stream again. Will open issue to look into that for next release
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
2 participants
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.
Fixes #1803
This PR is ready for review.
Risk
This PR makes no API changes.
Testing Plan
Unit Tests
Ran Existing CI Tests
Core Tests
Tested on Samsung Galaxy S21 5G, Android 12 using sdl_test_suite_java
Expected results: The low resolution video fails to play, an error is printed. Afterwards, the supported resolution plays successfully
Observed results: The low resolution video fails to play, an error is printed. Afterwards, the supported resolution plays successfully
Core version / branch / commit hash / module tested against: SDL Core release/8.1.0 cbc9d3928fb3c4a8db6608e84bad67d2398a52e7
HMI name / version / branch / commit hash / module tested against: SDL HMI release/5.7.0 cd2d250cba100d1e0dd354172d2e24c0bd3449fc
Summary
Adds a check in
VirtualDisplayEncoderfor unsupported resolutions that makes the virtual display encoder fail to start, prints a more helpful error message, and propagates an exception to the video stream manager. Changes inVideoStreamManagernow kill the video stream service when the encoder fails to start, allowing for subsequent streams to start after an encoder error.Changelog
Breaking Changes
None
Enhancements
None
Bug Fixes
Fixes #1803
CLA