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

Fix auto restart video stream to ensure hmi level is streamable #1808

Merged
merged 6 commits into from
Apr 11, 2022

Conversation

JulianKast
Copy link
Contributor

@JulianKast JulianKast commented Apr 8, 2022

Fixes #1807

This PR is [ready] for review.

Risk

This PR makes [minor] API changes.

Testing Plan

  • I have verified that I have not introduced new warnings in this PR (or explain why below)
  • I have run the unit tests with this PR
  • I have tested this PR against Core and verified behavior (if applicable, if not applicable, explain why below).
  • I have tested Android

Core Tests

Test:
Start a video stream to sdl_hmi
Using voice commands exit out of the app
Open app on HMI

Result:
You should be prompted by hmi to start streaming, click ok and video should stream.

Core 8.1 with sdl_hmi

Summary

Adds check to VideoStreamManager to check if HMI level is streamable before restarting video streaming, and added behavior for the stream to auto-restart when hmi is in a streamable state

Changelog

Bug Fixes
  • Fixes video stream manager form going into an unrecoverable state when it tries to start video streaming in a nonstreamable state

CLA

@codecov
Copy link

codecov bot commented Apr 8, 2022

Codecov Report

Merging #1808 (eea49e9) into 5.4.0_RC (a624a06) will decrease coverage by 0.29%.
The diff coverage is 16.66%.

Impacted file tree graph

@@              Coverage Diff               @@
##             5.4.0_RC    #1808      +/-   ##
==============================================
- Coverage       54.05%   53.76%   -0.30%     
- Complexity       5519     5549      +30     
==============================================
  Files             562      562              
  Lines           25723    26060     +337     
  Branches         3372     3503     +131     
==============================================
+ Hits            13904    14010     +106     
- Misses          10562    10779     +217     
- Partials         1257     1271      +14     
Impacted Files Coverage Δ
...tdevicelink/managers/video/VideoStreamManager.java 23.41% <0.00%> (+1.01%) ⬆️
...smartdevicelink/encoder/VirtualDisplayEncoder.java 41.83% <40.00%> (-3.43%) ⬇️
...ink/managers/screen/BaseTextAndGraphicManager.java 64.58% <0.00%> (+0.41%) ⬆️
...rc/main/java/com/android/grafika/gles/EglCore.java 44.82% <0.00%> (+0.86%) ⬆️
...rtdevicelink/streaming/video/SdlRemoteDisplay.java 51.21% <0.00%> (+1.21%) ⬆️
...icelink/managers/screen/PresentAlertOperation.java 59.92% <0.00%> (+2.71%) ⬆️

…icitly released -- state may be leaked", So I modified logic to release it if we have already created EglCore object and are creating a new one
…form an HMI state that was not streamable to one that is.
@noah-livio noah-livio self-requested a review April 11, 2022 20:35
Copy link
Contributor

@noah-livio noah-livio left a comment

Choose a reason for hiding this comment

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

The code looks good and the fix works

@noah-livio noah-livio merged commit 64ade94 into 5.4.0_RC Apr 11, 2022
@noah-livio noah-livio deleted the video_streaming_restart_error branch April 11, 2022 20:37
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