From 2a71ea16bcee6cbba6c8111d4e0e9d4f07f1037c Mon Sep 17 00:00:00 2001 From: "msu.koo" Date: Tue, 15 Dec 2015 13:27:29 +0900 Subject: [PATCH] fixup! [MM] Fix flickering when playing video. Previous patch relied mostly on CleanUp() for releasing used tbm_surface. But it has issues like - It causes that many number of tbm_surfaces are pended for releasing, because compositor sometimes does not signal to release. This causes locking on capi-player because capi-player has constraints on output tbm_surface counts. - CleanUp() is cost operation. This fixup introduces Closure to bound the lifecycle of tbm_surface with VideoFrame by following the Chromium's VideoFrame approach. (So no more hack is required on GLImage for releasing tbm_surface.) When Chromium invalidates the VideoFrame, tbm_surface will also be released. For this approach, this fixup introduces IPC message for releasing tbm_surface, so any process/thread can request to release tbm_surface. (On previous approach, Browser-Process can only request releasing tbm_surface). By taking this approach, it's guaranteed that every tbm_handles are requested to be released, so CleanUp is removed. Bug: http://165.213.149.170/jira/browse/TSAM-676 Bug: http://165.213.149.170/jira/browse/TSAM-662 Bug: http://107.108.218.239/bugzilla/show_bug.cgi?id=15255 Bug: http://suprem.sec.samsung.net/jira/browse/CBEFL-835 Reviewed by: msu.koo, sm.venugopal, sns.park Change-Id: I9c622396402ec34e0aa77e28bff7bf3e937f83eb Signed-off-by: msu.koo --- .../media/efl/browser_media_player_manager_efl.cc | 10 +++++++ .../media/efl/browser_media_player_manager_efl.h | 2 ++ .../media/media_web_contents_observer_efl.cc | 5 ++++ .../common/media/efl/media_player_messages_efl.h | 5 ++++ .../media/efl/renderer_media_player_manager_efl.cc | 17 +++++++++++- .../media/efl/renderer_media_player_manager_efl.h | 1 + .../renderer/media/efl/webmediaplayer_efl.cc | 7 ++--- .../renderer/media/efl/webmediaplayer_efl.h | 6 +++-- .../media/base/tizen/media_packet_manager.cc | 31 ---------------------- .../media/base/tizen/media_packet_manager.h | 3 --- 10 files changed, 47 insertions(+), 40 deletions(-) diff --git a/tizen_src/chromium_impl/content/browser/media/efl/browser_media_player_manager_efl.cc b/tizen_src/chromium_impl/content/browser/media/efl/browser_media_player_manager_efl.cc index b808c82..4141530 100644 --- a/tizen_src/chromium_impl/content/browser/media/efl/browser_media_player_manager_efl.cc +++ b/tizen_src/chromium_impl/content/browser/media/efl/browser_media_player_manager_efl.cc @@ -14,6 +14,10 @@ #include "ipc/ipc_logging.h" #include "media/base/efl/media_player_efl.h" +#if defined(TIZEN_TBM_SUPPORT) +#include "media/base/tizen/media_packet_manager.h" +#endif + namespace content { #if defined(OS_TIZEN_TV) @@ -124,6 +128,12 @@ void BrowserMediaPlayerManagerEfl::OnNewTbmBufferAvailable( Send(new MediaPlayerEflMsg_NewTbmBufferAvailable( GetRoutingID(), player_id, tbm_handle, timestamp)); } + +void BrowserMediaPlayerManagerEfl::OnTbmBufferRelease( + int player_id, gfx::TbmBufferHandle tbm_handle) { + media::MediaPacketManager::GetInstance()->Release( + static_cast(tbm_handle.tbm_surface)); +} #endif void BrowserMediaPlayerManagerEfl::OnTimeChanged(int player_id) { diff --git a/tizen_src/chromium_impl/content/browser/media/efl/browser_media_player_manager_efl.h b/tizen_src/chromium_impl/content/browser/media/efl/browser_media_player_manager_efl.h index 430982b..a6f2086 100644 --- a/tizen_src/chromium_impl/content/browser/media/efl/browser_media_player_manager_efl.h +++ b/tizen_src/chromium_impl/content/browser/media/efl/browser_media_player_manager_efl.h @@ -55,6 +55,8 @@ 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. diff --git a/tizen_src/chromium_impl/content/browser/media/media_web_contents_observer_efl.cc b/tizen_src/chromium_impl/content/browser/media/media_web_contents_observer_efl.cc index 025e131..d7c1b5b 100644 --- a/tizen_src/chromium_impl/content/browser/media/media_web_contents_observer_efl.cc +++ b/tizen_src/chromium_impl/content/browser/media/media_web_contents_observer_efl.cc @@ -57,6 +57,11 @@ 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; diff --git a/tizen_src/chromium_impl/content/common/media/efl/media_player_messages_efl.h b/tizen_src/chromium_impl/content/common/media/efl/media_player_messages_efl.h index c750bb2..4371cce 100644 --- a/tizen_src/chromium_impl/content/common/media/efl/media_player_messages_efl.h +++ b/tizen_src/chromium_impl/content/common/media/efl/media_player_messages_efl.h @@ -155,6 +155,11 @@ IPC_MESSAGE_ROUTED3(MediaPlayerEflMsg_NewTbmBufferAvailable, int /* player_id */, gfx::TbmBufferHandle /* Handle */, base::TimeDelta /* time stamp */) + +// Requests to release tbm buffer. +IPC_MESSAGE_ROUTED2(MediaPlayerEflHostMsg_ReleaseTbmBuffer, + int /* player_id */, + gfx::TbmBufferHandle /* Handle */) #endif // Seek. diff --git a/tizen_src/chromium_impl/content/renderer/media/efl/renderer_media_player_manager_efl.cc b/tizen_src/chromium_impl/content/renderer/media/efl/renderer_media_player_manager_efl.cc index 9871a46..b71c6fe 100644 --- a/tizen_src/chromium_impl/content/renderer/media/efl/renderer_media_player_manager_efl.cc +++ b/tizen_src/chromium_impl/content/renderer/media/efl/renderer_media_player_manager_efl.cc @@ -8,6 +8,7 @@ #include "base/message_loop/message_loop.h" #include "content/common/media/efl/media_player_messages_efl.h" #include "content/renderer/media/efl/webmediaplayer_efl.h" +#include "media/base/bind_to_current_loop.h" namespace content { @@ -175,8 +176,22 @@ void RendererMediaPlayerManager::OnNewTbmBufferAvailable( int player_id, gfx::TbmBufferHandle tbm_handle, base::TimeDelta timestamp) { WebMediaPlayerEfl* player = GetMediaPlayer(player_id); + if (player) - player->OnNewTbmBufferAvailable(tbm_handle, timestamp); + player->OnNewTbmBufferAvailable( + tbm_handle, timestamp, + media::BindToCurrentLoop(base::Bind( + &RendererMediaPlayerManager::OnTbmBufferRelease, + base::Unretained(this), player_id, tbm_handle))); +} + +void RendererMediaPlayerManager::OnTbmBufferRelease( + int player_id, gfx::TbmBufferHandle tbm_handle) { + DLOG(INFO) << __FUNCTION__ + << " player_id: " << player_id + << ", tbm_handle: " << tbm_handle.tbm_surface; + Send(new MediaPlayerEflHostMsg_ReleaseTbmBuffer(routing_id(), player_id, + tbm_handle)); } #endif diff --git a/tizen_src/chromium_impl/content/renderer/media/efl/renderer_media_player_manager_efl.h b/tizen_src/chromium_impl/content/renderer/media/efl/renderer_media_player_manager_efl.h index 8275cc9..940aa97 100644 --- a/tizen_src/chromium_impl/content/renderer/media/efl/renderer_media_player_manager_efl.h +++ b/tizen_src/chromium_impl/content/renderer/media/efl/renderer_media_player_manager_efl.h @@ -72,6 +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); #endif WebMediaPlayerEfl* GetMediaPlayer(int player_id); diff --git a/tizen_src/chromium_impl/content/renderer/media/efl/webmediaplayer_efl.cc b/tizen_src/chromium_impl/content/renderer/media/efl/webmediaplayer_efl.cc index 1c004c7..11ab77f 100644 --- a/tizen_src/chromium_impl/content/renderer/media/efl/webmediaplayer_efl.cc +++ b/tizen_src/chromium_impl/content/renderer/media/efl/webmediaplayer_efl.cc @@ -447,12 +447,13 @@ void WebMediaPlayerEfl::SetNetworkState(WebMediaPlayer::NetworkState state) { } #if defined(TIZEN_TBM_SUPPORT) -void WebMediaPlayerEfl::OnNewTbmBufferAvailable(gfx::TbmBufferHandle tbm_handle, - base::TimeDelta timestamp) { +void WebMediaPlayerEfl::OnNewTbmBufferAvailable( + const gfx::TbmBufferHandle& tbm_handle, base::TimeDelta timestamp, + const base::Closure& cb) { gfx::Size size(gst_width_, gst_height_); scoped_refptr video_frame = VideoFrame::WrapTBMSurface(size, timestamp, tbm_handle); - + video_frame->AddDestructionObserver(cb); FrameReady(video_frame); } #endif diff --git a/tizen_src/chromium_impl/content/renderer/media/efl/webmediaplayer_efl.h b/tizen_src/chromium_impl/content/renderer/media/efl/webmediaplayer_efl.h index a909883..ace9352 100644 --- a/tizen_src/chromium_impl/content/renderer/media/efl/webmediaplayer_efl.h +++ b/tizen_src/chromium_impl/content/renderer/media/efl/webmediaplayer_efl.h @@ -146,8 +146,10 @@ class WebMediaPlayerEfl uint32 length, base::TimeDelta timestamp); #if defined(TIZEN_TBM_SUPPORT) - void OnNewTbmBufferAvailable(gfx::TbmBufferHandle tbm_handle, - base::TimeDelta timestamp); + void OnNewTbmBufferAvailable( + const gfx::TbmBufferHandle& tbm_handle, + base::TimeDelta timestamp, + const base::Closure& cb); #endif void OnMediaDataChange(int format, int height, int width, int media); 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 index 5b5880a..bae95d4 100644 --- a/tizen_src/chromium_impl/media/base/tizen/media_packet_manager.cc +++ b/tizen_src/chromium_impl/media/base/tizen/media_packet_manager.cc @@ -7,28 +7,14 @@ #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() { @@ -55,23 +41,6 @@ void MediaPacketManager::Release(tbm_surface_h 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 media_packet) { 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 index 390eff1..4a2f8d5 100644 --- a/tizen_src/chromium_impl/media/base/tizen/media_packet_manager.h +++ b/tizen_src/chromium_impl/media/base/tizen/media_packet_manager.h @@ -10,7 +10,6 @@ #include #include "base/memory/scoped_ptr.h" -#include "base/time/time.h" #include "media/base/media_export.h" namespace base { @@ -22,7 +21,6 @@ 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(); @@ -38,7 +36,6 @@ class MEDIA_EXPORT MediaPacketManager { private: friend struct base::DefaultSingletonTraits; MediaPacketManager(); - void CleanUp(); void DoPush(scoped_ptr media_packet); void DoRelease(tbm_surface_h handle); -- 2.7.4