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.

Sign up for GitHub

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

Jump to bottom

Fix streamer activity in case of suspend #3488

Merged
merged 6 commits into from
Oct 5, 2020
Merged

Fix streamer activity in case of suspend #3488

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
50a5aa2
Fix streamer activity in case of suspend
AKalinich-Luxoft Aug 31, 2020
dcd7ce2
Adjust fix to work with audio socket streaming
AKalinich-Luxoft Sep 1, 2020
f29a3e7
Merge remote-tracking branch 'origin_lux/develop' into fix/fix_stream…
AKalinich-Luxoft Oct 2, 2020
3fd087e
Revert "[WIP] initial fix of socket streaming by guessing playback ti…
AKalinich-Luxoft Oct 2, 2020
e6abe62
Remove unused code after revert
AKalinich-Luxoft Oct 2, 2020
e0c7a84
Fix style issues
AKalinich-Luxoft Oct 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions src/components/application_manager/include/application_manager/application.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,10 @@ class DynamicApplicationData {
class Application : public virtual InitialApplicationData,
public virtual DynamicApplicationData {
public:
/**
* @brief The StreamingState enum defines current streaming state
*/
enum class StreamingState { kStopped, kStarted, kSuspended };
enum ApplicationRegisterState { kRegistered = 0, kWaitingForRegistration };

Application() : is_greyed_out_(false) {}
Expand Down Expand Up @@ -660,10 +664,8 @@ class Application : public virtual InitialApplicationData,
/**
* @brief Wakes up streaming process for application
* @param service_type Type of streaming service
* @param timer_len The amount of time in ms the timer will wait
*/
virtual void WakeUpStreaming(protocol_handler::ServiceType service_type,
uint32_t timer_len = 0) = 0;
virtual void WakeUpStreaming(protocol_handler::ServiceType service_type) = 0;

virtual bool is_voice_communication_supported() const = 0;
virtual void set_voice_communication_supported(
Expand Down
3 changes: 1 addition & 2 deletions src/components/application_manager/include/application_manager/application_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,7 @@ class ApplicationImpl : public virtual Application,
void StopStreamingForce(protocol_handler::ServiceType service_type);
void StopStreaming(protocol_handler::ServiceType service_type);
void SuspendStreaming(protocol_handler::ServiceType service_type);
void WakeUpStreaming(protocol_handler::ServiceType service_type,
uint32_t timer_len = 0);
void WakeUpStreaming(protocol_handler::ServiceType service_type);

virtual bool is_voice_communication_supported() const;
virtual void set_voice_communication_supported(bool option);
Expand Down
8 changes: 1 addition & 7 deletions src/components/application_manager/include/application_manager/application_manager_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -880,15 +880,9 @@ class ApplicationManagerImpl
bool result,
std::vector<std::string>& rejected_params) OVERRIDE;

/**
* @brief Callback calls when application starts/stops data streaming
* @param app_id Streaming application id
* @param service_type Streaming service type
* @param state Shows if streaming started or stopped
*/
void OnAppStreaming(uint32_t app_id,
protocol_handler::ServiceType service_type,
bool state) OVERRIDE;
const Application::StreamingState new_state) OVERRIDE;

mobile_api::HMILevel::eType GetDefaultHmiLevel(
ApplicationConstSharedPtr application) const;
Expand Down
36 changes: 23 additions & 13 deletions src/components/application_manager/src/application_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -616,6 +616,9 @@ void ApplicationImpl::StopStreaming(
void ApplicationImpl::StopNaviStreaming() {
SDL_LOG_AUTO_TRACE();
video_stream_suspend_timer_.Stop();
application_manager_.OnAppStreaming(app_id(),
protocol_handler::ServiceType::kMobileNav,
StreamingState::kStopped);
MessageHelper::SendNaviStopStream(app_id(), application_manager_);
set_video_streaming_approved(false);
set_video_stream_retry_number(0);
Expand All @@ -624,6 +627,9 @@ void ApplicationImpl::StopNaviStreaming() {
void ApplicationImpl::StopAudioStreaming() {
SDL_LOG_AUTO_TRACE();
audio_stream_suspend_timer_.Stop();
application_manager_.OnAppStreaming(app_id(),
protocol_handler::ServiceType::kAudio,
StreamingState::kStopped);
MessageHelper::SendAudioStopStream(app_id(), application_manager_);
set_audio_streaming_approved(false);
set_audio_stream_retry_number(0);
Expand All @@ -634,14 +640,17 @@ void ApplicationImpl::SuspendStreaming(
using namespace protocol_handler;
SDL_LOG_AUTO_TRACE();

if (ServiceType::kMobileNav == service_type) {
if (ServiceType::kMobileNav == service_type && !video_streaming_suspended_) {
video_stream_suspend_timer_.Stop();
application_manager_.OnAppStreaming(app_id(), service_type, false);
application_manager_.OnAppStreaming(
app_id(), service_type, StreamingState::kSuspended);
sync_primitives::AutoLock lock(video_streaming_suspended_lock_);
video_streaming_suspended_ = true;
} else if (ServiceType::kAudio == service_type) {
} else if (ServiceType::kAudio == service_type &&
!audio_streaming_suspended_) {
audio_stream_suspend_timer_.Stop();
application_manager_.OnAppStreaming(app_id(), service_type, false);
application_manager_.OnAppStreaming(
app_id(), service_type, StreamingState::kSuspended);
sync_primitives::AutoLock lock(audio_streaming_suspended_lock_);
audio_streaming_suspended_ = true;
}
Expand All @@ -650,7 +659,7 @@ void ApplicationImpl::SuspendStreaming(
}

void ApplicationImpl::WakeUpStreaming(
protocol_handler::ServiceType service_type, uint32_t timer_len) {
protocol_handler::ServiceType service_type) {
using namespace protocol_handler;
SDL_LOG_AUTO_TRACE();

Expand All @@ -662,28 +671,29 @@ void ApplicationImpl::WakeUpStreaming(
{ // reduce the range of video_streaming_suspended_lock_
sync_primitives::AutoLock lock(video_streaming_suspended_lock_);
if (video_streaming_suspended_) {
application_manager_.OnAppStreaming(app_id(), service_type, true);
application_manager_.OnAppStreaming(
app_id(), service_type, StreamingState::kStarted);
application_manager_.ProcessOnDataStreamingNotification(
service_type, app_id(), true);
video_streaming_suspended_ = false;
}
}
video_stream_suspend_timer_.Start(
timer_len == 0 ? video_stream_suspend_timeout_ : timer_len,
timer::kPeriodic);

video_stream_suspend_timer_.Start(video_stream_suspend_timeout_,
timer::kPeriodic);
} else if (ServiceType::kAudio == service_type) {
{ // reduce the range of audio_streaming_suspended_lock_
sync_primitives::AutoLock lock(audio_streaming_suspended_lock_);
if (audio_streaming_suspended_) {
application_manager_.OnAppStreaming(app_id(), service_type, true);
application_manager_.OnAppStreaming(
app_id(), service_type, StreamingState::kStarted);
application_manager_.ProcessOnDataStreamingNotification(
service_type, app_id(), true);
audio_streaming_suspended_ = false;
}
}
audio_stream_suspend_timer_.Start(
timer_len == 0 ? audio_stream_suspend_timeout_ : timer_len,
timer::kPeriodic);
audio_stream_suspend_timer_.Start(audio_stream_suspend_timeout_,
timer::kPeriodic);
}
}

Expand Down
35 changes: 28 additions & 7 deletions src/components/application_manager/src/application_manager_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3566,7 +3566,9 @@ void ApplicationManagerImpl::ForbidStreaming(
}

void ApplicationManagerImpl::OnAppStreaming(
uint32_t app_id, protocol_handler::ServiceType service_type, bool state) {
uint32_t app_id,
protocol_handler::ServiceType service_type,
const Application::StreamingState new_state) {
SDL_LOG_AUTO_TRACE();

ApplicationSharedPtr app = application(app_id);
Expand All @@ -3577,12 +3579,31 @@ void ApplicationManagerImpl::OnAppStreaming(
}
DCHECK_OR_RETURN_VOID(media_manager_);

if (state) {
state_ctrl_.OnVideoStreamingStarted(app);
media_manager_->StartStreaming(app_id, service_type);
} else {
media_manager_->StopStreaming(app_id, service_type);
state_ctrl_.OnVideoStreamingStopped(app);
SDL_LOG_DEBUG("New state for service " << static_cast<int32_t>(service_type)
<< " is "
<< static_cast<int32_t>(new_state));
switch (new_state) {
case Application::StreamingState::kStopped: {
// Stop activity in media_manager_ when service is stopped
// State controller has been already notified by kSuspended event
// received before
media_manager_->StopStreaming(app_id, service_type);
break;
}

case Application::StreamingState::kStarted: {
// Apply temporary streaming state and start activity in media_manager_
state_ctrl_.OnVideoStreamingStarted(app);
media_manager_->StartStreaming(app_id, service_type);
break;
}

case Application::StreamingState::kSuspended: {
// Don't stop activity in media_manager_ in that case
// Just cancel the temporary streaming state
state_ctrl_.OnVideoStreamingStopped(app);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this change the video state? Video could still be playing on the HMI if the app sent over a long video quickly.

Copy link
Contributor Author

@AKalinich-Luxoft AKalinich-Luxoft Oct 2, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@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.

break;
}
}
}

Expand Down
26 changes: 18 additions & 8 deletions src/components/application_manager/test/application_impl_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -837,15 +837,13 @@ TEST_F(ApplicationImplTest, StartStreaming_StreamingApproved) {
TEST_F(ApplicationImplTest, SuspendNaviStreaming) {
protocol_handler::ServiceType type =
protocol_handler::ServiceType::kMobileNav;
EXPECT_CALL(mock_application_manager_, OnAppStreaming(app_id, type, false));
EXPECT_CALL(mock_application_manager_,
ProcessOnDataStreamingNotification(type, app_id, false));
app_impl->SuspendStreaming(type);
}

TEST_F(ApplicationImplTest, SuspendAudioStreaming) {
protocol_handler::ServiceType type = protocol_handler::ServiceType::kAudio;
EXPECT_CALL(mock_application_manager_, OnAppStreaming(app_id, type, false));
EXPECT_CALL(mock_application_manager_,
ProcessOnDataStreamingNotification(type, app_id, false));
app_impl->SuspendStreaming(type);
Expand All @@ -854,12 +852,16 @@ TEST_F(ApplicationImplTest, SuspendAudioStreaming) {
// TODO {AKozoriz} : Fix tests with streaming (APPLINK-19289)
TEST_F(ApplicationImplTest, DISABLED_Suspend_WakeUpAudioStreaming) {
protocol_handler::ServiceType type = protocol_handler::ServiceType::kAudio;
EXPECT_CALL(mock_application_manager_, OnAppStreaming(app_id, type, false));
EXPECT_CALL(
mock_application_manager_,
OnAppStreaming(app_id, type, Application::StreamingState::kSuspended));
EXPECT_CALL(*MockMessageHelper::message_helper_mock(),
SendOnDataStreaming(type, false, _));
app_impl->SuspendStreaming(type);

EXPECT_CALL(mock_application_manager_, OnAppStreaming(app_id, type, true));
EXPECT_CALL(
mock_application_manager_,
OnAppStreaming(app_id, type, Application::StreamingState::kStarted));
EXPECT_CALL(*MockMessageHelper::message_helper_mock(),
SendOnDataStreaming(type, true, _));
app_impl->WakeUpStreaming(type);
Expand All @@ -868,12 +870,16 @@ TEST_F(ApplicationImplTest, DISABLED_Suspend_WakeUpAudioStreaming) {
TEST_F(ApplicationImplTest, DISABLED_Suspend_WakeUpNaviStreaming) {
protocol_handler::ServiceType type =
protocol_handler::ServiceType::kMobileNav;
EXPECT_CALL(mock_application_manager_, OnAppStreaming(app_id, type, false));
EXPECT_CALL(
mock_application_manager_,
OnAppStreaming(app_id, type, Application::StreamingState::kSuspended));
EXPECT_CALL(*MockMessageHelper::message_helper_mock(),
SendOnDataStreaming(type, false, _));
app_impl->SuspendStreaming(type);

EXPECT_CALL(mock_application_manager_, OnAppStreaming(app_id, type, true));
EXPECT_CALL(
mock_application_manager_,
OnAppStreaming(app_id, type, Application::StreamingState::kStarted));
EXPECT_CALL(*MockMessageHelper::message_helper_mock(),
SendOnDataStreaming(type, true, _));
app_impl->WakeUpStreaming(type);
Expand All @@ -885,7 +891,9 @@ TEST_F(ApplicationImplTest, StopStreaming_StreamingApproved) {
protocol_handler::ServiceType::kMobileNav;
app_impl->set_video_streaming_approved(true);

EXPECT_CALL(mock_application_manager_, OnAppStreaming(app_id, type, false));
EXPECT_CALL(
mock_application_manager_,
OnAppStreaming(app_id, type, Application::StreamingState::kStopped));
EXPECT_CALL(mock_application_manager_,
ProcessOnDataStreamingNotification(type, app_id, false));
EXPECT_CALL(*MockMessageHelper::message_helper_mock(),
Expand All @@ -897,7 +905,9 @@ TEST_F(ApplicationImplTest, StopStreaming_StreamingApproved) {
// Stop audio streaming
app_impl->set_audio_streaming_approved(true);
type = protocol_handler::ServiceType::kAudio;
EXPECT_CALL(mock_application_manager_, OnAppStreaming(app_id, type, false));
EXPECT_CALL(
mock_application_manager_,
OnAppStreaming(app_id, type, Application::StreamingState::kStopped));
EXPECT_CALL(mock_application_manager_,
ProcessOnDataStreamingNotification(type, app_id, false));
EXPECT_CALL(*MockMessageHelper::message_helper_mock(),
Expand Down
5 changes: 2 additions & 3 deletions src/components/application_manager/test/include/application_manager/mock_application.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,8 @@ class MockApplication : public ::application_manager::Application {
void(protocol_handler::ServiceType service_type));
MOCK_METHOD1(SuspendStreaming,
void(protocol_handler::ServiceType service_type));
MOCK_METHOD2(WakeUpStreaming,
void(protocol_handler::ServiceType service_type,
uint32_t timer_len));
MOCK_METHOD1(WakeUpStreaming,
void(protocol_handler::ServiceType service_type));
MOCK_CONST_METHOD0(is_voice_communication_supported, bool());
MOCK_METHOD1(set_voice_communication_supported,
void(bool is_voice_communication_supported));
Expand Down
4 changes: 2 additions & 2 deletions src/components/include/application_manager/application_manager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -813,11 +813,11 @@ class ApplicationManager {
* @brief Callback calls when application starts/stops data streaming
* @param app_id Streaming application id
* @param service_type Streaming service type
* @param state Shows if streaming started or stopped
* @param new_state Defines new streaming state
*/
virtual void OnAppStreaming(uint32_t app_id,
protocol_handler::ServiceType service_type,
bool state) = 0;
const Application::StreamingState new_state) = 0;

/**
* @brief CreateRegularState create regular HMI state for application
Expand Down
8 changes: 0 additions & 8 deletions src/components/include/media_manager/media_manager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,6 @@ class MediaManager {
*/
virtual const MediaManagerSettings& settings() const = 0;

/**
* \brief Convert an amount of audio bytes to an estimated time in ms
* \param data_size number of bytes to be played
* \return milliseconds required to play <data_size> many bytes with
* the current pcm stream capabilities
*/
virtual uint32_t DataSizeToMilliseconds(uint64_t data_size) const = 0;

virtual ~MediaManager() {}
};

Expand Down
9 changes: 5 additions & 4 deletions src/components/include/test/application_manager/mock_application_manager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,10 +309,11 @@ class MockApplicationManager : public application_manager::ApplicationManager {
MOCK_METHOD1(OnAppUnauthorized, void(const uint32_t& app_id));
MOCK_METHOD1(ActivateApplication,
bool(application_manager::ApplicationSharedPtr app));
MOCK_METHOD3(OnAppStreaming,
void(uint32_t app_id,
protocol_handler::ServiceType service_type,
bool state));
MOCK_METHOD3(
OnAppStreaming,
void(uint32_t app_id,
protocol_handler::ServiceType service_type,
application_manager::Application::StreamingState new_state));
MOCK_CONST_METHOD6(CreateRegularState,
application_manager::HmiStatePtr(
application_manager::ApplicationSharedPtr app,
Expand Down
1 change: 0 additions & 1 deletion src/components/include/test/media_manager/mock_media_manager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ class MockMediaManager : public media_manager::MediaManager {
MOCK_METHOD2(FramesProcessed,
void(int32_t application_key, int32_t frame_number));
MOCK_CONST_METHOD0(settings, const media_manager::MediaManagerSettings&());
MOCK_CONST_METHOD1(DataSizeToMilliseconds, uint32_t(uint64_t data_size));
};

} // namespace media_manager_test
Expand Down
10 changes: 0 additions & 10 deletions src/components/media_manager/include/media_manager/media_manager_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
#ifndef SRC_COMPONENTS_MEDIA_MANAGER_INCLUDE_MEDIA_MANAGER_MEDIA_MANAGER_IMPL_H_
#define SRC_COMPONENTS_MEDIA_MANAGER_INCLUDE_MEDIA_MANAGER_MEDIA_MANAGER_IMPL_H_

#include <chrono>
#include <map>
#include <string>
#include "interfaces/MOBILE_API.h"
Expand Down Expand Up @@ -80,7 +79,6 @@ class MediaManagerImpl : public MediaManager,
protocol_handler::ServiceType service_type);
virtual void StopStreaming(int32_t application_key,
protocol_handler::ServiceType service_type);

virtual void SetProtocolHandler(
protocol_handler::ProtocolHandler* protocol_handler);
virtual void OnMessageReceived(
Expand All @@ -91,8 +89,6 @@ class MediaManagerImpl : public MediaManager,

virtual const MediaManagerSettings& settings() const OVERRIDE;

virtual uint32_t DataSizeToMilliseconds(uint64_t data_size) const OVERRIDE;

#ifdef BUILD_TESTS
void set_mock_a2dp_player(MediaAdapter* media_adapter);
void set_mock_mic_listener(MediaListenerPtr media_listener);
Expand All @@ -118,12 +114,6 @@ class MediaManagerImpl : public MediaManager,
std::map<protocol_handler::ServiceType, MediaAdapterImplPtr> streamer_;
std::map<protocol_handler::ServiceType, MediaListenerPtr> streamer_listener_;

uint32_t bits_per_sample_;
uint32_t sampling_rate_;
uint64_t stream_data_size_;
std::chrono::time_point<std::chrono::system_clock>
socket_audio_stream_start_time_;

application_manager::ApplicationManager& application_manager_;

private:
Expand Down
2 changes: 1 addition & 1 deletion src/components/media_manager/include/media_manager/streamer_adapter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ class StreamerAdapter : public MediaAdapterImpl {
};

private:
int32_t current_application_;
std::atomic_int current_application_;
utils::MessageQueue<protocol_handler::RawMessagePtr> messages_;

Streamer* streamer_;
Expand Down
Loading