Fix streamer activity in case of suspend #3488
Conversation
AKalinich-Luxoft
requested review from
LuxoftAKutsan,
Ypostolov,
SNiukalov and
ychernysheva
AKalinich-Luxoft
force-pushed
the
fix/fix_streamer_activity_in_case_of_suspend
branch
2 times, most recently
from
d3982a3
to
0c95e8a
Compare
AKalinich-Luxoft
force-pushed
the
fix/fix_streamer_activity_in_case_of_suspend
branch
from
0c95e8a
to
ce5bce1
Compare
The problem of the current implementation is that SDL kills streaming thread responsible for sending a/v streaming data to HMI when streaming timeout expires. This issue is observed when mobile app dumps 10-seconds audio file to SDL during 2 seconds and after that app does not send any data. In that case HMI will play audio file during 2 seconds + timeout = 5 seconds. At the 5th second SDL just kills streaming thread with all pending messages, however audio service is still open. As a result not a whole audio file is played. The correct behavior from SDL side in that case is not kill streaming thread when streaming timeout was expired. SDL should kill streaming thread only when service is actually stopped. Current SDL behavior was updated to align with a correct behavior described above.
AKalinich-Luxoft
force-pushed
the
fix/fix_streamer_activity_in_case_of_suspend
branch
from
ce5bce1
to
dcd7ce2
Compare
There was a problem hiding this comment.
The code changes only seem to affect the stream time of a stream that was suspended at some point. Am I correct that stream timeouts are not changed by this proposal unless a stream was suspended? I am confused because the issue and PR description seem to suggest otherwise, the examples and reproduction steps do not have any suspend in them, but the titles of both sound to match the code.
@iCollin you are right, the code changes does not change any streaming timeouts, but just updates SDL behavior little bit. This PR provides a new state for a streaming - |
…er_activity_in_case_of_suspend
…me (smartdevicelink#3057)" This reverts commit 7c442ab.
|
@iCollin I did a few things here:
Please double check this PR one more time |
| case Application::StreamingState::kSuspended: { | ||
| // Don't stop activity in media_manager_ in that case | ||
| // Just cancel the temporary streaming state | ||
| state_ctrl_.OnVideoStreamingStopped(app); |
There was a problem hiding this comment.
Should this change the video state? Video could still be playing on the HMI if the app sent over a long video quickly.
There was a problem hiding this comment.
@iCollin it just updates video streaming state via OnHMIStatus notification. This is an opposite action to what is done for kStarted case. Note that streaming may transit between suspended <-> started several times while service is opened so SDL should notify application each time on each such transition.
This also allows mobile proxy lib to decide should it stop a/v service or keep it alive. I know that SPT tracks such transitions during streaming and performs some actions.
This reverts commit 42721d5.
This reverts commit 42721d5.
…ignition cycle (#3468) * Temporary commit to change URL of rpc_spec repository * Change rpc_spec reference according to the new changes in the rpc_spec repo * Add the new OnAppCapabilityUpdated RPC with appropriate types to the HMI_API * Add the smart objects keys for the new parameters * Add OnAppCapabilityUpdatedNotification and BCOnAppCapabilityUpdatedNotification * Add Unit tests for the OnAppCapabilityUpdated RPCs * Fix code generation for recursive structures Currently InterfaceGenerator is not able to generate recursive structures i.e. one or several structure attributes have its own type e.g.: struct VideoStreamingCapability { ... VideoStreamingCapability additionalVideoStreamingCapabilities; } because structure type fully defined only when all of it attributes are defined. Otherwise param with type of structure will have AlwaysFalseSchema for validation so this param will never be validated successfully. With this commit InterfaceGenerator is able to handle such problem. * Update OnBCSystemCapabilityUpdatedNotificationFromHMITest Unit tests Add the checking of the SystemCapabilityType::VIDEO_STREAMING in the message and setting it into the HMICapabilities. * Add additionalVideoStreamingCapabilities to default hmi_capabilities * Add Processing OnSystemCapabilityUpdated with videoStreamingCapabilities * Update Unit tests for OnSystemCapabilityUpdated notifications * Add app_id to OnAppCapabilityUpdated notification to HMI * Updates during code review * Update logger * Update link to rpc_spec * Return back the driverDistractionCapability that was missed during conflicts resolving * Fix functionality that was broken during conflicts resolving * Updates during code review * Update link to rpc_spec * Revert "Fix streamer activity in case of suspend (#3488)" This reverts commit 42721d5. * Adjust old fix for all streaming transports * Change commit to rpc_spec * Update rpc_spec * Mark function as deprecated * Updates during code review * Remove trailing whitespaces * Update link to rpc_spec * Update rpc_spec Co-authored-by: Igor Gapchuk <igapchuck@luxoft.com> Co-authored-by: YarikMamykin <ymamykin@gmail.com> Co-authored-by: Andrii Kalinich <AKalinich@luxoft.com> Co-authored-by: Dmitriy Boltovskiy <dboltovskyi@luxoft.com>
Fixes #3160
This PR is ready for review.
Risk
This PR makes no API changes.
Testing Plan
[Describe how you plan to unit test the changes in this PR]
Summary
The problem of the current implementation is that SDL kills streaming thread responsible for sending a/v streaming data to HMI when streaming timeout expires. This issue is observed when mobile app dumps 10-seconds audio file to SDL during 2 seconds and after that app does not send any data. In that case HMI will play audio file during 2 seconds + timeout = 5 seconds. At the 5th second SDL just kills streaming thread with all pending messages, however audio service is still open. As a result not a whole audio file is played.
The correct behavior from SDL side in that case is not kill streaming thread when streaming timeout was expired. SDL should kill streaming thread only when service is actually stopped.
Current SDL behavior was updated to align with a correct behavior described above.
Tasks Remaining:
CLA