[MM] Fix flickering when playing video.
authormsu.koo <msu.koo@samsung.com>
Wed, 9 Dec 2015 10:18:47 +0000 (19:18 +0900)
committerYoungsoo Choi <kenshin.choi@samsung.com>
Tue, 10 Jul 2018 07:55:23 +0000 (07:55 +0000)
Previously, Player deletes |tbm_surface| when next frame is delivered.
But occationally, it causes frame drops and crashs
because |tbm_surface| is released before the gpu does not finish to handle it.

This patch introduces |MediaPacketManager|
to hold media_packets which wraps tbm_surface and CleanUp regularly.

Also supplying APIs to let Renderer explicitly release the |tbm_surface|s
by indexing the |media_packet|s using |tbm_surface_h| as the key.
(Renderer only knows tbm_surface.)

CleanUp is introduced to prevent media_packet(tbm_surface) leaks
even Renderer does not request to release.
Theoritically CleanUp should do nothing because Renderer invoke
Release() for all |tbm_surface|s afer rendering so that
implementation is focused on optimizing Release() rather than CleanUp().

Bug: http://165.213.149.170/jira/browse/TSAM-653
Bug: http://107.108.218.239/bugzilla/show_bug.cgi?id=15255

Reviewed by: sns.park

Change-Id: I9a1e3b0710da0072b3459818690d16e5c43d6fd3
Signed-off-by: msu.koo <msu.koo@samsung.com>
tizen_src/chromium_impl/media/base/tizen/media_packet_manager.cc [new file with mode: 0644]
tizen_src/chromium_impl/media/base/tizen/media_packet_manager.h [new file with mode: 0644]
tizen_src/chromium_impl/media/base/tizen/media_player_bridge_capi.cc
tizen_src/chromium_impl/media/base/tizen/media_player_bridge_capi.h
tizen_src/chromium_impl/media/base/tizen/media_source_player_capi.cc
tizen_src/chromium_impl/media/base/tizen/media_source_player_capi.h
tizen_src/chromium_impl/media/media_efl.gypi

diff --git a/tizen_src/chromium_impl/media/base/tizen/media_packet_manager.cc b/tizen_src/chromium_impl/media/base/tizen/media_packet_manager.cc
new file mode 100644 (file)
index 0000000..5b5880a
--- /dev/null
@@ -0,0 +1,88 @@
+// Copyright 2015 Samsung Electronics Inc. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "media/base/tizen/media_packet_manager.h"
+
+#include "base/memory/singleton.h"
+#include "content/public/browser/browser_thread.h"
+
+namespace {
+
+const base::TimeDelta kCleanUpInterval =
+    base::TimeDelta::FromMilliseconds(10000);
+const base::TimeDelta kSurfaceLifetime =
+    base::TimeDelta::FromMilliseconds(1000);
+
+}  // namespace
+
+namespace media {
+
+void ReleaseTizenSurface(tbm_surface_h internal_surface) {
+  MediaPacketManager::GetInstance()->Release(internal_surface);
+}
+
+MediaPacketProxy::MediaPacketProxy(media_packet_h handle)
+    : mediapacket_handle(handle), tbmsurface_handle(nullptr) {
+  if (MEDIA_PACKET_ERROR_NONE !=
+      media_packet_get_tbm_surface(mediapacket_handle, &tbmsurface_handle)) {
+    LOG(ERROR) << "Invalid |mediapacket_handle|: " << handle;
+  }
+  pushed_time = base::Time::Now();
+}
+
+MediaPacketProxy::~MediaPacketProxy() {
+  if (mediapacket_handle)
+    media_packet_destroy(mediapacket_handle);
+}
+
+MediaPacketManager* MediaPacketManager::GetInstance() {
+  return base::Singleton<MediaPacketManager>::get();
+}
+
+void MediaPacketManager::Push(scoped_ptr<MediaPacketProxy> media_packet) {
+  content::BrowserThread::PostTask(
+      content::BrowserThread::IO, FROM_HERE,
+      base::Bind(&MediaPacketManager::DoPush,
+                 base::Unretained(this), base::Passed(media_packet.Pass())));
+}
+
+void MediaPacketManager::Release(tbm_surface_h handle) {
+  content::BrowserThread::PostTask(
+      content::BrowserThread::IO, FROM_HERE,
+      base::Bind(&MediaPacketManager::DoRelease,
+                 base::Unretained(this), handle));
+}
+
+MediaPacketManager::MediaPacketManager() {
+  CleanUp();
+}
+
+void MediaPacketManager::CleanUp() {
+  for (auto itr = mediapacket_map_.begin(); itr != mediapacket_map_.end();) {
+    if (itr->second->pushed_time < base::Time::Now() - kSurfaceLifetime) {
+      DLOG(INFO) << "Cleaned: " << itr->second->tbmsurface_handle;
+      itr = mediapacket_map_.erase(itr);
+    } else {
+      ++itr;
+    }
+  }
+
+  content::BrowserThread::PostDelayedTask(
+      content::BrowserThread::IO, FROM_HERE,
+      base::Bind(&MediaPacketManager::CleanUp, base::Unretained(this)),
+      kCleanUpInterval);
+}
+
+void MediaPacketManager::DoPush(scoped_ptr<MediaPacketProxy> media_packet) {
+  size_t index = reinterpret_cast<size_t>(media_packet->tbmsurface_handle);
+  DLOG(INFO) << "Pushed: " << media_packet->tbmsurface_handle;
+  mediapacket_map_.insert(std::make_pair(index, media_packet.Pass()));
+}
+
+void MediaPacketManager::DoRelease(tbm_surface_h handle) {
+  DLOG(INFO) << "Released: " << handle;
+  mediapacket_map_.erase(reinterpret_cast<size_t>(handle));
+}
+
+}  // namespace media
diff --git a/tizen_src/chromium_impl/media/base/tizen/media_packet_manager.h b/tizen_src/chromium_impl/media/base/tizen/media_packet_manager.h
new file mode 100644 (file)
index 0000000..390eff1
--- /dev/null
@@ -0,0 +1,52 @@
+// Copyright 2015 Samsung Electronics Inc. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef MEDIA_BASE_TIZEN_MEDIA_PACKET_MANAGER_H_
+#define MEDIA_BASE_TIZEN_MEDIA_PACKET_MANAGER_H_
+
+#include <media_packet.h>
+#include <tbm_surface.h>
+#include <unordered_map>
+
+#include "base/memory/scoped_ptr.h"
+#include "base/time/time.h"
+#include "media/base/media_export.h"
+
+namespace base {
+template <typename T> struct DefaultSingletonTraits;
+}  // namespace base
+
+namespace media {
+
+struct MEDIA_EXPORT MediaPacketProxy final {
+  media_packet_h mediapacket_handle;
+  tbm_surface_h tbmsurface_handle;
+  base::Time pushed_time;
+
+  explicit MediaPacketProxy(media_packet_h handle);
+  ~MediaPacketProxy();
+};
+
+class MEDIA_EXPORT MediaPacketManager {
+ public:
+  static MediaPacketManager* GetInstance();
+
+  void Push(scoped_ptr<MediaPacketProxy> media_packet);
+  void Release(tbm_surface_h handle);
+
+ private:
+  friend struct base::DefaultSingletonTraits<MediaPacketManager>;
+  MediaPacketManager();
+  void CleanUp();
+  void DoPush(scoped_ptr<MediaPacketProxy> media_packet);
+  void DoRelease(tbm_surface_h handle);
+
+  std::unordered_map<size_t, scoped_ptr<MediaPacketProxy>> mediapacket_map_;
+
+  DISALLOW_COPY_AND_ASSIGN(MediaPacketManager);
+};
+
+}  // namespace media
+
+#endif  // MEDIA_BASE_TIZEN_MEDIA_PACKET_MANAGER_H_
index d736c47..8c45595 100644 (file)
@@ -77,10 +77,8 @@ static void MediaPacketDecodedCb(media_packet_h packet, void* data) {
   media::MediaPlayerBridgeCapi* player =
       static_cast<media::MediaPlayerBridgeCapi*>(data);
 
-  scoped_ptr<media::MediaPacketHolder> packet_holder(
-      new media::MediaPacketHolder(packet));
-
-  player->OnMediaPacketUpdated(packet_holder.Pass());
+  player->OnMediaPacketUpdated(
+      scoped_ptr<media::MediaPacketProxy>(new media::MediaPacketProxy(packet)));
 }
 
 // Called by player_set_completed_cb()
@@ -137,13 +135,6 @@ MediaPlayerEfl* MediaPlayerEfl::CreatePlayer(int player_id, const GURL& url,
   return new MediaPlayerBridgeCapi(player_id, url, volume, manager, user_agent);
 }
 
-MediaPacketHolder::MediaPacketHolder(media_packet_h handle)
-    : media_packet_handle(handle) {}
-
-MediaPacketHolder::~MediaPacketHolder() {
-  if (media_packet_handle) media_packet_destroy(media_packet_handle);
-}
-
 MediaPlayerBridgeCapi::MediaPlayerBridgeCapi(
     int player_id,
     const GURL& url,
@@ -636,11 +627,9 @@ void MediaPlayerBridgeCapi::PlayerPrepared() {
 }
 
 void MediaPlayerBridgeCapi::MediaPacketUpdated(
-    scoped_ptr<MediaPacketHolder> packet) {
-  tbm_surface_h surface;
-  media_packet_get_tbm_surface(packet->media_packet_handle, &surface);
-  SendFrame(surface);
-  previous_packet_ = packet.Pass();
+    scoped_ptr<MediaPacketProxy> packet) {
+  SendFrame(packet->tbmsurface_handle);
+  MediaPacketManager::GetInstance()->Push(packet.Pass());
 }
 
 void MediaPlayerBridgeCapi::SendFrame(tbm_surface_h surface) {
@@ -838,7 +827,7 @@ void MediaPlayerBridgeCapi::ExecuteDelayedPlayerState() {
 }
 
 void MediaPlayerBridgeCapi::OnMediaPacketUpdated(
-    scoped_ptr<MediaPacketHolder> packet) {
+    scoped_ptr<MediaPacketProxy> packet) {
   task_runner_->PostTask(
       FROM_HERE, base::Bind(&MediaPlayerBridgeCapi::MediaPacketUpdated,
                             weak_factory_.GetWeakPtr(), base::Passed(&packet)));
index 2d6908b..3f4ad59 100644 (file)
@@ -14,6 +14,7 @@
 #include "content/public/browser/browser_message_filter.h"
 #include "media/base/ranges.h"
 #include "media/base/efl/media_player_efl.h"
+#include "media/base/tizen/media_packet_manager.h"
 #include "media/base/video_frame.h"
 
 #include <media_packet.h>
 
 namespace media {
 
-struct MediaPacketHolder final {
-  media_packet_h media_packet_handle;
-
-  explicit MediaPacketHolder(media_packet_h handle);
-  ~MediaPacketHolder();
-};
-
 class MEDIA_EXPORT MediaPlayerBridgeCapi
     : public MediaPlayerEfl {
  public:
@@ -52,7 +46,7 @@ class MEDIA_EXPORT MediaPlayerBridgeCapi
 
   void ExecuteDelayedPlayerState();
 
-  void OnMediaPacketUpdated(scoped_ptr<MediaPacketHolder> packet);
+  void OnMediaPacketUpdated(scoped_ptr<MediaPacketProxy> packet);
   void SendFrame(tbm_surface_h surface);
 
   void OnPlaybackCompleteUpdate();
@@ -83,7 +77,7 @@ class MEDIA_EXPORT MediaPlayerBridgeCapi
   void UpdateSeekState(bool state);
   void UpdateDuration();
 
-  void MediaPacketUpdated(scoped_ptr<MediaPacketHolder> packet);
+  void MediaPacketUpdated(scoped_ptr<MediaPacketProxy> packet);
   void PlaybackCompleteUpdate();
   void SeekCompleteUpdate();
   void PlayerPrepared();
@@ -94,8 +88,6 @@ class MEDIA_EXPORT MediaPlayerBridgeCapi
   player_state_e GetPlayerState();
 
  private:
-  scoped_ptr<MediaPacketHolder> previous_packet_;
-
   const scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
 
   player_h player_;
index f30ef3e..815404b 100644 (file)
@@ -6,6 +6,7 @@
 #include "base/thread_task_runner_handle.h"
 #include "content/browser/media/efl/browser_demuxer_efl.h"
 #include "media/base/efl/media_player_manager_efl.h"
+#include "media/base/tizen/media_packet_manager.h"
 #include "media/base/tizen/media_source_player_capi.h"
 #include "third_party/libyuv/include/libyuv.h"
 
@@ -213,7 +214,6 @@ MediaSourcePlayerCapi::MediaSourcePlayerCapi(
       native_window_(0),
 #else
       media_packet_(NULL),
-      previous_packet_(NULL),
 #endif
       audio_format_(NULL),
       video_format_(NULL) {
@@ -448,14 +448,10 @@ void MediaSourcePlayerCapi::MoveAndresizeWindow(int x, int y, int width,
 }
 #else
 void MediaSourcePlayerCapi::MediaPacketUpdated(media_packet_h packet) {
-  // If packets are destroyed right away, error logs about TBM occur.
-  if (previous_packet_)
-    media_packet_destroy(previous_packet_);
-
-  tbm_surface_h surface = nullptr;
-  media_packet_get_tbm_surface(packet, &surface);
-  SendFrame(surface);
-  previous_packet_ = packet;
+  scoped_ptr<media::MediaPacketProxy> packet_proxy(
+      new media::MediaPacketProxy(packet));
+  SendFrame(packet_proxy->tbmsurface_handle);
+  MediaPacketManager::GetInstance()->Push(packet_proxy.Pass());
 }
 
 void MediaSourcePlayerCapi::SendFrame(tbm_surface_h surface) {
@@ -669,9 +665,6 @@ void MediaSourcePlayerCapi::Release() {
   if (media_packet_)
     media_packet_destroy(media_packet_);
   media_packet_ = NULL;
-  if (previous_packet_)
-    media_packet_destroy(previous_packet_);
-  previous_packet_ = NULL;
 #endif
 }
 
index 7ce83ec..bb1a013 100644 (file)
@@ -153,7 +153,6 @@ class MEDIA_EXPORT MediaSourcePlayerCapi
   unsigned native_window_;
 #else
   media_packet_h media_packet_;
-  media_packet_h previous_packet_;
 #endif
 
   // FIXME(Venu): Make them a smart pointer to avoid leak.
index bf1212f..ee5ac69 100644 (file)
               }],
               ['tizen_multimedia_use_capi_for_me==1', {
                 'sources': [
+                  'base/tizen/media_packet_manager.cc',
+                  'base/tizen/media_packet_manager.h',
                   'base/tizen/media_player_bridge_capi.cc',
                   'base/tizen/media_player_bridge_capi.h', # ME
                   'base/tizen/media_source_player_capi.cc',