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

Fixed multiple issues where video stream resuming fails #1538

Closed

Conversation

NicoleYarroch
Copy link
Contributor

@NicoleYarroch NicoleYarroch commented Jan 30, 2020

Fixes #1527

This PR is ready for review.

Risk

This PR makes no 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).

Unit Tests

I added unit test for testing stopping the SDLStreamingVideoScaleManager, SDLStreamingAudioLifecycleManager and SDLH264VideoEncoder when the session between the accessory and device is closed.

Core Tests

Core version / branch / commit hash / module tested against: SYNC 3.4 BUILD 19353_DEVTEST_r133796
HMI name / version / branch / commit hash / module tested against: same as above

Summary

  1. The streaming video lifecycle manager was not reset correctly on disconnect so sometimes an end video service request was sent incorrectly when a new session opened.
  2. The pixel buffer pool in the SDLH264VideoEncoder class randomly invalidates itself when the device app is backgrounded. Handling was added to reset the pixel buffer pool when necessary, otherwise video frames will not be sent.
  3. If the the response to the video start service comes after the app has been put in the background then the app now transitions to the suspended state instead of attempting to send video frames.
  4. The streaming audio lifecycle manager now resets correctly on disconnect.

When testing you may run into this issue 1540 where no video is streamed by the CarWindow due to the video encoder failing when the app on the device is backgrounded. You will see error logs now when the pixelBuffer fails.

Changelog

Bug Fixes
  • Fixed several issues where video stream resuming fails.

CLA

Sorry, something went wrong.

@NicoleYarroch NicoleYarroch added the bug A defect in the library label Jan 30, 2020
@NicoleYarroch NicoleYarroch self-assigned this Jan 30, 2020
@NicoleYarroch NicoleYarroch added the manager-streaming-video Relating to the manager layer - video streaming label Feb 4, 2020
@NicoleYarroch NicoleYarroch changed the title WIP: Fixing issues where video stream resuming fails Fixed multiple issues where video stream resuming fails Feb 4, 2020
@NicoleYarroch
Copy link
Contributor Author

@SatbirTanda I know you found additional issues with handling error messages from the security managers but can you also test this PR and see if it fixes some issues (r.e. #1527)?

@SatbirTanda
Copy link
Contributor

Hi @NicoleYarroch, I was able to test this branch with my encrypted streaming app and I am still consistently seeing this issue: #1527

@NicoleYarroch
Copy link
Contributor Author

@SatbirTanda Thanks for testing the PR. Can you see from the logs if any of these scenarios is occurring:

  1. Video frames are being sent by the app but the head unit simply isn't showing the video.
  2. Video frames are not being created because of an error with creating the video encoder or the pixel buffer.
  3. The video start service NAKs or the video end service NAKs.

@SatbirTanda
Copy link
Contributor

SatbirTanda commented Feb 10, 2020

Yes I see 3) a NAK occurs on resumption - at which point the streaming state does not get set to STOPPED because an error is not handled properly #1527, it occurs specifically before the ssl handshake and needs to be handled in SDL iOS, looks like there was a TODO from 4 yrs ago:

// TODO: (Joel F.)[2016-02-15] Should check for errors
Handling this error is a key in determining why NAKs occur in the first place. syncFrame is not being called to send any frames, its an infinite loop in sdl_processSecurityMessage

@NicoleYarroch
Copy link
Contributor Author

Closing in favor of PR #1551

@joeljfischer joeljfischer deleted the bugfix/issue_1527_video_stream_resuming_fails branch April 8, 2020 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A defect in the library manager-streaming-video Relating to the manager layer - video streaming
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants