[GS][WebRTC] Fix ESPlusPlayer crash when application is being suspended

[Problem]
Assertion failed on ESPlusPlayer internal thread.

[Cause]
`nullptr` was provided to `esplusplayer_set_display` method as `window`
parameter. It seems that ESPlusPlayer is not validating input parameters,
which resulted in crash.

`nullptr` was received from `media::MediaPlayerEfl::main_window_handle()`
method, due to Suspend operation being performed on main browser thread.
`WRTNativeWindow` class was destroyed, which resulted in calling
`media::MediaPlayerEfl::SetSharedVideoWindowHandle` with `nullptr`.

[Solution]
1) Ensure that `RenderWidgetHostVisibilityChanged` will always be propagated to
`AudioManager` and `VideoManager`, to ensure creation of Players
with proper state (kUninitialized or kSuspended).

2) Validate `main_window_handle()` returned from `media::MediaPlayerEfl` before
passing it to ESPlusPlayer - in case of invalid parameter, fail `PrepareStep`.

Bug: https://jira-eu.sec.samsung.net/browse/VDWASM-1407
Change-Id: I42878e480aa3731ed0fa048ccc288dc55fc5487d
Signed-off-by: Michal Jurkiewicz <m.jurkiewicz@samsung.com>

diff --git a/tizen_src/chromium_impl/media/filters/tizen/stream_player_manager_impl.cc b/tizen_src/chromium_impl/media/filters/tizen/stream_player_manager_impl.cc
index b2d72ef66c734c777130bbd240cabff7cc23da12..d8b32b638d887dbf42913cd3369bdd428857b6db 100644 (file)
--- a/tizen_src/chromium_impl/media/filters/tizen/stream_player_manager_impl.cc
+++ b/tizen_src/chromium_impl/media/filters/tizen/stream_player_manager_impl.cc
@@ -184,28 +184,30 @@ void StreamPlayerManagerImpl::RenderWidgetHostVisibilityChanged(
   auto video_cb =
       became_visible ? &VideoManager::ResumeAll : &VideoManager::SuspendAll;
 
-  // When threads are not running, it means we got no player instances.
-  // No need to do anything in such case.
-  if (video_thread_.IsRunning()) {
-    if (!video_thread_.task_runner()->PostTask(
-            FROM_HERE,
-            base::BindOnce(video_cb, base::Unretained(video_manager_.get())))) {
-      LOG_ID(ERROR, this)
-          << "Could not post VideoManager [SuspendAll|ResumeAll] task";
-    }
-  } else {
-    LOG_ID(INFO, this) << "Video thread is not running";
+  EnsureVideoThreadStarted();
+  if (!video_thread_.IsRunning()) {
+    LOG_ID(ERROR, this) << "Video thread is not running";
+    return;
   }
 
-  if (audio_thread_.IsRunning()) {
-    if (!audio_thread_.task_runner()->PostTask(
-            FROM_HERE,
-            base::BindOnce(audio_cb, base::Unretained(audio_manager_.get())))) {
-      LOG_ID(ERROR, this)
-          << "Could not post AudioManager [SuspendAll|ResumeAll] task";
-    }
-  } else {
-    LOG_ID(INFO, this) << "Audio thread is not running";
+  if (!video_thread_.task_runner()->PostTask(
+          FROM_HERE,
+          base::BindOnce(video_cb, base::Unretained(video_manager_.get())))) {
+    LOG_ID(ERROR, this)
+        << "Could not post VideoManager [SuspendAll|ResumeAll] task";
+  }
+
+  EnsureAudioThreadStarted();
+  if (!audio_thread_.IsRunning()) {
+    LOG_ID(ERROR, this) << "Audio thread is not running";
+    return;
+  }
+
+  if (!audio_thread_.task_runner()->PostTask(
+          FROM_HERE,
+          base::BindOnce(audio_cb, base::Unretained(audio_manager_.get())))) {
+    LOG_ID(ERROR, this)
+        << "Could not post AudioManager [SuspendAll|ResumeAll] task";
   }
 }
 

diff --git a/tizen_src/chromium_impl/media/filters/tizen/stream_player_video_impl.cc b/tizen_src/chromium_impl/media/filters/tizen/stream_player_video_impl.cc
index f40261a91bb1aa77a2c9f154674ca6b21dab98c2..d5af9173f3b6a42bdc535f49a3c468ab8a37242f 100644 (file)
--- a/tizen_src/chromium_impl/media/filters/tizen/stream_player_video_impl.cc
+++ b/tizen_src/chromium_impl/media/filters/tizen/stream_player_video_impl.cc
@@ -56,10 +56,17 @@ bool StreamPlayerVideoImpl::PrepareStep() {
   CHECK(GetPlayerState() == PlayerState::kPreparing ||
         GetPlayerState() == PlayerState::kResuming);
 
-  CHECK_PLAYER_RESULT(
-      esplusplayer_set_display(player_, ESPLUSPLAYER_DISPLAY_TYPE_OVERLAY,
-                               media::MediaPlayerEfl::main_window_handle()),
-      false);
+  auto* window = media::MediaPlayerEfl::main_window_handle();
+  if (!window) {
+    SP_LOG_TYPED(ERROR)
+        << "Null window handle provided - could not further proceed "
+           "with player prepare.";
+    return false;
+  }
+
+  CHECK_PLAYER_RESULT(esplusplayer_set_display(
+                          player_, ESPLUSPLAYER_DISPLAY_TYPE_OVERLAY, window),
+                      false);
   CHECK_PLAYER_RESULT(
       esplusplayer_set_display_mode(player_, ESPLUSPLAYER_DISPLAY_MODE_DST_ROI),
       false);
@@ -212,8 +219,8 @@ void StreamPlayerVideoImpl::SetGeometry(const gfx::RectF& geometry,
   // According to ESPlusPlayer API documentation, setting region of interest
   // needs to be performed after setting display. Setting display is being
   // performed during resuming/preparing stage, so if we are currently
-  // resuming/preparing, it is better to return here and wait for `StartStep()`,
-  // which will update geometry.
+  // resuming/preparing, it is better to return here and wait for
+  // `StartStep()`, which will update geometry.
   if (GetPlayerState() != PlayerState::kPlaying) {
     SP_LOG_TYPED(INFO);
     return;
@@ -228,8 +235,8 @@ void StreamPlayerVideoImpl::SetVisibility(bool visible) {
   // According to ESPlusPlayer API documentation, setting visibility
   // needs to be performed after setting display. Setting display is being
   // performed during resuming/preparing stage, so if we are currently
-  // resuming/preparing, it is better to return here and wait for `StartStep()`,
-  // which will update visibility.
+  // resuming/preparing, it is better to return here and wait for
+  // `StartStep()`, which will update visibility.
   if (GetPlayerState() != PlayerState::kPlaying) {
     SP_LOG_TYPED(INFO);
     return;
@@ -247,8 +254,8 @@ bool StreamPlayerVideoImpl::GetDroppedFrameCount(uint32_t* count) {
     return true;
   }
 
-  // According to ESPlusPlayer API description, `esplusplayer_get_adaptive_info`
-  // needs to be called on prepared player.
+  // According to ESPlusPlayer API description,
+  // `esplusplayer_get_adaptive_info` needs to be called on prepared player.
   if (GetPlayerState() != PlayerState::kPlaying) {
     SP_LOG_TYPED(INFO)
         << "Cannot get dropped frame count of not prepared player";

diff --git a/tizen_src/chromium_impl/third_party/blink/renderer/platform/peerconnection/rtc_video_decoder_efl_low_latency.cc b/tizen_src/chromium_impl/third_party/blink/renderer/platform/peerconnection/rtc_video_decoder_efl_low_latency.cc
index e8ced54657d86423e220c09f9ff694dde3ccf7ef..a922a9e6f1c8eb247b74cbebe2851bcc879f0d63 100644 (file)
--- a/tizen_src/chromium_impl/third_party/blink/renderer/platform/peerconnection/rtc_video_decoder_efl_low_latency.cc
+++ b/tizen_src/chromium_impl/third_party/blink/renderer/platform/peerconnection/rtc_video_decoder_efl_low_latency.cc
@@ -515,6 +515,10 @@ void RTCVideoDecoderEflLowLatency::PlayerErrorOnPlayerThread(
       // fall through
     // No need to do anything
     case media::tizen::PlayerStatus::kSuccessDoNotFeed:
+      if (backend_error_) {
+        LOG_ID(INFO, this) << "Recovering from backend_error";
+        backend_error_ = false;
+      }
       break;
     case media::tizen::PlayerStatus::kNeedsKeyframe:
       needs_keyframe_ = true;