[MM] Fix random crashes and flickering while playing video.
authormsu.koo <msu.koo@samsung.com>
Mon, 21 Dec 2015 11:01:41 +0000 (20:01 +0900)
committerYoungsoo Choi <kenshin.choi@samsung.com>
Tue, 10 Jul 2018 07:55:23 +0000 (07:55 +0000)
Random crash and flickering happens because
- Lifecycle between BrowserMediaPlayerManagerEfl(BMPMEfl) and
MediaPlayerEfls(MPEfl) is not properly managed.
- Lifecycle of delivered media_packet is not properly managed so that
Renderer occasionally accesses media_packet which was already destroyed.

Bundle of patches fix theses issues by
- Introducing weak pointer when posting tasks not to make task outlive
than tasker.
- Making MPEfl's lifecycle dependent with BMPMEfl.
- Introducing IPC for destroying media_packet explicitly after Renderer
uses it.
- Introducing scoped_ptr for media_packet not to make leaks even task
is cancelled.

In detail, this patch consists of following beta/m47 patches:
1. http://165.213.202.130/gerrit/#/c/100528/ by msu.koo
Previously, crash happens on refresh while playing videos using HTML's
iframe tag, because |OnNewTbmBufferAvailable| is queued after RenderFrame
is deleted, so it refers already-deleted RenderFrame.
This patch includes 2 major changes.
- On Renderer-side,
This separates OnNewTbmBufferAvailable() from RenderFrame's lifecycle.
- On Browser-side,
This introduces BrowserMediaPacketManager which is Control IPC handler
outlives than BrowsermediaPlayerManager. This class can handle releasing
|media_packet| without affected by Player's lifecycle.

2. http://165.213.202.130/gerrit/#/c/100838/ by sangdeug.kim
This patch fixes crash during run |MediaPlayerEfl::DeliverMediaPacket| by
checking the destructing state.

3. http://165.213.202.130/gerrit/#/c/100839/ by msu.koo
Previously, random crash happens because player accesses manager
without considering the manager's lifecycle.
Currently, BrowserMediaPlayerManager request to delete Players
when it's deconstructed, which means that Player can outlive
than Manager.
This fix introduces WeakPtr and let Player access Manager
via WeakPtr for lifetime handling between Player and Manager.

4. http://165.213.202.130/gerrit/#/c/102313/ by msu.koo
"Improve MediaPlayerEfl lifecycle management."
Previously, MediaPlayerEfl outlives than BrowserMediaPlayerManagerEfl
so additional checking if Manager is alive before callback.
This patch make MediaPlayerEfl's lifetime dependant with BMPMEfl
and remove above checking routines.
Also remove IsPlayerDestructing and relates, which is no more required
after introducing WeakPtr for PostTask.

5. http://165.213.202.130/gerrit/#/c/102463/ by msu.koo
"Fix possible |media_packet| leaks when DeliverMediaPacket is cancelled."
Previously, DeliverMediaPacket task can be cancelled if queued after
MediaPlayerEfl is destroyed. But when DeliverMediaPacket is cancelled,
there is no handling to destroy the passed |media_packet| on it, so it
causes |media_packet| leaks.
This patch introduces MediaPacketProxy and scoped_ptr to fix it.

6. http://165.213.202.130/gerrit/#/c/103502/ by sangdeug.kim
After making MediaPlayerEfl's lifetime dependent with
BrowserMediaPlayerManager(BMPM), crash happens while destructing player.
Root cause is
- WeakPtr of BMPM is invalidated before Players are destroyed
by the destruction sequence of member variables.
- While destructing Players, Player accesses BMPM
via already-invalidated WeakPtr -> Crash happens.
This patch removes redundant WeakPtr of BMPM and related codes,
and let Player access Manager with raw pointer.
This patch also ensure destroy all players before BMPM is destroyed.

7. http://165.213.202.130/gerrit/#/c/103039/ by msu.koo
Previous implementation implements Proxy Class for media_packet,
but it can be replaced with Chromium's scoped_ptr.

Together with: Iecc266fee83ae009285554dce4581d6c7e9bf545

Bug: http://10.113.136.204/jira/browse/TSAM-751
Bug: http://10.113.136.204/jira/browse/TSAM-836
Bug: http://web.sec.samsung.net/bugzilla/show_bug.cgi?id=15296
Bug: http://web.sec.samsung.net/bugzilla/show_bug.cgi?id=15410
Bug: http://web.sec.samsung.net/bugzilla/show_bug.cgi?id=15425
Bug: http://web.sec.samsung.net/bugzilla/show_bug.cgi?id=15439
Bug: http://web.sec.samsung.net/bugzilla/show_bug.cgi?id=15445
Bug: http://web.sec.samsung.net/bugzilla/show_bug.cgi?id=15458
Bug: http://web.sec.samsung.net/bugzilla/show_bug.cgi?id=15502

Reviewed by: msu.koo, sm.venugopal, sns.park

Change-Id: I59747d19e18cdcb8708d6ba91c49bc2eb7fad66b
Signed-off-by: msu.koo <msu.koo@samsung.com>
Signed-off-by: sangdeug.kim <sangdeug.kim@samsung.com>
17 files changed:
tizen_src/chromium_impl/content/browser/media/browser_mediapacket_manager.cc [new file with mode: 0644]
tizen_src/chromium_impl/content/browser/media/browser_mediapacket_manager.h [new file with mode: 0644]
tizen_src/chromium_impl/content/browser/media/efl/browser_media_player_manager_efl.cc
tizen_src/chromium_impl/content/browser/media/efl/browser_media_player_manager_efl.h
tizen_src/chromium_impl/content/browser/media/media_web_contents_observer_efl.cc
tizen_src/chromium_impl/content/common/media/efl/media_player_messages_efl.h
tizen_src/chromium_impl/content/content_browser_efl.gypi
tizen_src/chromium_impl/content/renderer/media/efl/renderer_media_player_manager_efl.cc
tizen_src/chromium_impl/content/renderer/media/efl/renderer_media_player_manager_efl.h
tizen_src/chromium_impl/media/base/efl/media_player_efl.cc
tizen_src/chromium_impl/media/base/efl/media_player_efl.h
tizen_src/chromium_impl/media/base/efl/media_player_util_efl.cc
tizen_src/chromium_impl/media/base/efl/media_player_util_efl.h
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

diff --git a/tizen_src/chromium_impl/content/browser/media/browser_mediapacket_manager.cc b/tizen_src/chromium_impl/content/browser/media/browser_mediapacket_manager.cc
new file mode 100644 (file)
index 0000000..4b30772
--- /dev/null
@@ -0,0 +1,54 @@
+// 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 "content/browser/media/browser_mediapacket_manager.h"
+
+#include <media_packet.h>
+
+#include "content/common/media/efl/media_player_messages_efl.h"
+#include "content/public/browser/browser_thread.h"
+#include "ipc/ipc_message_macros.h"
+
+namespace {
+
+void DoDestroyMediaPacket(media_packet_h packet) {
+  if (MEDIA_PACKET_ERROR_NONE != media_packet_destroy(packet))
+    LOG(WARNING) << "Fail to release media_packet";
+}
+
+}  // namespace
+
+namespace content {
+
+scoped_refptr<BrowserMessageFilter> CreateBrowserMediapacketManager() {
+  return new BrowserMediaPacketManager();
+}
+
+BrowserMediaPacketManager::BrowserMediaPacketManager()
+    : BrowserMessageFilter(MediaPlayerMsgStart) {}
+
+BrowserMediaPacketManager::~BrowserMediaPacketManager() {}
+
+bool BrowserMediaPacketManager::OnMessageReceived(
+    const IPC::Message& message) {
+  bool handled = true;
+  IPC_BEGIN_MESSAGE_MAP(BrowserMediaPacketManager, message)
+    IPC_MESSAGE_HANDLER(MediaPlayerEflHostMsg_ReleaseTbmBuffer,
+                        ReleaseMediaPacket)
+    IPC_MESSAGE_UNHANDLED(handled = false)
+  IPC_END_MESSAGE_MAP()
+  return handled;
+}
+
+void BrowserMediaPacketManager::ReleaseMediaPacket(
+    gfx::TbmBufferHandle packet) {
+  DLOG(INFO) << "ReleaseMediaPacket, media_packet: " << packet.media_packet;
+  BrowserThread::PostDelayedTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(&DoDestroyMediaPacket,
+                 static_cast<media_packet_h>(packet.media_packet)),
+      base::TimeDelta::FromMilliseconds(40));
+}
+
+}  // namespace content
diff --git a/tizen_src/chromium_impl/content/browser/media/browser_mediapacket_manager.h b/tizen_src/chromium_impl/content/browser/media/browser_mediapacket_manager.h
new file mode 100644 (file)
index 0000000..5250019
--- /dev/null
@@ -0,0 +1,30 @@
+// 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 CONTENT_BROWSER_MEDIA_BROWSER_MEDIAPACKET_MANAGER_H_
+#define CONTENT_BROWSER_MEDIA_BROWSER_MEDIAPACKET_MANAGER_H_
+
+#include "content/public/browser/browser_message_filter.h"
+#include "ui/gfx/gpu_memory_buffer.h"
+
+namespace content {
+
+class CONTENT_EXPORT BrowserMediaPacketManager : public BrowserMessageFilter {
+ public:
+  BrowserMediaPacketManager();
+  bool OnMessageReceived(const IPC::Message& message) override;
+
+ protected:
+  friend class base::RefCountedThreadSafe<BrowserMediaPacketManager>;
+  ~BrowserMediaPacketManager() override;
+
+ private:
+  void ReleaseMediaPacket(gfx::TbmBufferHandle packet);
+
+  DISALLOW_COPY_AND_ASSIGN(BrowserMediaPacketManager);
+};
+
+}  // namespace content
+
+#endif  // CONTENT_BROWSER_MEDIA_BROWSER_MEDIAPACKET_MANAGER_H_
index 4f7209d..e5ea306 100644 (file)
@@ -42,15 +42,14 @@ BrowserMediaPlayerManagerEfl::BrowserMediaPlayerManagerEfl(
     RenderFrameHost* render_frame_host)
     : current_player_(-1),
       render_frame_host_(render_frame_host),
-      web_contents_(WebContents::FromRenderFrameHost(render_frame_host)),
-      weak_ptr_factory_(this) {
+      web_contents_(WebContents::FromRenderFrameHost(render_frame_host)) {
 }
 
 BrowserMediaPlayerManagerEfl::~BrowserMediaPlayerManagerEfl() {
-  for (auto& player : players_) {
-    player->Destroy();
-  }
-  players_.weak_clear();
+  // Players can be destroyed during destruction of BMPM without clear(),
+  // but not to make destruction order dependencies with other member variables
+  // we explicitly destroy player instances here.
+  players_.clear();
 }
 
 media::MediaPlayerEfl* BrowserMediaPlayerManagerEfl::GetPlayer(
@@ -129,15 +128,6 @@ void BrowserMediaPlayerManagerEfl::OnNewTbmBufferAvailable(
   Send(new MediaPlayerEflMsg_NewTbmBufferAvailable(
       GetRoutingID(), player_id, tbm_handle, timestamp));
 }
-
-void BrowserMediaPlayerManagerEfl::OnTbmBufferRelease(
-    int player_id, gfx::TbmBufferHandle handle) {
-  DLOG(INFO) << "Release media_packet:" << handle.media_packet;
-  media::MediaPlayerEfl* player = GetPlayer(player_id);
-  if (player)
-    player->DestroyMediaPacket(
-        static_cast<media_packet_h>(handle.media_packet));
-}
 #endif
 
 void BrowserMediaPlayerManagerEfl::OnTimeChanged(int player_id) {
@@ -328,8 +318,7 @@ void BrowserMediaPlayerManagerEfl::RemovePlayer(int player_id) {
   for (auto it = players_.begin(); it != players_.end(); ++it) {
     media::MediaPlayerEfl* player = *it;
     if (player->GetPlayerId() == player_id) {
-      players_.weak_erase(it);
-      player->Destroy();
+      players_.erase(it);
 
       if (!init_queue_.empty()) {
         if(player_id == init_queue_.front()) {
index a6f2086..401e145 100644 (file)
@@ -55,8 +55,6 @@ class CONTENT_EXPORT BrowserMediaPlayerManagerEfl
   void OnNewTbmBufferAvailable(
       int player_id, gfx::TbmBufferHandle tbm_handle,
       base::TimeDelta timestamp) override;
-  void OnTbmBufferRelease(
-      int player_id, gfx::TbmBufferHandle tbm_handle);
 #endif
 
   // Helper function to handle IPC from RenderMediaPlayerMangaerEfl.
@@ -94,14 +92,6 @@ class CONTENT_EXPORT BrowserMediaPlayerManagerEfl
   WebContents* web_contents() const { return web_contents_; }
 
  private:
-  int current_player_;
-
-  // An array of managed players.
-  ScopedVector<media::MediaPlayerEfl> players_;
-  RenderFrameHost* render_frame_host_;
-  WebContents* const web_contents_;
-  base::WeakPtrFactory<BrowserMediaPlayerManagerEfl> weak_ptr_factory_;
-
   void SuspendPlayers();
   void PausePlayers(int player_id);
   void InitNextPlayer();
@@ -109,6 +99,13 @@ class CONTENT_EXPORT BrowserMediaPlayerManagerEfl
   void RemovePlayerFromResumedQueue(int player_id);
   void CleanUpResumedQueue();
 
+  int current_player_;
+
+  // An array of managed players.
+  ScopedVector<media::MediaPlayerEfl> players_;
+  RenderFrameHost* render_frame_host_;
+  WebContents* const web_contents_;
+
   // A queue to initialize player including preroll.
   std::deque<int> init_queue_;
   std::deque<int> resumed_queue_;
index d7c1b5b..025e131 100644 (file)
@@ -57,11 +57,6 @@ bool MediaWebContentsObserver::OnMessageReceived(
     IPC_MESSAGE_FORWARD(MediaPlayerEflHostMsg_Seek,
                         GetMediaPlayerManager(render_frame_host),
                         BrowserMediaPlayerManagerEfl::OnSeek)
-#if defined(TIZEN_TBM_SUPPORT)
-    IPC_MESSAGE_FORWARD(MediaPlayerEflHostMsg_ReleaseTbmBuffer,
-                        GetMediaPlayerManager(render_frame_host),
-                        BrowserMediaPlayerManagerEfl::OnTbmBufferRelease)
-#endif
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   return handled;
index 535c0df..c0785ac 100644 (file)
@@ -153,9 +153,8 @@ IPC_MESSAGE_ROUTED3(MediaPlayerEflMsg_NewTbmBufferAvailable,
                     base::TimeDelta /* time stamp */)
 
 // Requests to release tbm buffer.
-IPC_MESSAGE_ROUTED2(MediaPlayerEflHostMsg_ReleaseTbmBuffer,
-                    int /* player_id */,
-                    gfx::TbmBufferHandle /* Handle */)
+IPC_MESSAGE_CONTROL1(MediaPlayerEflHostMsg_ReleaseTbmBuffer,
+                     gfx::TbmBufferHandle /* Handle */)
 #endif
 
 // Seek.
index b2fd2c2..3bf85d9 100644 (file)
         'browser/media/efl/browser_media_player_manager_efl.h',
       ],
     }], # tizen_multimedia_support==1
+    ['tizen_tbm_support==1', {
+      'sources': [
+        'browser/media/browser_mediapacket_manager.cc',
+        'browser/media/browser_mediapacket_manager.h',
+      ],
+    }], # tizen_tbm_support==1
     ['enable_web_speech==0', {
       'sources/': [
         ['exclude', '^browser/speech/'],
index 1fd011f..a183af3 100644 (file)
@@ -7,6 +7,7 @@
 #include "base/bind.h"
 #include "base/message_loop/message_loop.h"
 #include "content/common/media/efl/media_player_messages_efl.h"
+#include "content/public/renderer/render_thread.h"
 #include "content/renderer/media/efl/webmediaplayer_efl.h"
 #include "media/base/bind_to_current_loop.h"
 
@@ -182,16 +183,13 @@ void RendererMediaPlayerManager::OnNewTbmBufferAvailable(
         tbm_handle, timestamp,
         media::BindToCurrentLoop(base::Bind(
             &RendererMediaPlayerManager::OnTbmBufferRelease,
-            base::Unretained(this), player_id, tbm_handle)));
+            base::Unretained(this), tbm_handle)));
 }
 
 void RendererMediaPlayerManager::OnTbmBufferRelease(
-    int player_id, gfx::TbmBufferHandle tbm_handle) {
-  DLOG(INFO) << __FUNCTION__
-             << " player_id: " << player_id
-             << ", media_packet: " << tbm_handle.media_packet;
-  Send(new MediaPlayerEflHostMsg_ReleaseTbmBuffer(routing_id(), player_id,
-                                                  tbm_handle));
+    gfx::TbmBufferHandle tbm_handle) {
+  RenderThread::Get()->Send(
+      new MediaPlayerEflHostMsg_ReleaseTbmBuffer(tbm_handle));
 }
 #endif
 
index 940aa97..ddbcb86 100644 (file)
@@ -72,7 +72,7 @@ class RendererMediaPlayerManager : public RenderFrameObserver {
 #if defined(TIZEN_TBM_SUPPORT)
   void OnNewTbmBufferAvailable(int player_id, gfx::TbmBufferHandle tbm_handle,
                                base::TimeDelta timestamp);
-  void OnTbmBufferRelease(int player_id, gfx::TbmBufferHandle tbm_handle);
+  void OnTbmBufferRelease(gfx::TbmBufferHandle tbm_handle);
 #endif
 
   WebMediaPlayerEfl* GetMediaPlayer(int player_id);
index c655be5..217421a 100644 (file)
@@ -4,52 +4,31 @@
 
 #include "media/base/efl/media_player_efl.h"
 
-#include "content/public/browser/browser_thread.h"
-#include "media/base/efl/media_player_manager_efl.h"
-
-#if defined(TIZEN_TBM_SUPPORT)
 #include <media_packet.h>
 
-namespace {
-
-void DoDestroyMediaPacket(media_packet_h packet) {
-  if (MEDIA_PACKET_ERROR_NONE != media_packet_destroy(packet))
-    LOG(WARNING) << "Fail to release media_packet";
-}
-
-}  // namespace
-#endif
+#include "content/public/browser/browser_thread.h"
+#include "media/base/efl/media_player_manager_efl.h"
 
 namespace media {
 
-MediaPlayerEfl::MediaPlayerEfl(int player_id, MediaPlayerManager* manager)
-    : destructing_(false), width_(0), height_(0), player_id_(player_id),
-      suspended_(false), manager_(manager) {}
+MediaPlayerEfl::MediaPlayerEfl(int player_id,
+                               MediaPlayerManager* manager)
+    : width_(0), height_(0), player_id_(player_id), suspended_(false),
+      manager_(manager) {}
 
 #if defined(TIZEN_TBM_SUPPORT)
-void MediaPlayerEfl::DestroyMediaPacket(media_packet_h packet) {
-  // TODO(max): 40 ms delay is a workaround which should be removed.
-  // Currently sometimes GLImage accesses |tbm_surface| after releasing it.
-  content::BrowserThread::PostDelayedTask(
-      content::BrowserThread::IO, FROM_HERE,
-      base::Bind(&DoDestroyMediaPacket, packet),
-      base::TimeDelta::FromMilliseconds(40));
-}
-
-void MediaPlayerEfl::DeliverMediaPacket(media_packet_h packet) {
+void MediaPlayerEfl::DeliverMediaPacket(ScopedMediaPacket packet) {
   tbm_surface_info_s suf_info = {0, };
   tbm_surface_h tbm_surface = nullptr;
 
   if (MEDIA_PACKET_ERROR_NONE !=
-      media_packet_get_tbm_surface(packet, &tbm_surface)) {
+      media_packet_get_tbm_surface(packet.get(), &tbm_surface)) {
     LOG(ERROR) << "|media_packet_get_tbm_surface| failed";
-    DestroyMediaPacket(packet);
     return;
   }
 
   if (TBM_SURFACE_ERROR_NONE != tbm_surface_get_info(tbm_surface, &suf_info)) {
     LOG(ERROR) << "|tbm_surface_get_info| failed";
-    DestroyMediaPacket(packet);
     return;
   }
 
@@ -59,7 +38,7 @@ void MediaPlayerEfl::DeliverMediaPacket(media_packet_h packet) {
     height_ = static_cast<int>(suf_info.height);
 
     // Format will always convert to I420
-    manager_->OnMediaDataChange(
+    manager()->OnMediaDataChange(
         GetPlayerId(), TBM_FORMAT_YUV420, height_, width_, MEDIA_VIDEO_MASK);
   }
 
@@ -69,7 +48,7 @@ void MediaPlayerEfl::DeliverMediaPacket(media_packet_h packet) {
       base::TimeDelta::FromSecondsD(GetCurrentTime());
 
 #if defined(TIZEN_MULTIMEDIA_ZEROCOPY_SUPPORT)
-  gfx::TbmBufferHandle tbm_handle = { tbm_surface, packet };
+  gfx::TbmBufferHandle tbm_handle = { tbm_surface, packet.release() };
   manager()->OnNewTbmBufferAvailable(GetPlayerId(), tbm_handle, timestamp);
 #else
   base::SharedMemory shared_memory;
@@ -117,8 +96,8 @@ void MediaPlayerEfl::DeliverMediaPacket(media_packet_h packet) {
       return;
     }
   }
-  manager()->OnNewFrameAvailable(GetPlayerId(), foreign_memory_handle,
-                                 shared_memory_size, timestamp);
+  manager()->OnNewFrameAvailable(
+      GetPlayerId(), foreign_memory_handle, shared_memory_size, timestamp);
 #endif
 }
 #endif
index 618d81a..0c92926 100644 (file)
@@ -7,11 +7,9 @@
 
 #include <string>
 
-#include "base/basictypes.h"
 #include "base/callback.h"
 #include "base/time/time.h"
-#include "media/base/media_export.h"
-#include "url/gurl.h"
+#include "media/base/efl/media_player_util_efl.h"
 
 typedef struct media_packet_s* media_packet_h;
 
@@ -22,7 +20,6 @@ class BrowserDemuxerEfl;
 namespace media {
 
 class MediaPlayerManager;
-struct MediaPacketProxy;
 
 // Error types for MediaErrorCB.
 enum MediaErrorType {
@@ -75,20 +72,12 @@ class MEDIA_EXPORT MediaPlayerEfl {
 
   int GetPlayerId() { return player_id_; }
 
-  bool IsPlayerDestructing() { return destructing_; }
   bool IsPlayerSuspended() { return suspended_; }
 
   virtual void Initialize() {}
   virtual void Resume() { suspended_ = false; }
   virtual void Suspend() { suspended_ = true; }
 
-  // Destroy this object when all messages for it are delivered
-  virtual void Destroy() = 0;
-
-#if defined(TIZEN_TBM_SUPPORT)
-  void DestroyMediaPacket(media_packet_h packet);
-#endif
-
  protected:
   explicit MediaPlayerEfl(int player_id, MediaPlayerManager* manager);
 
@@ -97,10 +86,9 @@ class MEDIA_EXPORT MediaPlayerEfl {
   MediaPlayerManager* manager() { return manager_; }
 
 #if defined(TIZEN_TBM_SUPPORT)
-  void DeliverMediaPacket(media_packet_h packet);
+  void DeliverMediaPacket(ScopedMediaPacket packet);
 #endif
 
-  bool destructing_;
   int width_;
   int height_;
 
index 457ac2e..b49c2bb 100644 (file)
@@ -4,6 +4,8 @@
 
 #include "media/base/efl/media_player_util_efl.h"
 
+#include <player.h>
+
 #include "base/logging.h"
 #include "base/time/time.h"
 
 
 namespace media {
 
+void MediaPacketDeleter::operator()(media_packet_s* ptr) const {
+  if (ptr != nullptr)
+    media_packet_destroy(ptr);
+}
+
 double ConvertNanoSecondsToSeconds(int64 time) {
   return base::TimeDelta::FromMicroseconds(
       time /
index 1e4a54e..93620b1 100644 (file)
@@ -6,11 +6,20 @@
 #define MEDIA_BASE_EFL_MEDIA_PLAYER_UTIL_EFL_H_
 
 #include "base/basictypes.h"
+#include "base/memory/scoped_ptr.h"
 #include "media/base/media_export.h"
 #include "url/gurl.h"
 
+struct media_packet_s;
+
 namespace media {
 
+struct MediaPacketDeleter {
+  void operator()(media_packet_s* ptr) const;
+};
+
+typedef scoped_ptr<media_packet_s, MediaPacketDeleter> ScopedMediaPacket;
+
 typedef enum {
   MEDIA_SEEK_NONE,  // No seek
   MEDIA_SEEK_DEMUXER,  // Demuxer seeking
index c27aaa4..e0439cd 100644 (file)
@@ -128,21 +128,17 @@ static void InterruptCb(player_interrupted_code_e code, void* data) {
 namespace media {
 
 //static
-MediaPlayerEfl* MediaPlayerEfl::CreatePlayer(int player_id, const GURL& url,
-                                             double volume,
-                                             MediaPlayerManager* manager,
-                                             const std::string& user_agent) {
+MediaPlayerEfl* MediaPlayerEfl::CreatePlayer(
+    int player_id, const GURL& url, double volume,
+    MediaPlayerManager* manager, const std::string& ua) {
   LOG(INFO) << "MediaElement is using |CAPI| to play media";
-  return new MediaPlayerBridgeCapi(player_id, url, volume, manager, user_agent);
+  return new MediaPlayerBridgeCapi(player_id, url, volume, manager, ua);
 }
 
 MediaPlayerBridgeCapi::MediaPlayerBridgeCapi(
-    int player_id,
-    const GURL& url,
-    double volume,
-    MediaPlayerManager* manager_in,
-    const std::string& user_agent)
-    : MediaPlayerEfl(player_id, manager_in),
+    int player_id, const GURL& url, double volume,
+    MediaPlayerManager* manager, const std::string& user_agent)
+    : MediaPlayerEfl(player_id, manager),
       task_runner_(base::ThreadTaskRunnerHandle::Get()),
       player_(NULL),
       url_(url),
@@ -170,19 +166,10 @@ MediaPlayerBridgeCapi::MediaPlayerBridgeCapi(
 }
 
 MediaPlayerBridgeCapi::~MediaPlayerBridgeCapi() {
-}
-
-void MediaPlayerBridgeCapi::Destroy() {
-  if (IsPlayerDestructing())
-    return;
-  destructing_ = true;
   weak_factory_.InvalidateWeakPtrs();
 
   Release();
   player_destroy(player_);
-  player_ = NULL;
-
-  task_runner_->DeleteSoon(FROM_HERE, this);
 }
 
 void MediaPlayerBridgeCapi::Prepare(CompleteCB cb) {
@@ -586,7 +573,7 @@ void MediaPlayerBridgeCapi::UpdateSeekState(bool state) {
 }
 
 void MediaPlayerBridgeCapi::PlayerPrepared() {
-  if (IsPlayerDestructing() || GetPlayerState() < PLAYER_STATE_READY) {
+  if (GetPlayerState() < PLAYER_STATE_READY) {
     return;
   }
 
@@ -715,9 +702,11 @@ void MediaPlayerBridgeCapi::ExecuteDelayedPlayerState() {
 }
 
 void MediaPlayerBridgeCapi::OnMediaPacketUpdated(media_packet_h packet) {
+  ScopedMediaPacket packet_proxy(packet);
   task_runner_->PostTask(
-      FROM_HERE, base::Bind(&MediaPlayerBridgeCapi::DeliverMediaPacket,
-                            weak_factory_.GetWeakPtr(), packet));
+      FROM_HERE, base::Bind(
+          &MediaPlayerBridgeCapi::DeliverMediaPacket,
+          weak_factory_.GetWeakPtr(), base::Passed(&packet_proxy)));
 }
 
 void MediaPlayerBridgeCapi::OnPlaybackCompleteUpdate() {
@@ -759,4 +748,5 @@ void MediaPlayerBridgeCapi::OnInitComplete(bool success) {
   is_initialized_ = true;
   manager()->OnInitComplete(GetPlayerId(), success);
 }
+
 }  // namespace media
index 3f8f7a8..fe81e4d 100644 (file)
@@ -32,7 +32,6 @@ class MEDIA_EXPORT MediaPlayerBridgeCapi
   void Initialize() override;
   void Resume() override;
   void Suspend() override;
-  void Destroy() override;
   void Play() override;
   void Pause(bool is_media_related_action) override;
   void SetRate(double rate) override;
index aa31eee..2ef2ac0 100644 (file)
@@ -24,6 +24,7 @@ const unsigned int min_threshold = 30;
 }  // namespace
 
 namespace media {
+
 struct ErrorList {
   player_error_e error_code;
   std::string error_message;
@@ -58,22 +59,18 @@ const int ERROR_MAX = sizeof(errorlist) / sizeof(errorlist[0]);
 void OnCapiPlayerPreparedCB(void* user_data) {
   MediaSourcePlayerCapi* player =
       static_cast<MediaSourcePlayerCapi*>(user_data);
-  if (player)
-    player->OnPrepareComplete();
+  player->OnPrepareComplete();
 }
 
 void OnSeekCompleteCB(void* user_data) {
   MediaSourcePlayerCapi* player =
       static_cast<MediaSourcePlayerCapi*>(user_data);
-  if (player)
-    player->OnSeekComplete();
+  player->OnSeekComplete();
 }
 
 void OnHandleBufferingMessageCB(int percent, void* user_data) {
   MediaSourcePlayerCapi* player =
       static_cast<MediaSourcePlayerCapi*>(user_data);
-  if (!player || player->IsPlayerDestructing())
-    return;
   player->OnHandleBufferingMessage(percent);
 }
 
@@ -81,8 +78,6 @@ void OnCapiAudioBufStatusCB(player_media_stream_buffer_status_e status,
                             void* user_data) {
   MediaSourcePlayerCapi* player =
       static_cast<MediaSourcePlayerCapi*>(user_data);
-  if (!player || player->IsPlayerDestructing())
-    return;
   if (status == PLAYER_MEDIA_STREAM_BUFFER_UNDERRUN) {
     player->OnUpdateDataStatus(media::DemuxerStream::AUDIO, true);
   } else if (status == PLAYER_MEDIA_STREAM_BUFFER_OVERFLOW) {
@@ -94,8 +89,6 @@ void OnCapiVideoBufStatusCB(player_media_stream_buffer_status_e status,
                             void* user_data) {
   MediaSourcePlayerCapi* player =
       static_cast<MediaSourcePlayerCapi*>(user_data);
-  if (!player || player->IsPlayerDestructing())
-    return;
   if (status == PLAYER_MEDIA_STREAM_BUFFER_UNDERRUN) {
     player->OnUpdateDataStatus(media::DemuxerStream::VIDEO, true);
   } else if (status == PLAYER_MEDIA_STREAM_BUFFER_OVERFLOW) {
@@ -138,8 +131,8 @@ void MediaSourcePlayerCapi::PrepareComplete() {
   player_prepared_ = true;
   manager()->OnReadyStateChange(
       GetPlayerId(), blink::WebMediaPlayer::ReadyStateHaveEnoughData);
-  manager()->OnNetworkStateChange(GetPlayerId(),
-                                  blink::WebMediaPlayer::NetworkStateLoaded);
+  manager()->OnNetworkStateChange(
+      GetPlayerId(), blink::WebMediaPlayer::NetworkStateLoaded);
 }
 
 MediaPlayerEfl* MediaPlayerEfl::CreatePlayer(
@@ -152,7 +145,8 @@ MediaPlayerEfl* MediaPlayerEfl::CreatePlayer(
 }
 
 MediaSourcePlayerCapi::MediaSourcePlayerCapi(
-    int player_id, scoped_ptr<DemuxerEfl> demuxer, MediaPlayerManager* manager)
+    int player_id, scoped_ptr<DemuxerEfl> demuxer,
+    MediaPlayerManager* manager)
     : MediaPlayerEfl(player_id, manager),
       demuxer_(demuxer.Pass()),
       task_runner_(base::ThreadTaskRunnerHandle::Get()),
@@ -224,6 +218,11 @@ MediaSourcePlayerCapi::MediaSourcePlayerCapi(
   }
 }
 
+MediaSourcePlayerCapi::~MediaSourcePlayerCapi() {
+  Release();
+  player_destroy(player_);
+}
+
 void MediaSourcePlayerCapi::SeekComplete() {
   DCHECK(task_runner_->BelongsToCurrentThread());
   UpdateSeekState(MEDIA_SEEK_NONE);
@@ -348,9 +347,11 @@ void MediaSourcePlayerCapi::OnDemuxerSeekDone(
 }
 
 void MediaSourcePlayerCapi::OnMediaPacketUpdated(media_packet_h packet) {
+  ScopedMediaPacket packet_proxy(packet);
   task_runner_->PostTask(
-      FROM_HERE, base::Bind(&MediaSourcePlayerCapi::DeliverMediaPacket,
-                            weak_factory_.GetWeakPtr(), packet));
+      FROM_HERE, base::Bind(
+          &MediaSourcePlayerCapi::DeliverMediaPacket,
+          weak_factory_.GetWeakPtr(), base::Passed(&packet_proxy)));
 }
 
 void MediaSourcePlayerCapi::SetVolume(double volume) {
@@ -445,17 +446,6 @@ void MediaSourcePlayerCapi::PlaybackComplete() {
 #endif
 }
 
-void MediaSourcePlayerCapi::Destroy() {
-  if (IsPlayerDestructing())
-    return;
-
-  destructing_ = true;
-  Release();
-  player_destroy(player_);
-  player_ = NULL;
-  task_runner_->DeleteSoon(FROM_HERE, this);
-}
-
 // TODO(sam) : It's worked as bypass now. Need Suspend/Resume/Initialize
 // implementation for MSE multiple instance.
 void MediaSourcePlayerCapi::Initialize() {
@@ -463,7 +453,6 @@ void MediaSourcePlayerCapi::Initialize() {
 }
 
 void MediaSourcePlayerCapi::Release() {
-  DCHECK(IsPlayerDestructing());
   playing_ = false;
   StopCurrentTimeUpdateTimer();
   audio_buffer_queue_.clear();
@@ -587,8 +576,8 @@ void MediaSourcePlayerCapi::OnDemuxerConfigsAvailable(
   if (width_ > 0 && height_ > 0)
     media_type_ |= MEDIA_VIDEO_MASK;
 
-  manager()->OnMediaDataChange(GetPlayerId(), TBM_FORMAT_YUV420, height_,
-                               width_, media_type_);
+  manager()->OnMediaDataChange(
+      GetPlayerId(), TBM_FORMAT_YUV420, height_, width_, media_type_);
   manager()->OnReadyStateChange(
       GetPlayerId(), blink::WebMediaPlayer::ReadyStateHaveMetadata);
 
@@ -600,23 +589,25 @@ void MediaSourcePlayerCapi::OnDemuxerConfigsAvailable(
     }
   }
 
-  // FIXME(Venu): Need to check where is the exact place for this code.
+  // TODO(Venu): Need to check where is the exact place for this code.
   // For make it work added this below code
+  // TODO(max): We have a contradiction here.
+  // To Complete preparing the capi-player, we have to get the first frame
+  // But to get the first frame we have to set readyState to EnoughData,
+  // even we are not ready for play(Setting the Enoughdata make HTML
+  // think that player is ready for play)
+  // We have to make some hack to get/set the first frame without notifying
+  // to HTML.
   manager()->OnReadyStateChange(
       GetPlayerId(), blink::WebMediaPlayer::ReadyStateHaveEnoughData);
 }
 
 void MediaSourcePlayerCapi::ReadDemuxedData(
     media::DemuxerStream::Type type) {
-  if (IsPlayerDestructing())
-    return;
   demuxer_->RequestDemuxerData(type);
 }
 
 void MediaSourcePlayerCapi::HandleBufferingMessage(int percent) {
-  if (IsPlayerDestructing())
-    return;
-
   // FIXME: add code if required
   return;
 }
@@ -671,11 +662,6 @@ void MediaSourcePlayerCapi::OnDemuxerDataAvailable(
     return;
   }
 
-  if (IsPlayerDestructing()) {
-    LOG(WARNING) << "player deinitializing. Just return";
-    return;
-  }
-
   if (meta_data.status != media::DemuxerStream::kOk ||
       meta_data.end_of_stream)
     BufferMetaDataAvailable(meta_data);
@@ -1002,11 +988,6 @@ void MediaSourcePlayerCapi::OnHandleBufferingMessage(int percent) {
 
 void MediaSourcePlayerCapi::OnReadDemuxedData(
     media::DemuxerStream::Type type) {
-  if (IsPlayerDestructing()) {
-    LOG(ERROR) << "CAPI player deinitializing. Just return";
-    return;
-  }
-
   task_runner_->PostTask(
       FROM_HERE, base::Bind(&MediaSourcePlayerCapi::ReadDemuxedData,
                             weak_factory_.GetWeakPtr(), type));
@@ -1014,11 +995,6 @@ void MediaSourcePlayerCapi::OnReadDemuxedData(
 
 void MediaSourcePlayerCapi::OnUpdateDataStatus(
     media::DemuxerStream::Type type, bool underflow_status) {
-  if (IsPlayerDestructing()) {
-    LOG(ERROR) << "CAPI player deinitializing. Just return";
-    return;
-  }
-
   task_runner_->PostTask(
       FROM_HERE, base::Bind(&MediaSourcePlayerCapi::HandleUnderFlowStatus,
                             weak_factory_.GetWeakPtr(),
index 6a183f9..19a5892 100644 (file)
@@ -31,7 +31,7 @@ class MEDIA_EXPORT MediaSourcePlayerCapi
       int player_id, scoped_ptr<DemuxerEfl> demuxer,
       MediaPlayerManager* manager);
 
-  ~MediaSourcePlayerCapi() override {};
+  ~MediaSourcePlayerCapi() override;
 
   // MediaPlayerEfl implementation.
   void Play() override;
@@ -40,7 +40,6 @@ class MEDIA_EXPORT MediaSourcePlayerCapi
   void Seek(const double time) override;
   void SetVolume(double volume) override;
   double GetCurrentTime() override;
-  void Destroy() override;
   void Initialize() override;
 
   // DemuxerEflClient implementation.