diff options
26 files changed, 296 insertions, 385 deletions
diff --git a/content/common/gpu/gpu_messages.h b/content/common/gpu/gpu_messages.h index 8f7c926..888da59 100644 --- a/content/common/gpu/gpu_messages.h +++ b/content/common/gpu/gpu_messages.h @@ -496,8 +496,8 @@ IPC_MESSAGE_ROUTED1(AcceleratedVideoDecoderMsg_Reset, gpu::ReadWriteTokens) /* tokens */ // Send destroy request to the decoder. -IPC_MESSAGE_ROUTED1(AcceleratedVideoDecoderMsg_Destroy, - gpu::ReadWriteTokens) /* tokens */ +IPC_SYNC_MESSAGE_ROUTED1_0(AcceleratedVideoDecoderMsg_Destroy, + gpu::ReadWriteTokens) /* tokens */ //------------------------------------------------------------------------------ // Accelerated Video Decoder Host Messages @@ -536,9 +536,6 @@ IPC_MESSAGE_ROUTED0(AcceleratedVideoDecoderHostMsg_FlushDone) // Confirm decoder has been reset. IPC_MESSAGE_ROUTED0(AcceleratedVideoDecoderHostMsg_ResetDone) -// Confirm decoder has been destroyed. -IPC_MESSAGE_ROUTED0(AcceleratedVideoDecoderHostMsg_DestroyDone) - // Decoder has faced end of stream marker in the stream. IPC_MESSAGE_ROUTED0(AcceleratedVideoDecoderHostMsg_EndOfStream) diff --git a/content/common/gpu/media/gpu_video_decode_accelerator.cc b/content/common/gpu/media/gpu_video_decode_accelerator.cc index 82fd408..6d9b17f 100644 --- a/content/common/gpu/media/gpu_video_decode_accelerator.cc +++ b/content/common/gpu/media/gpu_video_decode_accelerator.cc @@ -163,7 +163,7 @@ void GpuVideoDecodeAccelerator::Initialize(const std::vector<uint32>& configs) { omx_decoder->SetEglState( gfx::GLSurfaceEGL::GetDisplay(), stub_->scheduler()->decoder()->GetGLContext()->GetHandle()); - video_decode_accelerator_.reset(omx_decoder); + video_decode_accelerator_ = omx_decoder; video_decode_accelerator_->Initialize(configs); #else NOTIMPLEMENTED() << "HW video decode acceleration not available."; @@ -250,11 +250,6 @@ void GpuVideoDecodeAccelerator::NotifyResetDone() { LOG(ERROR) << "Send(AcceleratedVideoDecoderHostMsg_ResetDone) failed"; } -void GpuVideoDecodeAccelerator::NotifyDestroyDone() { - if (!Send(new AcceleratedVideoDecoderHostMsg_DestroyDone(host_route_id_))) - LOG(ERROR) << "Send(AcceleratedVideoDecoderHostMsg_DestroyDone) failed"; -} - bool GpuVideoDecodeAccelerator::Send(IPC::Message* message) { DCHECK(sender_); return sender_->Send(message); diff --git a/content/common/gpu/media/gpu_video_decode_accelerator.h b/content/common/gpu/media/gpu_video_decode_accelerator.h index 5d0302d..621e47c 100644 --- a/content/common/gpu/media/gpu_video_decode_accelerator.h +++ b/content/common/gpu/media/gpu_video_decode_accelerator.h @@ -47,7 +47,6 @@ class GpuVideoDecodeAccelerator virtual void NotifyEndOfBitstreamBuffer(int32 bitstream_buffer_id) OVERRIDE; virtual void NotifyFlushDone() OVERRIDE; virtual void NotifyResetDone() OVERRIDE; - virtual void NotifyDestroyDone() OVERRIDE; // Function to delegate sending to actual sender. virtual bool Send(IPC::Message* message); @@ -93,7 +92,7 @@ class GpuVideoDecodeAccelerator GpuCommandBufferStub* stub_; // Pointer to the underlying VideoDecodeAccelerator. - scoped_ptr<media::VideoDecodeAccelerator> video_decode_accelerator_; + scoped_refptr<media::VideoDecodeAccelerator> video_decode_accelerator_; DISALLOW_IMPLICIT_CONSTRUCTORS(GpuVideoDecodeAccelerator); }; diff --git a/content/common/gpu/media/omx_video_decode_accelerator.cc b/content/common/gpu/media/omx_video_decode_accelerator.cc index 28614f2..5d05668 100644 --- a/content/common/gpu/media/omx_video_decode_accelerator.cc +++ b/content/common/gpu/media/omx_video_decode_accelerator.cc @@ -57,10 +57,10 @@ static bool AreOMXFunctionPointersInitialized() { } while (0) // OMX-specific version of RETURN_ON_FAILURE which compares with OMX_ErrorNone. -#define RETURN_ON_OMX_FAILURE(omx_result, log, error, ret_val) \ - RETURN_ON_FAILURE( \ - ((omx_result) == OMX_ErrorNone), \ - log << ", OMX result: " << std::hex << std::showbase << omx_result, \ +#define RETURN_ON_OMX_FAILURE(omx_result, log, error, ret_val) \ + RETURN_ON_FAILURE( \ + ((omx_result) == OMX_ErrorNone), \ + log << ", OMX result: 0x" << std::hex << omx_result, \ error, ret_val) OmxVideoDecodeAccelerator::OmxVideoDecodeAccelerator( @@ -182,6 +182,7 @@ bool OmxVideoDecodeAccelerator::CreateComponent() { VIDEODECODERERROR_UNSUPPORTED, false); // Get the handle to the component. + AddRef(); // To reflect passing |this| to OMX_GetHandle below. result = omx_gethandle( &component_handle_, reinterpret_cast<OMX_STRING>(component.get()), this, &omx_accelerator_callbacks); @@ -265,7 +266,7 @@ bool OmxVideoDecodeAccelerator::CreateComponent() { OMX_BUFFERHEADERTYPE* buffer; result = OMX_UseBuffer(component_handle_, &buffer, output_port_, NULL, 0, reinterpret_cast<OMX_U8*>(0x1)); - RETURN_ON_OMX_FAILURE(result, "OMX_UseBuffer failed with: " << result, + RETURN_ON_OMX_FAILURE(result, "OMX_UseBuffer failed", VIDEODECODERERROR_INVALIDINPUT, false); buffer->pAppPrivate = NULL; buffer->nTimeStamp = -1; @@ -284,7 +285,7 @@ void OmxVideoDecodeAccelerator::Decode( RETURN_ON_FAILURE(current_state_change_ == NO_TRANSITION && (client_state_ == OMX_StateIdle || client_state_ == OMX_StateExecuting), - "Call to Decode() during invalid state or transition:" + "Call to Decode() during invalid state or transition: " << current_state_change_ << ", " << client_state_, VIDEODECODERERROR_UNSUPPORTED,); @@ -315,8 +316,7 @@ void OmxVideoDecodeAccelerator::Decode( // Give this buffer to OMX. OMX_ERRORTYPE result = OMX_EmptyThisBuffer(component_handle_, omx_buffer); - RETURN_ON_OMX_FAILURE(result, - "OMX_EmptyThisBuffer() failed with result " << result, + RETURN_ON_OMX_FAILURE(result, "OMX_EmptyThisBuffer() failed", VIDEODECODERERROR_INVALIDINPUT,); input_buffers_at_component_++; @@ -365,8 +365,7 @@ void OmxVideoDecodeAccelerator::ReusePictureBuffer(int32 picture_buffer_id) { ++output_buffers_at_component_; OMX_ERRORTYPE result = OMX_FillThisBuffer(component_handle_, output_picture.omx_buffer_header); - RETURN_ON_OMX_FAILURE(result, - "OMX_FillThisBuffer() failed with result " << result, + RETURN_ON_OMX_FAILURE(result, "OMX_FillThisBuffer() failed", VIDEODECODERERROR_INVALIDINPUT,); } @@ -384,8 +383,7 @@ void OmxVideoDecodeAccelerator::Flush() { omx_buffer->nFlags |= OMX_BUFFERFLAG_EOS; omx_buffer->nTimeStamp = -2; OMX_ERRORTYPE result = OMX_EmptyThisBuffer(component_handle_, omx_buffer); - RETURN_ON_OMX_FAILURE(result, - "OMX_EmptyThisBuffer() failed with result " << result, + RETURN_ON_OMX_FAILURE(result, "OMX_EmptyThisBuffer() failed", VIDEODECODERERROR_INVALIDINPUT,); input_buffers_at_component_++; } @@ -426,16 +424,33 @@ void OmxVideoDecodeAccelerator::Reset() { void OmxVideoDecodeAccelerator::Destroy() { DCHECK_EQ(message_loop_, MessageLoop::current()); + if (current_state_change_ == ERRORING) + return; DCHECK_EQ(current_state_change_, NO_TRANSITION); + // If we were never initializeed there's no teardown to do. + if (client_state_ == OMX_StateMax) + return; + // If we can already call OMX_FreeHandle, simply do so. + if (client_state_ == OMX_StateInvalid || client_state_ == OMX_StateLoaded) { + ShutdownComponent(); + return; + } DCHECK_EQ(client_state_, OMX_StateExecuting); current_state_change_ = DESTROYING; + client_ = NULL; BeginTransitionToState(OMX_StateIdle); + BusyLoopInDestroying(); } void OmxVideoDecodeAccelerator::BeginTransitionToState( OMX_STATETYPE new_state) { DCHECK_EQ(message_loop_, MessageLoop::current()); DCHECK_NE(current_state_change_, NO_TRANSITION); + DCHECK_NE(current_state_change_, ERRORING); + if (current_state_change_ == NO_TRANSITION || + current_state_change_ == ERRORING) { + return; + } OMX_ERRORTYPE result = OMX_SendCommand( component_handle_, OMX_CommandStateSet, new_state, 0); RETURN_ON_OMX_FAILURE(result, "SendCommand(OMX_CommandStateSet) failed", @@ -461,13 +476,13 @@ void OmxVideoDecodeAccelerator::OnReachedExecutingInInitializing() { it != fake_output_buffers_.end(); ++it) { OMX_BUFFERHEADERTYPE* buffer = *it; OMX_ERRORTYPE result = OMX_FillThisBuffer(component_handle_, buffer); - RETURN_ON_OMX_FAILURE(result, - "OMX_FillThisBuffer() failed with: " << result, + RETURN_ON_OMX_FAILURE(result, "OMX_FillThisBuffer()", VIDEODECODERERROR_INVALIDINPUT,); ++output_buffers_at_component_; } - client_->NotifyInitializeDone(); + if (client_) + client_->NotifyInitializeDone(); } void OmxVideoDecodeAccelerator::OnReachedPauseInFlushing() { @@ -482,7 +497,8 @@ void OmxVideoDecodeAccelerator::OnReachedExecutingInFlushing() { DCHECK(saw_eos_during_flush_); saw_eos_during_flush_ = false; current_state_change_ = NO_TRANSITION; - client_->NotifyFlushDone(); + if (client_) + client_->NotifyFlushDone(); } void OmxVideoDecodeAccelerator::OnReachedPauseInResetting() { @@ -495,7 +511,25 @@ void OmxVideoDecodeAccelerator::OnReachedExecutingInResetting() { DCHECK_EQ(client_state_, OMX_StatePause); client_state_ = OMX_StateExecuting; current_state_change_ = NO_TRANSITION; - client_->NotifyResetDone(); + if (client_) + client_->NotifyResetDone(); +} + +// Alert: HORROR ahead! OMX shutdown is an asynchronous dance but our clients +// enjoy the fire-and-forget nature of a synchronous Destroy() call that +// ensures no further callbacks are made. Since the interface between OMX +// callbacks and this class is a MessageLoop, we need to ensure the loop +// outlives the shutdown dance, even during process shutdown. We do this by +// repeatedly enqueuing a no-op task until shutdown is complete, since +// MessageLoop's shutdown drains pending tasks. +void OmxVideoDecodeAccelerator::BusyLoopInDestroying() { + if (!component_handle_) return; + // Can't use PostDelayedTask here because MessageLoop doesn't drain delayed + // tasks. Instead we sleep for 5ms. Really. + base::PlatformThread::Sleep(5); + message_loop_->PostTask( + FROM_HERE, NewRunnableMethod( + this, &OmxVideoDecodeAccelerator::BusyLoopInDestroying)); } void OmxVideoDecodeAccelerator::OnReachedIdleInDestroying() { @@ -514,15 +548,8 @@ void OmxVideoDecodeAccelerator::OnReachedIdleInDestroying() { FreeInputBuffers(); if (!output_buffers_at_component_) FreeOutputBuffers(); -} -void OmxVideoDecodeAccelerator::ShutdownComponent() { - OMX_ERRORTYPE result = omx_free_handle(component_handle_); - if (result != OMX_ErrorNone) - LOG(ERROR) << "OMX_FreeHandle() error. Error code: " << result; - component_handle_ = NULL; - omx_deinit(); - client_state_ = OMX_StateMax; + BusyLoopInDestroying(); } void OmxVideoDecodeAccelerator::OnReachedLoadedInDestroying() { @@ -530,7 +557,6 @@ void OmxVideoDecodeAccelerator::OnReachedLoadedInDestroying() { client_state_ = OMX_StateLoaded; current_state_change_ = NO_TRANSITION; ShutdownComponent(); - client_->NotifyDestroyDone(); } void OmxVideoDecodeAccelerator::OnReachedInvalidInErroring() { @@ -538,17 +564,31 @@ void OmxVideoDecodeAccelerator::OnReachedInvalidInErroring() { ShutdownComponent(); } +void OmxVideoDecodeAccelerator::ShutdownComponent() { + OMX_ERRORTYPE result = omx_free_handle(component_handle_); + if (result != OMX_ErrorNone) + LOG(ERROR) << "OMX_FreeHandle() error. Error code: " << result; + component_handle_ = NULL; + client_state_ = OMX_StateMax; + // This Release() call must happen *after* any access to |*this| because it + // might result in |this| being deleted. + Release(); // Since OMX no longer has |this| to call back to. + omx_deinit(); +} + void OmxVideoDecodeAccelerator::StopOnError( media::VideoDecodeAccelerator::Error error) { DCHECK_EQ(message_loop_, MessageLoop::current()); - current_state_change_ = ERRORING; - client_->NotifyError(error); + if (client_) + client_->NotifyError(error); + client_ = NULL; if (client_state_ == OMX_StateInvalid || client_state_ == OMX_StateMax) return; BeginTransitionToState(OMX_StateInvalid); + current_state_change_ = ERRORING; } bool OmxVideoDecodeAccelerator::AllocateInputBuffers() { @@ -589,7 +629,7 @@ bool OmxVideoDecodeAccelerator::AllocateOutputBuffers() { OMX_ERRORTYPE result = OMX_UseEGLImage( component_handle_, omx_buffer, output_port_, &gles_buffer, it->second.egl_image); - RETURN_ON_OMX_FAILURE(result, "OMX_UseEGLImage failed with: " << result, + RETURN_ON_OMX_FAILURE(result, "OMX_UseEGLImage", VIDEODECODERERROR_MEMFAILURE, false); // Here we set a garbage bitstream buffer id, and then overwrite it before // passing to PictureReady. @@ -610,10 +650,9 @@ void OmxVideoDecodeAccelerator::FreeInputBuffers() { omx_buffer = free_input_buffers_.front(); free_input_buffers_.pop(); result = OMX_FreeBuffer(component_handle_, input_port_, omx_buffer); - RETURN_ON_OMX_FAILURE(result, "OMX_FreeBuffer failed with: " << result, + RETURN_ON_OMX_FAILURE(result, "OMX_FreeBuffer", VIDEODECODERERROR_INVALIDINPUT,); } - VLOG(1) << "Input buffers freed."; } void OmxVideoDecodeAccelerator::FreeOutputBuffers() { @@ -627,11 +666,12 @@ void OmxVideoDecodeAccelerator::FreeOutputBuffers() { CHECK(omx_buffer); delete reinterpret_cast<media::Picture*>(omx_buffer->pAppPrivate); result = OMX_FreeBuffer(component_handle_, output_port_, omx_buffer); - RETURN_ON_OMX_FAILURE(result, "OMX_FreeBuffer failed with: " << result, + RETURN_ON_OMX_FAILURE(result, "OMX_FreeBuffer", VIDEODECODERERROR_INVALIDINPUT,); texture2eglImage_translator.DestroyEglImage(egl_display_, it->second.egl_image); - client_->DismissPictureBuffer(it->first); + if (client_) + client_->DismissPictureBuffer(it->first); } pictures_.clear(); } @@ -643,7 +683,7 @@ void OmxVideoDecodeAccelerator::OnOutputPortDisabled() { port_format.nPortIndex = output_port_; OMX_ERRORTYPE result = OMX_GetParameter( component_handle_, OMX_IndexParamPortDefinition, &port_format); - RETURN_ON_OMX_FAILURE(result, "OMX_GetParameter failed with: " << result, + RETURN_ON_OMX_FAILURE(result, "OMX_GetParameter", VIDEODECODERERROR_UNSUPPORTED,); DCHECK_EQ(port_format.nBufferCountMin, kNumPictureBuffers); @@ -655,10 +695,12 @@ void OmxVideoDecodeAccelerator::OnOutputPortDisabled() { // ProvidePictureBuffers() will trigger AssignGLESBuffers, which ultimately // assigns the textures to the component and re-enables the port. const OMX_VIDEO_PORTDEFINITIONTYPE& vformat = port_format.format.video; - client_->ProvidePictureBuffers( - kNumPictureBuffers, - gfx::Size(vformat.nFrameWidth, vformat.nFrameHeight), - PICTUREBUFFER_MEMORYTYPE_GL_TEXTURE); + if (client_) { + client_->ProvidePictureBuffers( + kNumPictureBuffers, + gfx::Size(vformat.nFrameWidth, vformat.nFrameHeight), + PICTUREBUFFER_MEMORYTYPE_GL_TEXTURE); + } } void OmxVideoDecodeAccelerator::OnOutputPortEnabled() { @@ -679,8 +721,7 @@ void OmxVideoDecodeAccelerator::OnOutputPortEnabled() { omx_buffer->nOutputPortIndex = output_port_; ++output_buffers_at_component_; OMX_ERRORTYPE result = OMX_FillThisBuffer(component_handle_, omx_buffer); - RETURN_ON_OMX_FAILURE(result, - "OMX_FillThisBuffer() failed with result " << result, + RETURN_ON_OMX_FAILURE(result, "OMX_FillThisBuffer() failed", VIDEODECODERERROR_INSUFFICIENT_BUFFERS,); } } @@ -695,7 +736,7 @@ void OmxVideoDecodeAccelerator::FillBufferDoneTask( CHECK_EQ(fake_output_buffers_.erase(buffer), 1U); OMX_ERRORTYPE result = OMX_FreeBuffer(component_handle_, output_port_, buffer); - RETURN_ON_OMX_FAILURE(result, "OMX_FreeBuffer failed with: " << result, + RETURN_ON_OMX_FAILURE(result, "OMX_FreeBuffer failed", VIDEODECODERERROR_INVALIDINPUT,); return; } @@ -721,7 +762,8 @@ void OmxVideoDecodeAccelerator::FillBufferDoneTask( reinterpret_cast<media::Picture*>(buffer->pAppPrivate); // See Decode() for an explanation of this abuse of nTimeStamp. picture->set_bitstream_buffer_id(buffer->nTimeStamp); - client_->PictureReady(*picture); + if (client_) + client_->PictureReady(*picture); } void OmxVideoDecodeAccelerator::EmptyBufferDoneTask( @@ -739,7 +781,8 @@ void OmxVideoDecodeAccelerator::EmptyBufferDoneTask( reinterpret_cast<SharedMemoryAndId*>(buffer->pAppPrivate); DCHECK(input_buffer_details); buffer->pAppPrivate = NULL; - client_->NotifyEndOfBitstreamBuffer(input_buffer_details->second); + if (client_) + client_->NotifyEndOfBitstreamBuffer(input_buffer_details->second); delete input_buffer_details; } @@ -790,6 +833,14 @@ void OmxVideoDecodeAccelerator::DispatchStateReached(OMX_STATETYPE reached) { default: NOTREACHED() << "Unexpected state in DESTROYING: " << reached; } + case ERRORING: + switch (reached) { + case OMX_StateInvalid: + OnReachedInvalidInErroring(); + return; + default: + NOTREACHED() << "Unexpected state in ERRORING: " << reached; + } default: NOTREACHED() << "Unexpected state in " << current_state_change_ << ": " << reached; @@ -801,23 +852,19 @@ void OmxVideoDecodeAccelerator::EventHandlerCompleteTask(OMX_EVENTTYPE event, OMX_U32 data2) { DCHECK_EQ(message_loop_, MessageLoop::current()); switch (event) { - case OMX_EventCmdComplete: { - // If the last command was successful, we have completed - // a state transition. So notify that we have done it - // accordingly. - OMX_COMMANDTYPE cmd = static_cast<OMX_COMMANDTYPE>(data1); - switch (cmd) { + case OMX_EventCmdComplete: + switch (data1) { case OMX_CommandPortDisable: DCHECK_EQ(data2, output_port_); OnOutputPortDisabled(); - break; + return; case OMX_CommandPortEnable: DCHECK_EQ(data2, output_port_); OnOutputPortEnabled(); - break; + return; case OMX_CommandStateSet: DispatchStateReached(static_cast<OMX_STATETYPE>(data2)); - break; + return; case OMX_CommandFlush: DCHECK(current_state_change_ == FLUSHING || current_state_change_ == RESETTING || @@ -828,16 +875,19 @@ void OmxVideoDecodeAccelerator::EventHandlerCompleteTask(OMX_EVENTTYPE event, OutputPortFlushDone(); else NOTREACHED() << "Unexpected port flushed: " << data2; - break; + return; default: RETURN_ON_FAILURE(false, "Unknown command completed: " << data1, VIDEODECODERERROR_HARDWARE,); } - break; - } + return; case OMX_EventError: - RETURN_ON_FAILURE(false, "EventError: " << data1, - VIDEODECODERERROR_HARDWARE,); + if (current_state_change_ != DESTROYING && + current_state_change_ != ERRORING) { + RETURN_ON_FAILURE(false, "EventError: 0x" << std::hex << data1, + VIDEODECODERERROR_HARDWARE,); + } + return; case OMX_EventPortSettingsChanged: if (data2 == OMX_IndexParamPortDefinition) { DCHECK_EQ(data1, output_port_); @@ -853,7 +903,7 @@ void OmxVideoDecodeAccelerator::EventHandlerCompleteTask(OMX_EVENTTYPE event, << data1 << ", " << data2, VIDEODECODERERROR_HARDWARE,); } - break; + return; case OMX_EventBufferFlag: if (data1 == output_port_) { DCHECK_EQ(current_state_change_, FLUSHING); @@ -864,7 +914,7 @@ void OmxVideoDecodeAccelerator::EventHandlerCompleteTask(OMX_EVENTTYPE event, << data1 << ", " << data2, VIDEODECODERERROR_HARDWARE,); } - break; + return; default: RETURN_ON_FAILURE(false, "Unexpected unhandled event: " << event, VIDEODECODERERROR_HARDWARE,); @@ -883,10 +933,9 @@ OMX_ERRORTYPE OmxVideoDecodeAccelerator::EventHandler(OMX_HANDLETYPE component, static_cast<OmxVideoDecodeAccelerator*>(priv_data); DCHECK_EQ(component, decoder->component_handle_); decoder->message_loop_->PostTask( - FROM_HERE, - NewRunnableMethod(decoder, - &OmxVideoDecodeAccelerator::EventHandlerCompleteTask, - event, data1, data2)); + FROM_HERE, NewRunnableMethod( + decoder, &OmxVideoDecodeAccelerator::EventHandlerCompleteTask, + event, data1, data2)); return OMX_ErrorNone; } @@ -926,6 +975,10 @@ OMX_ERRORTYPE OmxVideoDecodeAccelerator::FillBufferCallback( bool OmxVideoDecodeAccelerator::CanFillBuffer() { DCHECK_EQ(message_loop_, MessageLoop::current()); + if (current_state_change_ == DESTROYING || + current_state_change_ == ERRORING) { + return false; + } return client_state_ == OMX_StateIdle || client_state_ == OMX_StateExecuting || client_state_ == OMX_StatePause; @@ -936,9 +989,7 @@ bool OmxVideoDecodeAccelerator::SendCommandToPort( DCHECK_EQ(message_loop_, MessageLoop::current()); OMX_ERRORTYPE result = OMX_SendCommand(component_handle_, cmd, port_index, 0); - RETURN_ON_OMX_FAILURE(result, "SendCommand() failed" << cmd << ":" << result, + RETURN_ON_OMX_FAILURE(result, "SendCommand() failed" << cmd, VIDEODECODERERROR_INVALIDINPUT, false); return true; } - -DISABLE_RUNNABLE_METHOD_REFCOUNT(OmxVideoDecodeAccelerator); diff --git a/content/common/gpu/media/omx_video_decode_accelerator.h b/content/common/gpu/media/omx_video_decode_accelerator.h index 4c246d0..edced28 100644 --- a/content/common/gpu/media/omx_video_decode_accelerator.h +++ b/content/common/gpu/media/omx_video_decode_accelerator.h @@ -15,7 +15,9 @@ #include "base/bind.h" #include "base/callback.h" +#include "base/compiler_specific.h" #include "base/logging.h" +#include "base/memory/ref_counted.h" #include "base/message_loop.h" #include "base/shared_memory.h" #include "media/video/video_decode_accelerator.h" @@ -31,12 +33,13 @@ // // This class lives on a single thread and DCHECKs that it is never accessed // from any other. OMX callbacks are trampolined from the OMX component's -// thread to maintain this invariant. +// thread to maintain this invariant. The only exception to thread-unsafety is +// that references can be added from any thread (practically used only by the +// OMX thread). class OmxVideoDecodeAccelerator : public media::VideoDecodeAccelerator { public: // Does not take ownership of |client| which must outlive |*this|. OmxVideoDecodeAccelerator(media::VideoDecodeAccelerator::Client* client); - virtual ~OmxVideoDecodeAccelerator(); // media::VideoDecodeAccelerator implementation. bool Initialize(const std::vector<uint32>& config) OVERRIDE; @@ -51,6 +54,8 @@ class OmxVideoDecodeAccelerator : public media::VideoDecodeAccelerator { void SetEglState(EGLDisplay egl_display, EGLContext egl_context); private: + virtual ~OmxVideoDecodeAccelerator(); + // Because OMX state-transitions are described solely by the "state reached" // (3.1.2.9.1, table 3-7 of the spec), we track what transition was requested // using this enum. Note that it is an error to request a transition while @@ -84,7 +89,6 @@ class OmxVideoDecodeAccelerator : public media::VideoDecodeAccelerator { // Create the Component for OMX. Handles all OMX initialization. bool CreateComponent(); - void ShutdownComponent(); // Buffer allocation/free methods for input and output buffers. bool AllocateInputBuffers(); bool AllocateOutputBuffers(); @@ -110,6 +114,8 @@ class OmxVideoDecodeAccelerator : public media::VideoDecodeAccelerator { void OnReachedLoadedInDestroying(); void OnReachedEOSInFlushing(); void OnReachedInvalidInErroring(); + void ShutdownComponent(); + void BusyLoopInDestroying(); // Port-flushing helpers. void FlushIOPorts(); diff --git a/content/common/gpu/media/omx_video_decode_accelerator_unittest.cc b/content/common/gpu/media/omx_video_decode_accelerator_unittest.cc index 6a6d35c..b08d0ae2 100644 --- a/content/common/gpu/media/omx_video_decode_accelerator_unittest.cc +++ b/content/common/gpu/media/omx_video_decode_accelerator_unittest.cc @@ -200,9 +200,9 @@ void RenderingHelper::Initialize( // Per-window/surface X11 & EGL initialization. for (int i = 0; i < num_windows; ++i) { - // Arrange X windows side by side whimsically. - int top_left_x = (width + 50) * i; - int top_left_y = 100 + (i % 2) * 250; + // Arrange X windows whimsically, with some padding. + int top_left_x = (width + 20) * (i % 4); + int top_left_y = (height + 12) * (i % 3); Window x_window = XCreateWindow( x_display_, DefaultRootWindow(x_display_), top_left_x, top_left_y, width_, height_, @@ -421,7 +421,6 @@ class EglRenderingVDAClient : public VideoDecodeAccelerator::Client { virtual void NotifyEndOfBitstreamBuffer(int32 bitstream_buffer_id); virtual void NotifyFlushDone(); virtual void NotifyResetDone(); - virtual void NotifyDestroyDone(); virtual void NotifyError(VideoDecodeAccelerator::Error error); // Simple getters for inspecting the state of the Client. @@ -432,7 +431,7 @@ class EglRenderingVDAClient : public VideoDecodeAccelerator::Client { EGLDisplay egl_display() { return rendering_helper_->egl_display(); } EGLContext egl_context() { return rendering_helper_->egl_context(); } double frames_per_second(); - bool decoder_deleted() { return !decoder_.get(); } + bool decoder_deleted() { return !decoder_; } private: typedef std::map<int, media::GLESBuffer*> PictureBufferById; @@ -456,7 +455,7 @@ class EglRenderingVDAClient : public VideoDecodeAccelerator::Client { size_t encoded_data_next_pos_to_decode_; int next_bitstream_buffer_id_; ClientStateNotification* note_; - scoped_ptr<OmxVideoDecodeAccelerator> decoder_; + scoped_refptr<OmxVideoDecodeAccelerator> decoder_; int delete_decoder_state_; ClientState state_; VideoDecodeAccelerator::Error error_; @@ -485,17 +484,18 @@ EglRenderingVDAClient::EglRenderingVDAClient(RenderingHelper* rendering_helper, } EglRenderingVDAClient::~EglRenderingVDAClient() { - CHECK(!decoder_.get()); + DeleteDecoder(); // Clean up in case of expected error. + CHECK(decoder_deleted()); STLDeleteValues(&picture_buffers_by_id_); SetState(CS_DESTROYED); } void EglRenderingVDAClient::CreateDecoder() { - CHECK(!decoder_.get()); - decoder_.reset(new OmxVideoDecodeAccelerator(this)); + CHECK(decoder_deleted()); + decoder_ = new OmxVideoDecodeAccelerator(this); decoder_->SetEglState(egl_display(), egl_context()); SetState(CS_DECODER_SET); - if (!decoder_.get()) + if (decoder_deleted()) return; // Configure the decoder. @@ -514,7 +514,7 @@ void EglRenderingVDAClient::ProvidePictureBuffers( uint32 requested_num_of_buffers, const gfx::Size& dimensions, VideoDecodeAccelerator::MemoryType type) { - if (!decoder_.get()) + if (decoder_deleted()) return; CHECK_EQ(type, VideoDecodeAccelerator::PICTUREBUFFER_MEMORYTYPE_GL_TEXTURE); std::vector<media::GLESBuffer> buffers; @@ -550,7 +550,7 @@ void EglRenderingVDAClient::PictureReady(const media::Picture& picture) { // We shouldn't be getting pictures delivered after Reset has completed. DCHECK_LT(state_, CS_RESET); - if (!decoder_.get()) + if (decoder_deleted()) return; last_frame_delivered_ticks_ = base::TimeTicks::Now(); @@ -587,25 +587,20 @@ void EglRenderingVDAClient::NotifyEndOfBitstreamBuffer( } void EglRenderingVDAClient::NotifyFlushDone() { - if (!decoder_.get()) + if (decoder_deleted()) return; SetState(CS_FLUSHED); - if (!decoder_.get()) + if (decoder_deleted()) return; decoder_->Reset(); } void EglRenderingVDAClient::NotifyResetDone() { - if (!decoder_.get()) + if (decoder_deleted()) return; SetState(CS_RESET); - if (!decoder_.get()) - return; - decoder_->Destroy(); -} - -void EglRenderingVDAClient::NotifyDestroyDone() { - SetState(CS_DESTROYED); + if (!decoder_deleted()) + DeleteDecoder(); } void EglRenderingVDAClient::NotifyError(VideoDecodeAccelerator::Error error) { @@ -622,12 +617,17 @@ static bool LookingAtNAL(const std::string& encoded, size_t pos) { void EglRenderingVDAClient::SetState(ClientState new_state) { note_->Notify(new_state); state_ = new_state; - if (new_state == delete_decoder_state_) + if (new_state == delete_decoder_state_) { + CHECK(!decoder_deleted()); DeleteDecoder(); + } } void EglRenderingVDAClient::DeleteDecoder() { - decoder_.reset(); + if (decoder_deleted()) + return; + decoder_->Destroy(); + decoder_ = NULL; // Cascade through the rest of the states to simplify test code below. for (int i = state_ + 1; i < CS_MAX; ++i) SetState(static_cast<ClientState>(i)); @@ -651,7 +651,7 @@ void EglRenderingVDAClient::GetRangeForNextNALUs( } void EglRenderingVDAClient::DecodeNextNALUs() { - if (!decoder_.get()) + if (decoder_deleted()) return; if (encoded_data_next_pos_to_decode_ == encoded_data_->size()) { decoder_->Flush(); @@ -680,7 +680,8 @@ void EglRenderingVDAClient::DecodeNextNALUs() { double EglRenderingVDAClient::frames_per_second() { base::TimeDelta delta = last_frame_delivered_ticks_ - initialize_done_ticks_; - CHECK_GT(delta.InSecondsF(), 0); + if (delta.InSecondsF() == 0) + return 0; return num_decoded_frames_ / delta.InSecondsF(); } @@ -705,6 +706,10 @@ static void AssertWaitForStateOrDeleted(ClientStateNotification* note, << ", instead of " << expected_state; } +// We assert the exact number of concurrent decoders we expect to succeed and +// that one more than that fails initialization. +enum { kMaxSupportedNumConcurrentDecoders = 5 }; + // Test the most straightforward case possible: data is decoded from a single // chunk and rendered to the screen. TEST_P(OmxVideoDecodeAcceleratorTest, TestSimpleDecode) { @@ -763,9 +768,19 @@ TEST_P(OmxVideoDecodeAcceleratorTest, TestSimpleDecode) { ASSERT_EQ(note->Wait(), CS_DECODER_SET); } // Then wait for all the decodes to finish. + bool saw_init_failure = false; for (size_t i = 0; i < num_concurrent_decoders; ++i) { ClientStateNotification* note = notes[i]; - ASSERT_EQ(note->Wait(), CS_INITIALIZED); + ClientState state = note->Wait(); + if (state != CS_INITIALIZED) { + saw_init_failure = true; + // We expect initialization to fail only when more than the supported + // number of decoders is instantiated. Assert here that something else + // didn't trigger failure. + ASSERT_GT(num_concurrent_decoders, kMaxSupportedNumConcurrentDecoders); + continue; + } + ASSERT_EQ(state, CS_INITIALIZED); // InitializeDone kicks off decoding inside the client, so we just need to // wait for Flush. ASSERT_NO_FATAL_FAILURE( @@ -777,8 +792,11 @@ TEST_P(OmxVideoDecodeAcceleratorTest, TestSimpleDecode) { ASSERT_NO_FATAL_FAILURE( AssertWaitForStateOrDeleted(note, clients[i], CS_DESTROYED)); } + ASSERT_EQ(saw_init_failure, + num_concurrent_decoders > kMaxSupportedNumConcurrentDecoders) + << num_concurrent_decoders; // Finally assert that decoding went as expected. - for (size_t i = 0; i < num_concurrent_decoders; ++i) { + for (size_t i = 0; i < num_concurrent_decoders && !saw_init_failure; ++i) { // We can only make performance/correctness assertions if the decoder was // allowed to finish. if (delete_decoder_state < CS_FLUSHED) @@ -813,10 +831,8 @@ TEST_P(OmxVideoDecodeAcceleratorTest, TestSimpleDecode) { rendering_thread.Stop(); }; -// TODO(fischman): Remove DISABLED_ prefix when ~OVDA can tear down OMX -// synchronously (http://crosbug.com/p/4869). INSTANTIATE_TEST_CASE_P( - DISABLED_TearDownTiming, OmxVideoDecodeAcceleratorTest, + TearDownTiming, OmxVideoDecodeAcceleratorTest, ::testing::Values( MakeTuple(1, 1, CS_DECODER_SET), MakeTuple(1, 1, CS_INITIALIZED), MakeTuple(1, 1, CS_FLUSHED), MakeTuple(1, 1, CS_RESET), @@ -824,7 +840,7 @@ INSTANTIATE_TEST_CASE_P( MakeTuple(1, 1, static_cast<ClientState>(-10)), MakeTuple(1, 1, static_cast<ClientState>(-100)))); -// TODO(fischman): using 2nd param of 16 and higher below breaks decode - visual +// TODO(fischman): using 1st param of 16 and higher below breaks decode - visual // artifacts are introduced as well as spurious frames are delivered (more // pictures are returned than NALUs are fed to the decoder). Increase the "15" // below when http://code.google.com/p/chrome-os-partner/issues/detail?id=4378 @@ -832,10 +848,19 @@ INSTANTIATE_TEST_CASE_P( INSTANTIATE_TEST_CASE_P( DecodeVariations, OmxVideoDecodeAcceleratorTest, ::testing::Values( - MakeTuple(1, 1, CS_DESTROYED), MakeTuple(1, 3, CS_DESTROYED), - MakeTuple(2, 1, CS_DESTROYED), MakeTuple(3, 1, CS_DESTROYED), - MakeTuple(5, 1, CS_DESTROYED), MakeTuple(8, 1, CS_DESTROYED), - MakeTuple(15, 1, CS_DESTROYED))); + MakeTuple(1, 1, CS_RESET), MakeTuple(1, 3, CS_RESET), + MakeTuple(2, 1, CS_RESET), MakeTuple(3, 1, CS_RESET), + MakeTuple(5, 1, CS_RESET), MakeTuple(8, 1, CS_RESET), + MakeTuple(15, 1, CS_RESET))); + +// Find out how many concurrent decoders can go before we exhaust system +// resources. +INSTANTIATE_TEST_CASE_P( + ResourceExhaustion, OmxVideoDecodeAcceleratorTest, + ::testing::Values( + // +0 hack below to promote enum to int. + MakeTuple(1, kMaxSupportedNumConcurrentDecoders + 0, CS_RESET), + MakeTuple(1, kMaxSupportedNumConcurrentDecoders + 1, CS_RESET))); // TODO(fischman, vrk): add more tests! In particular: // - Test life-cycle: Seek/Stop/Pause/Play/RePlay for a single decoder. diff --git a/content/renderer/gpu/command_buffer_proxy.cc b/content/renderer/gpu/command_buffer_proxy.cc index 42949b6..f9fe8d5 100644 --- a/content/renderer/gpu/command_buffer_proxy.cc +++ b/content/renderer/gpu/command_buffer_proxy.cc @@ -50,7 +50,7 @@ bool CommandBufferProxy::OnMessageReceived(const IPC::Message& message) { IPC_MESSAGE_UNHANDLED(handled = false) IPC_END_MESSAGE_MAP() - if (!handled && video_decoder_host_.get()) + if (!handled && video_decoder_host_) handled = video_decoder_host_->OnMessageReceived(message); DCHECK(handled); @@ -386,19 +386,20 @@ void CommandBufferProxy::SetNotifyRepaintTask(Task* task) { notify_repaint_task_.reset(task); } -GpuVideoDecodeAcceleratorHost* CommandBufferProxy::CreateVideoDecoder( +scoped_refptr<GpuVideoDecodeAcceleratorHost> +CommandBufferProxy::CreateVideoDecoder( const std::vector<uint32>& configs, gpu::CommandBufferHelper* cmd_buffer_helper, media::VideoDecodeAccelerator::Client* client) { - video_decoder_host_.reset(new GpuVideoDecodeAcceleratorHost( - channel_, route_id_, cmd_buffer_helper, client)); + video_decoder_host_ = new GpuVideoDecodeAcceleratorHost( + channel_, route_id_, cmd_buffer_helper, client); if (!Send(new GpuCommandBufferMsg_CreateVideoDecoder(route_id_, configs))) { LOG(ERROR) << "Send(GpuChannelMsg_CreateVideoDecoder) failed"; - video_decoder_host_.reset(); + video_decoder_host_ = NULL; } - return video_decoder_host_.get(); + return video_decoder_host_; } #if defined(OS_MACOSX) diff --git a/content/renderer/gpu/command_buffer_proxy.h b/content/renderer/gpu/command_buffer_proxy.h index 9879169..aba15c9 100644 --- a/content/renderer/gpu/command_buffer_proxy.h +++ b/content/renderer/gpu/command_buffer_proxy.h @@ -83,14 +83,12 @@ class CommandBufferProxy : public gpu::CommandBuffer, void SetNotifyRepaintTask(Task* task); // Sends an IPC message to create a GpuVideoDecodeAccelerator. Creates and - // returns a pointer to a GpuVideoDecodeAcceleratorHost. CommandBufferProxy - // owns the GpuVideoDecodeAcceleratorHost and does not transfer ownership to - // the caller of this method. + // returns a pointer to a GpuVideoDecodeAcceleratorHost. // Returns NULL on failure to create the GpuVideoDecodeAcceleratorHost. // Note that the GpuVideoDecodeAccelerator may still fail to be created in // the GPU process, even if this returns non-NULL. In this case the client is // notified of an error later. - GpuVideoDecodeAcceleratorHost* CreateVideoDecoder( + scoped_refptr<GpuVideoDecodeAcceleratorHost> CreateVideoDecoder( const std::vector<uint32>& configs, gpu::CommandBufferHelper* cmd_buffer_helper, media::VideoDecodeAccelerator::Client* client); @@ -126,7 +124,7 @@ class CommandBufferProxy : public gpu::CommandBuffer, // The video decoder host corresponding to the stub's video decoder in the GPU // process, if one exists. - scoped_ptr<GpuVideoDecodeAcceleratorHost> video_decoder_host_; + scoped_refptr<GpuVideoDecodeAcceleratorHost> video_decoder_host_; // The last cached state received from the service. State last_state_; diff --git a/content/renderer/gpu/gpu_video_decode_accelerator_host.cc b/content/renderer/gpu/gpu_video_decode_accelerator_host.cc index 7c2576b..91d88a1 100644 --- a/content/renderer/gpu/gpu_video_decode_accelerator_host.cc +++ b/content/renderer/gpu/gpu_video_decode_accelerator_host.cc @@ -59,8 +59,6 @@ bool GpuVideoDecodeAcceleratorHost::OnMessageReceived(const IPC::Message& msg) { OnFlushDone) IPC_MESSAGE_HANDLER(AcceleratedVideoDecoderHostMsg_ResetDone, OnResetDone) - IPC_MESSAGE_HANDLER(AcceleratedVideoDecoderHostMsg_DestroyDone, - OnDestroyDone) IPC_MESSAGE_HANDLER(AcceleratedVideoDecoderHostMsg_EndOfStream, OnEndOfStream) IPC_MESSAGE_HANDLER(AcceleratedVideoDecoderHostMsg_ErrorNotification, @@ -137,12 +135,8 @@ void GpuVideoDecodeAcceleratorHost::Flush() { void GpuVideoDecodeAcceleratorHost::Reset() { DCHECK(CalledOnValidThread()); - if (!ipc_sender_->Send(new AcceleratedVideoDecoderMsg_Reset( - command_buffer_route_id_, SyncTokens()))) { - LOG(ERROR) << "Send(AcceleratedVideoDecoderMsg_Reset) failed"; - // TODO(fischman/vrk): signal error to client. - return; - } + Send(new AcceleratedVideoDecoderMsg_Reset( + command_buffer_route_id_, SyncTokens())); } void GpuVideoDecodeAcceleratorHost::Destroy() { @@ -209,11 +203,6 @@ void GpuVideoDecodeAcceleratorHost::OnResetDone() { client_->NotifyResetDone(); } -void GpuVideoDecodeAcceleratorHost::OnDestroyDone() { - DCHECK(CalledOnValidThread()); - client_->NotifyDestroyDone(); -} - void GpuVideoDecodeAcceleratorHost::OnEndOfStream() { DCHECK(CalledOnValidThread()); client_->NotifyEndOfStream(); diff --git a/content/renderer/gpu/gpu_video_decode_accelerator_host.h b/content/renderer/gpu/gpu_video_decode_accelerator_host.h index 9c304ce..98dc30b 100644 --- a/content/renderer/gpu/gpu_video_decode_accelerator_host.h +++ b/content/renderer/gpu/gpu_video_decode_accelerator_host.h @@ -67,7 +67,6 @@ class GpuVideoDecodeAcceleratorHost const gfx::Size& decoded_size); void OnFlushDone(); void OnResetDone(); - void OnDestroyDone(); void OnEndOfStream(); void OnErrorNotification(uint32 error); diff --git a/content/renderer/gpu/gpu_video_service_host.cc b/content/renderer/gpu/gpu_video_service_host.cc deleted file mode 100644 index cb43332..0000000 --- a/content/renderer/gpu/gpu_video_service_host.cc +++ /dev/null @@ -1,76 +0,0 @@ -// Copyright (c) 2011 The Chromium Authors. 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/renderer/gpu/gpu_video_service_host.h" - -#include "content/common/gpu/gpu_messages.h" -#include "content/renderer/gpu/gpu_video_decode_accelerator_host.h" -#include "content/renderer/render_thread.h" -#include "media/video/video_decode_accelerator.h" - -GpuVideoServiceHost::GpuVideoServiceHost() - : channel_(NULL), - next_decoder_host_id_(0) { - DCHECK(RenderThread::current()); - DCHECK_EQ(RenderThread::current()->message_loop(), MessageLoop::current()); -} - -GpuVideoServiceHost::~GpuVideoServiceHost() { -} - -void GpuVideoServiceHost::set_channel(IPC::SyncChannel* channel) { - DCHECK(CalledOnValidThread()); - DCHECK(!channel_); - channel_ = channel; - if (!on_initialized_.is_null()) - on_initialized_.Run(); -} - -bool GpuVideoServiceHost::OnMessageReceived(const IPC::Message& msg) { - DCHECK(CalledOnValidThread()); - if (!channel_) - return false; - switch (msg.type()) { - case AcceleratedVideoDecoderHostMsg_BitstreamBufferProcessed::ID: - case AcceleratedVideoDecoderHostMsg_ProvidePictureBuffers::ID: - case AcceleratedVideoDecoderHostMsg_InitializeDone::ID: - case AcceleratedVideoDecoderHostMsg_DismissPictureBuffer::ID: - case AcceleratedVideoDecoderHostMsg_PictureReady::ID: - case AcceleratedVideoDecoderHostMsg_FlushDone::ID: - case AcceleratedVideoDecoderHostMsg_ResetDone::ID: - case AcceleratedVideoDecoderHostMsg_DestroyDone::ID: - case AcceleratedVideoDecoderHostMsg_EndOfStream::ID: - case AcceleratedVideoDecoderHostMsg_ErrorNotification::ID: - if (router_.RouteMessage(msg)) - return true; - LOG(ERROR) << "AcceleratedVideoDecoderHostMsg cannot be dispatched."; - default: - return false; - } -} - -void GpuVideoServiceHost::OnChannelError() { - DCHECK(CalledOnValidThread()); - channel_ = NULL; -} - -void GpuVideoServiceHost::SetOnInitialized( - const base::Closure& on_initialized) { - DCHECK(CalledOnValidThread()); - DCHECK(on_initialized_.is_null()); - on_initialized_ = on_initialized; - if (channel_) - on_initialized.Run(); -} - -GpuVideoDecodeAcceleratorHost* GpuVideoServiceHost::CreateVideoAccelerator( - media::VideoDecodeAccelerator::Client* client, - int32 command_buffer_route_id, - gpu::CommandBufferHelper* cmd_buffer_helper) { - DCHECK(CalledOnValidThread()); - DCHECK(channel_); - return new GpuVideoDecodeAcceleratorHost( - &router_, channel_, next_decoder_host_id_++, - command_buffer_route_id, cmd_buffer_helper, client); -} diff --git a/content/renderer/pepper_platform_video_decoder_impl.cc b/content/renderer/pepper_platform_video_decoder_impl.cc index 9c8b247..ac34a3b 100644 --- a/content/renderer/pepper_platform_video_decoder_impl.cc +++ b/content/renderer/pepper_platform_video_decoder_impl.cc @@ -20,8 +20,7 @@ PlatformVideoDecoderImpl::PlatformVideoDecoderImpl( gpu::CommandBufferHelper* cmd_buffer_helper) : client_(client), command_buffer_route_id_(command_buffer_route_id), - cmd_buffer_helper_(cmd_buffer_helper), - decoder_(NULL) { + cmd_buffer_helper_(cmd_buffer_helper) { DCHECK(client); } @@ -133,8 +132,3 @@ void PlatformVideoDecoderImpl::NotifyResetDone() { DCHECK_EQ(RenderThread::current()->message_loop(), MessageLoop::current()); client_->NotifyResetDone(); } - -void PlatformVideoDecoderImpl::NotifyDestroyDone() { - DCHECK_EQ(RenderThread::current()->message_loop(), MessageLoop::current()); - client_->NotifyDestroyDone(); -} diff --git a/content/renderer/pepper_platform_video_decoder_impl.h b/content/renderer/pepper_platform_video_decoder_impl.h index b4d0d03..32b0080 100644 --- a/content/renderer/pepper_platform_video_decoder_impl.h +++ b/content/renderer/pepper_platform_video_decoder_impl.h @@ -25,9 +25,8 @@ class PlatformVideoDecoderImpl media::VideoDecodeAccelerator::Client* client, int32 command_buffer_route_id, gpu::CommandBufferHelper* cmd_buffer_helper); - virtual ~PlatformVideoDecoderImpl(); - // PlatformVideoDecoder implementation. + // PlatformVideoDecoder (a.k.a. VideoDecodeAccelerator) implementation. virtual bool Initialize(const std::vector<uint32>& configs) OVERRIDE; virtual void Decode( const media::BitstreamBuffer& bitstream_buffer) OVERRIDE; @@ -52,9 +51,10 @@ class PlatformVideoDecoderImpl virtual void NotifyEndOfBitstreamBuffer(int32 bitstream_buffer_id) OVERRIDE; virtual void NotifyFlushDone() OVERRIDE; virtual void NotifyResetDone() OVERRIDE; - virtual void NotifyDestroyDone() OVERRIDE; private: + virtual ~PlatformVideoDecoderImpl(); + // Client lifetime must exceed lifetime of this class. // TODO(vrk/fischman): We should take another look at the overall // arcitecture of PPAPI Video Decode to make sure lifetime/ownership makes @@ -67,10 +67,8 @@ class PlatformVideoDecoderImpl // Helper for the command buffer associated with video decoder's context. gpu::CommandBufferHelper* cmd_buffer_helper_; - // Host for GpuVideoDecodeAccelerator. - // This is owned by the CommandBufferProxy associated with - // |command_buffer_route_id|. - media::VideoDecodeAccelerator* decoder_; + // Holds a GpuVideoDecodeAcceleratorHost. + scoped_refptr<media::VideoDecodeAccelerator> decoder_; DISALLOW_COPY_AND_ASSIGN(PlatformVideoDecoderImpl); }; diff --git a/media/video/video_decode_accelerator.h b/media/video/video_decode_accelerator.h index 85eef24..55d1ad1 100644 --- a/media/video/video_decode_accelerator.h +++ b/media/video/video_decode_accelerator.h @@ -161,10 +161,12 @@ enum VideoColorFormat { // Video decoder interface. // This interface is extended by the various components that ultimately // implement the backend of PPB_VideoDecode_Dev. -class VideoDecodeAccelerator { +// +// No thread-safety guarantees are implied by the use of RefCountedThreadSafe +// below. +class VideoDecodeAccelerator + : public base::RefCountedThreadSafe<VideoDecodeAccelerator> { public: - virtual ~VideoDecodeAccelerator(); - // Enumeration of potential errors generated by the API. // TODO(fischman): these errors are a bad match for both what OMX generates // and for what the plugin wants. Overhaul this list with a more useful set. @@ -226,9 +228,6 @@ class VideoDecodeAccelerator { // Reset completion callback. virtual void NotifyResetDone() = 0; - // Destroy completion callback. - virtual void NotifyDestroyDone() = 0; - // Callback to notify about decoding errors. virtual void NotifyError(Error error) = 0; }; @@ -279,11 +278,14 @@ class VideoDecodeAccelerator { virtual void Reset() = 0; // Destroys the decoder: all pending inputs are dropped immediately and the - // component is freed, followed by NotifyDestroyDone being called on the - // client. After this is called no other calls may be made on the decoder, - // and after NotifyDestroyDone is called no callbacks will be made by the - // decoder on the client. + // component is freed. This call may asynchornously free system resources, + // but its client-visible effects are synchronous. After this method returns + // no more callbacks will be made on the client. virtual void Destroy() = 0; + + protected: + friend class base::RefCountedThreadSafe<VideoDecodeAccelerator>; + virtual ~VideoDecodeAccelerator(); }; } // namespace media diff --git a/ppapi/c/dev/ppb_video_decoder_dev.h b/ppapi/c/dev/ppb_video_decoder_dev.h index 6564521..5b8f0d4 100644 --- a/ppapi/c/dev/ppb_video_decoder_dev.h +++ b/ppapi/c/dev/ppb_video_decoder_dev.h @@ -9,8 +9,8 @@ #include "ppapi/c/pp_completion_callback.h" #include "ppapi/c/pp_var.h" -#define PPB_VIDEODECODER_DEV_INTERFACE_0_11 "PPB_VideoDecoder(Dev);0.11" -#define PPB_VIDEODECODER_DEV_INTERFACE PPB_VIDEODECODER_DEV_INTERFACE_0_11 +#define PPB_VIDEODECODER_DEV_INTERFACE_0_12 "PPB_VideoDecoder(Dev);0.12" +#define PPB_VIDEODECODER_DEV_INTERFACE PPB_VIDEODECODER_DEV_INTERFACE_0_12 // Video decoder interface. // @@ -27,7 +27,7 @@ // callback. // - To reset the decoder (e.g. to implement Seek): call Reset() and wait for // NotifyResetDone callback. -// - To tear down the decoder call Destroy() and wait for NotifyDestroyDone. +// - To tear down the decoder call Destroy(). // // See PPP_VideoDecoder_Dev for the notifications the decoder may send the // plugin. @@ -132,18 +132,14 @@ struct PPB_VideoDecoder_Dev { struct PP_CompletionCallback callback); // Tear down the decoder as quickly as possible. Pending inputs and outputs - // are dropped and the decoder frees all of its resources. When |callback| is - // called the plugin is guaranteed not to receive any further callbacks and - // must make no further calls on |video_decoder|. + // are dropped and the decoder frees all of its resources. Although resources + // may be freed asynchronously, after this method returns no more callbacks + // will be made on the client. Any resources held by the client at that point + // may be freed. // // Parameters: // |video_decoder| is the previously created handle to the decoder resource. - // |callback| is one-time callback that will be called once the destroy - // request has been completed. - // - // Returns an error code from pp_errors.h. - int32_t (*Destroy)(PP_Resource video_decoder, - struct PP_CompletionCallback callback); + void (*Destroy)(PP_Resource video_decoder); }; #endif /* PPAPI_C_DEV_PPB_VIDEO_DECODER_DEV_H_ */ diff --git a/ppapi/c/dev/ppp_video_decoder_dev.h b/ppapi/c/dev/ppp_video_decoder_dev.h index 1940665..cc9c721 100644 --- a/ppapi/c/dev/ppp_video_decoder_dev.h +++ b/ppapi/c/dev/ppp_video_decoder_dev.h @@ -9,7 +9,7 @@ #include "ppapi/c/pp_resource.h" #include "ppapi/c/dev/pp_video_dev.h" -#define PPP_VIDEODECODER_DEV_INTERFACE "PPP_VideoDecoder(Dev);0.4" +#define PPP_VIDEODECODER_DEV_INTERFACE "PPP_VideoDecoder(Dev);0.5" // PPP_VideoDecoder_Dev structure contains the function pointers that the // plugin MUST implement to provide services needed by the video decoder @@ -25,13 +25,11 @@ struct PPP_VideoDecoder_Dev { // // Parameters: // |instance| the plugin instance to which the callback is responding. - // |decoder| is pointer to the Pepper Video Decoder resource. // |req_num_of_bufs| tells how many buffers are needed by the decoder. // |dimensions| tells the dimensions of the buffer to allocate. // |type| specifies whether the buffer lives in system memory or GL texture. void (*ProvidePictureBuffers)( PP_Instance instance, - PP_Resource decoder, uint32_t req_num_of_bufs, struct PP_Size dimensions, enum PP_PictureBufferType_Dev type); @@ -41,11 +39,8 @@ struct PPP_VideoDecoder_Dev { // // Parameters: // |instance| the plugin instance to which the callback is responding. - // |decoder| is pointer to the Pepper Video Decoder resource. // |picture_buffer| points to the picture buffer that is no longer needed. - void (*DismissPictureBuffer)(PP_Instance instance, - PP_Resource decoder, - int32_t picture_buffer_id); + void (*DismissPictureBuffer)(PP_Instance instance, int32_t picture_buffer_id); // Callback function for decoder to deliver decoded pictures ready to be // displayed. Decoder expects the plugin to return the buffer back to the @@ -53,11 +48,8 @@ struct PPP_VideoDecoder_Dev { // // Parameters: // |instance| the plugin instance to which the callback is responding. - // |decoder| is pointer to the Pepper Video Decoder resource. // |picture| is the picture that is ready. - void (*PictureReady)(PP_Instance instance, - PP_Resource decoder, - struct PP_Picture_Dev picture); + void (*PictureReady)(PP_Instance instance, struct PP_Picture_Dev picture); // Callback function to tell the plugin that decoder has decoded end of stream // marker and output all the pictures that should be displayed from the @@ -65,8 +57,7 @@ struct PPP_VideoDecoder_Dev { // // Parameters: // |instance| the plugin instance to which the callback is responding. - // |decoder| is pointer to the Pepper Video Decoder resource. - void (*EndOfStream)(PP_Instance instance, PP_Resource decoder); + void (*EndOfStream)(PP_Instance instance); // Error handler callback for decoder to deliver information about detected // errors to the plugin. @@ -75,11 +66,8 @@ struct PPP_VideoDecoder_Dev { // // Parameters: // |instance| the plugin instance to which the callback is responding. - // |decoder| is pointer to the Pepper Video Decoder resource. // |error| error is the enumeration specifying the error. - void (*NotifyError)(PP_Instance instance, - PP_Resource decoder, - enum PP_VideoDecodeError_Dev error); + void (*NotifyError)(PP_Instance instance, enum PP_VideoDecodeError_Dev error); }; #endif /* PPAPI_C_DEV_PPP_VIDEO_DECODER_DEV_H_ */ diff --git a/ppapi/cpp/dev/video_decoder_client_dev.cc b/ppapi/cpp/dev/video_decoder_client_dev.cc index aa00c6f..e0c174c 100644 --- a/ppapi/cpp/dev/video_decoder_client_dev.cc +++ b/ppapi/cpp/dev/video_decoder_client_dev.cc @@ -18,7 +18,6 @@ const char kPPPVideoDecoderInterface[] = PPP_VIDEODECODER_DEV_INTERFACE; // Callback to provide buffers for the decoded output pictures. void ProvidePictureBuffers(PP_Instance instance, - PP_Resource decoder_id, uint32_t req_num_of_bufs, struct PP_Size dimensions, enum PP_PictureBufferType_Dev type) { @@ -27,49 +26,41 @@ void ProvidePictureBuffers(PP_Instance instance, if (!object) return; static_cast<VideoDecoderClient_Dev*>(object)->ProvidePictureBuffers( - VideoDecoder_Dev(decoder_id), req_num_of_bufs, dimensions, type); + req_num_of_bufs, dimensions, type); } void DismissPictureBuffer(PP_Instance instance, - PP_Resource decoder_id, int32_t picture_buffer_id) { void* object = pp::Instance::GetPerInstanceObject( instance, kPPPVideoDecoderInterface); if (!object) return; static_cast<VideoDecoderClient_Dev*>(object)->DismissPictureBuffer( - VideoDecoder_Dev(decoder_id), picture_buffer_id); + picture_buffer_id); } -void PictureReady(PP_Instance instance, - PP_Resource decoder_id, - PP_Picture_Dev picture) { +void PictureReady(PP_Instance instance, PP_Picture_Dev picture) { void* object = pp::Instance::GetPerInstanceObject( instance, kPPPVideoDecoderInterface); if (!object) return; - static_cast<VideoDecoderClient_Dev*>(object)->PictureReady( - VideoDecoder_Dev(decoder_id), picture); + static_cast<VideoDecoderClient_Dev*>(object)->PictureReady(picture); } -void EndOfStream(PP_Instance instance, PP_Resource decoder_id) { +void EndOfStream(PP_Instance instance) { void* object = pp::Instance::GetPerInstanceObject( instance, kPPPVideoDecoderInterface); if (!object) return; - static_cast<VideoDecoderClient_Dev*>(object)->EndOfStream( - VideoDecoder_Dev(decoder_id)); + static_cast<VideoDecoderClient_Dev*>(object)->EndOfStream(); } -void NotifyError(PP_Instance instance, - PP_Resource decoder_id, - PP_VideoDecodeError_Dev error) { +void NotifyError(PP_Instance instance, PP_VideoDecodeError_Dev error) { void* object = pp::Instance::GetPerInstanceObject( instance, kPPPVideoDecoderInterface); if (!object) return; - static_cast<VideoDecoderClient_Dev*>(object)->NotifyError( - VideoDecoder_Dev(decoder_id), error); + static_cast<VideoDecoderClient_Dev*>(object)->NotifyError(error); } static PPP_VideoDecoder_Dev videodecoder_interface = { diff --git a/ppapi/cpp/dev/video_decoder_client_dev.h b/ppapi/cpp/dev/video_decoder_client_dev.h index 776d54b..d1c5742 100644 --- a/ppapi/cpp/dev/video_decoder_client_dev.h +++ b/ppapi/cpp/dev/video_decoder_client_dev.h @@ -24,29 +24,23 @@ class VideoDecoderClient_Dev { // Callback to provide buffers for the decoded output pictures. virtual void ProvidePictureBuffers( - VideoDecoder_Dev decoder, uint32_t req_num_of_bufs, struct PP_Size dimensions, enum PP_PictureBufferType_Dev type) = 0; - // Callback for decoder to delivered unneeded picture buffers back to the + // Callback for decoder to deliver unneeded picture buffers back to the // plugin. - virtual void DismissPictureBuffer( - VideoDecoder_Dev decoder, - int32_t picture_buffer_id) = 0; + virtual void DismissPictureBuffer(int32_t picture_buffer_id) = 0; // Callback to deliver decoded pictures ready to be displayed. - virtual void PictureReady( - VideoDecoder_Dev decoder, - const PP_Picture_Dev& picture) = 0; + virtual void PictureReady(const PP_Picture_Dev& picture) = 0; // Callback to notify that decoder has decoded end of stream marker and has // outputted all displayable pictures. - virtual void EndOfStream(VideoDecoder_Dev decoder) = 0; + virtual void EndOfStream() = 0; // Callback to notify about decoding errors. - virtual void NotifyError( - VideoDecoder_Dev decoder, PP_VideoDecodeError_Dev error) = 0; + virtual void NotifyError(PP_VideoDecodeError_Dev error) = 0; private: Instance* associated_instance_; diff --git a/ppapi/cpp/dev/video_decoder_dev.cc b/ppapi/cpp/dev/video_decoder_dev.cc index 89fe6ae..2ef19fc 100644 --- a/ppapi/cpp/dev/video_decoder_dev.cc +++ b/ppapi/cpp/dev/video_decoder_dev.cc @@ -32,7 +32,10 @@ VideoDecoder_Dev::VideoDecoder_Dev(const Instance& instance) { VideoDecoder_Dev::VideoDecoder_Dev(PP_Resource resource) : Resource(resource) { } -VideoDecoder_Dev::~VideoDecoder_Dev() {} +VideoDecoder_Dev::~VideoDecoder_Dev() { + get_interface<PPB_VideoDecoder_Dev>()->Destroy(pp_resource()); +} + int32_t VideoDecoder_Dev::Initialize(const PP_VideoConfigElement* config, const Context3D_Dev& context, @@ -82,11 +85,4 @@ int32_t VideoDecoder_Dev::Reset(CompletionCallback callback) { pp_resource(), callback.pp_completion_callback()); } -int32_t VideoDecoder_Dev::Destroy(CompletionCallback callback) { - if (!has_interface<PPB_VideoDecoder_Dev>()) - return callback.MayForce(PP_ERROR_NOINTERFACE); - return get_interface<PPB_VideoDecoder_Dev>()->Destroy( - pp_resource(), callback.pp_completion_callback()); -} - } // namespace pp diff --git a/ppapi/cpp/dev/video_decoder_dev.h b/ppapi/cpp/dev/video_decoder_dev.h index 03ddc59..3d6f2b9 100644 --- a/ppapi/cpp/dev/video_decoder_dev.h +++ b/ppapi/cpp/dev/video_decoder_dev.h @@ -42,7 +42,10 @@ class VideoDecoder_Dev : public Resource { void ReusePictureBuffer(int32_t picture_buffer_id); int32_t Flush(CompletionCallback callback); int32_t Reset(CompletionCallback callback); - int32_t Destroy(CompletionCallback callback); + + private: + // Disallow copy-construction to ensure Destroy() is called exactly once. + VideoDecoder_Dev(const VideoDecoder_Dev&); }; } // namespace pp diff --git a/ppapi/examples/gles2/gles2.cc b/ppapi/examples/gles2/gles2.cc index 60c5bdf..a7e076e 100644 --- a/ppapi/examples/gles2/gles2.cc +++ b/ppapi/examples/gles2/gles2.cc @@ -58,15 +58,12 @@ class GLES2DemoInstance : public pp::Instance, public pp::Graphics3DClient_Dev, // pp::VideoDecoderClient_Dev implementation. virtual void ProvidePictureBuffers( - pp::VideoDecoder_Dev decoder, uint32_t req_num_of_bufs, - PP_Size dimensions, PP_PictureBufferType_Dev type); - virtual void DismissPictureBuffer( - pp::VideoDecoder_Dev decoder, int32_t picture_buffer_id); - virtual void PictureReady( - pp::VideoDecoder_Dev decoder, const PP_Picture_Dev& picture); - virtual void EndOfStream(pp::VideoDecoder_Dev decoder); - virtual void NotifyError( - pp::VideoDecoder_Dev decoder, PP_VideoDecodeError_Dev error); + uint32_t req_num_of_bufs, PP_Size dimensions, + PP_PictureBufferType_Dev type); + virtual void DismissPictureBuffer(int32_t picture_buffer_id); + virtual void PictureReady(const PP_Picture_Dev& picture); + virtual void EndOfStream(); + virtual void NotifyError(PP_VideoDecodeError_Dev error); private: enum { kNumConcurrentDecodes = 7 }; @@ -78,7 +75,6 @@ class GLES2DemoInstance : public pp::Instance, public pp::Graphics3DClient_Dev, void DecoderInitDone(int32_t result); void DecoderBitstreamDone(int32_t result, int bitstream_buffer_id); void DecoderFlushDone(int32_t result); - void DecoderAbortDone(int32_t result); // Decode helpers. void DecodeNextNALUs(); @@ -146,7 +142,7 @@ GLES2DemoInstance::GLES2DemoInstance(PP_Instance instance, pp::Module* module) } GLES2DemoInstance::~GLES2DemoInstance() { - delete video_decoder_; + delete video_decoder_; // May be NULL, which is fine. delete surface_; delete context_; } @@ -194,9 +190,8 @@ void GLES2DemoInstance::DecoderFlushDone(int32_t result) { // Check that each bitstream buffer ID we handed to the decoder got handed // back to us. assert(bitstream_ids_at_decoder_.empty()); -} - -void GLES2DemoInstance::DecoderAbortDone(int32_t result) { + delete video_decoder_; + video_decoder_ = NULL; } static bool LookingAtNAL(const unsigned char* encoded, size_t pos) { @@ -256,7 +251,7 @@ void GLES2DemoInstance::DecodeNextNALU() { } void GLES2DemoInstance::ProvidePictureBuffers( - pp::VideoDecoder_Dev decoder, uint32_t req_num_of_bufs, PP_Size dimensions, + uint32_t req_num_of_bufs, PP_Size dimensions, PP_PictureBufferType_Dev type) { std::vector<PP_GLESBuffer_Dev> buffers; for (uint32_t i = 0; i < req_num_of_bufs; i++) { @@ -270,16 +265,14 @@ void GLES2DemoInstance::ProvidePictureBuffers( video_decoder_->AssignGLESBuffers(buffers); } -void GLES2DemoInstance::DismissPictureBuffer( - pp::VideoDecoder_Dev decoder, int32_t picture_buffer_id) { +void GLES2DemoInstance::DismissPictureBuffer(int32_t picture_buffer_id) { PictureBufferMap::iterator it = buffers_by_id_.find(picture_buffer_id); assert(it != buffers_by_id_.end()); DeleteTexture(it->second.texture_id); buffers_by_id_.erase(it); } -void GLES2DemoInstance::PictureReady( - pp::VideoDecoder_Dev decoder, const PP_Picture_Dev& picture) { +void GLES2DemoInstance::PictureReady(const PP_Picture_Dev& picture) { if (first_frame_delivered_ticks_ == -1) assert((first_frame_delivered_ticks_ = core_if_->GetTimeTicks()) != -1); if (is_painting_) { @@ -292,11 +285,10 @@ void GLES2DemoInstance::PictureReady( Render(it->second); } -void GLES2DemoInstance::EndOfStream(pp::VideoDecoder_Dev decoder) { +void GLES2DemoInstance::EndOfStream() { } -void GLES2DemoInstance::NotifyError( - pp::VideoDecoder_Dev decoder, PP_VideoDecodeError_Dev error) { +void GLES2DemoInstance::NotifyError(PP_VideoDecodeError_Dev error) { } // This object is the global object representing this plugin library as long @@ -366,11 +358,12 @@ void GLES2DemoInstance::PaintFinished(int32_t result, int picture_buffer_id) { << fps << ", with average ms/swap of: " << ms_per_swap << std::endl; } - video_decoder_->ReusePictureBuffer(picture_buffer_id); + if (video_decoder_) + video_decoder_->ReusePictureBuffer(picture_buffer_id); while (!pictures_pending_paint_.empty() && !is_painting_) { PP_Picture_Dev picture = pictures_pending_paint_.front(); pictures_pending_paint_.pop_front(); - PictureReady(*video_decoder_, picture); + PictureReady(picture); } } diff --git a/ppapi/thunk/ppb_video_decoder_api.h b/ppapi/thunk/ppb_video_decoder_api.h index d019e8e..15c540a 100644 --- a/ppapi/thunk/ppb_video_decoder_api.h +++ b/ppapi/thunk/ppb_video_decoder_api.h @@ -24,7 +24,7 @@ class PPB_VideoDecoder_API { virtual void ReusePictureBuffer(int32_t picture_buffer_id) = 0; virtual int32_t Flush(PP_CompletionCallback callback) = 0; virtual int32_t Reset(PP_CompletionCallback callback) = 0; - virtual int32_t Destroy(PP_CompletionCallback callback) = 0; + virtual void Destroy() = 0; }; } // namespace thunk diff --git a/ppapi/thunk/ppb_video_decoder_thunk.cc b/ppapi/thunk/ppb_video_decoder_thunk.cc index 434d804..afc3bde 100644 --- a/ppapi/thunk/ppb_video_decoder_thunk.cc +++ b/ppapi/thunk/ppb_video_decoder_thunk.cc @@ -81,13 +81,10 @@ int32_t Reset(PP_Resource video_decoder, return MayForceCallback(callback, result); } -int32_t Destroy(PP_Resource video_decoder, - PP_CompletionCallback callback) { +void Destroy(PP_Resource video_decoder) { EnterVideoDecoder enter(video_decoder, true); - if (enter.failed()) - return MayForceCallback(callback, PP_ERROR_BADRESOURCE); - int32_t result = enter.object()->Destroy(callback); - return MayForceCallback(callback, result); + if (enter.succeeded()) + enter.object()->Destroy(); } const PPB_VideoDecoder_Dev g_ppb_videodecoder_thunk = { diff --git a/webkit/plugins/ppapi/plugin_delegate.h b/webkit/plugins/ppapi/plugin_delegate.h index dae68a8..2b6e098 100644 --- a/webkit/plugins/ppapi/plugin_delegate.h +++ b/webkit/plugins/ppapi/plugin_delegate.h @@ -216,7 +216,7 @@ class PluginDelegate { // Interface for PlatformVideoDecoder is directly inherited from general media // VideoDecodeAccelerator interface. class PlatformVideoDecoder : public media::VideoDecodeAccelerator { - public: + protected: virtual ~PlatformVideoDecoder() {} }; diff --git a/webkit/plugins/ppapi/ppb_video_decoder_impl.cc b/webkit/plugins/ppapi/ppb_video_decoder_impl.cc index 234a906..0f887b8 100644 --- a/webkit/plugins/ppapi/ppb_video_decoder_impl.cc +++ b/webkit/plugins/ppapi/ppb_video_decoder_impl.cc @@ -36,7 +36,6 @@ PPB_VideoDecoder_Impl::PPB_VideoDecoder_Impl(PluginInstance* instance) : Resource(instance), callback_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)), context3d_id_(0), - destroy_callback_(PP_BlockUntilComplete()), flush_callback_(PP_BlockUntilComplete()), reset_callback_(PP_BlockUntilComplete()) { ppp_videodecoder_ = @@ -76,11 +75,10 @@ int32_t PPB_VideoDecoder_Impl::Initialize( if (command_buffer_route_id == 0) return PP_ERROR_FAILED; - platform_video_decoder_.reset( - instance()->delegate()->CreateVideoDecoder( - this, command_buffer_route_id, context3d->gles2_impl()->helper())); + platform_video_decoder_ = instance()->delegate()->CreateVideoDecoder( + this, command_buffer_route_id, context3d->gles2_impl()->helper()); - if (!platform_video_decoder_.get()) + if (!platform_video_decoder_) return PP_ERROR_FAILED; std::vector<uint32> copied; @@ -108,7 +106,7 @@ int32_t PPB_VideoDecoder_Impl::Initialize( int32_t PPB_VideoDecoder_Impl::Decode( const PP_VideoBitstreamBuffer_Dev* bitstream_buffer, PP_CompletionCallback callback) { - if (!platform_video_decoder_.get()) + if (!platform_video_decoder_) return PP_ERROR_BADRESOURCE; EnterResourceNoLock<PPB_Buffer_API> enter(bitstream_buffer->data, true); @@ -129,7 +127,7 @@ int32_t PPB_VideoDecoder_Impl::Decode( void PPB_VideoDecoder_Impl::AssignGLESBuffers( uint32_t no_of_buffers, const PP_GLESBuffer_Dev* buffers) { - if (!platform_video_decoder_.get()) + if (!platform_video_decoder_) return; std::vector<media::GLESBuffer> wrapped_buffers; @@ -145,13 +143,13 @@ void PPB_VideoDecoder_Impl::AssignGLESBuffers( } void PPB_VideoDecoder_Impl::ReusePictureBuffer(int32_t picture_buffer_id) { - if (!platform_video_decoder_.get()) + if (!platform_video_decoder_) return; platform_video_decoder_->ReusePictureBuffer(picture_buffer_id); } int32_t PPB_VideoDecoder_Impl::Flush(PP_CompletionCallback callback) { - if (!platform_video_decoder_.get()) + if (!platform_video_decoder_) return PP_ERROR_BADRESOURCE; // Store the callback to be called when Flush() is done. @@ -163,7 +161,7 @@ int32_t PPB_VideoDecoder_Impl::Flush(PP_CompletionCallback callback) { } int32_t PPB_VideoDecoder_Impl::Reset(PP_CompletionCallback callback) { - if (!platform_video_decoder_.get()) + if (!platform_video_decoder_) return PP_ERROR_BADRESOURCE; // Store the callback to be called when Reset() is done. @@ -174,16 +172,10 @@ int32_t PPB_VideoDecoder_Impl::Reset(PP_CompletionCallback callback) { return PP_OK_COMPLETIONPENDING; } -int32_t PPB_VideoDecoder_Impl::Destroy(PP_CompletionCallback callback) { - if (!platform_video_decoder_.get()) - return PP_ERROR_BADRESOURCE; - - // Store the callback to be called when Destroy() is done. - // TODO(fischman,vrk): consider implications of already-outstanding callback. - destroy_callback_ = callback; - +void PPB_VideoDecoder_Impl::Destroy() { + if (!platform_video_decoder_) + return; platform_video_decoder_->Destroy(); - return PP_OK_COMPLETIONPENDING; } void PPB_VideoDecoder_Impl::ProvidePictureBuffers( @@ -198,17 +190,14 @@ void PPB_VideoDecoder_Impl::ProvidePictureBuffers( PP_PictureBufferType_Dev out_type = static_cast<PP_PictureBufferType_Dev>(type); PP_Size out_dim = PP_MakeSize(dimensions.width(), dimensions.height()); - ScopedResourceId resource(this); ppp_videodecoder_->ProvidePictureBuffers( - instance()->pp_instance(), resource.id, requested_num_of_buffers, - out_dim, out_type); + instance()->pp_instance(), requested_num_of_buffers, out_dim, out_type); } void PPB_VideoDecoder_Impl::PictureReady(const media::Picture& picture) { if (!ppp_videodecoder_) return; - ScopedResourceId resource(this); PP_Picture_Dev output; output.picture_buffer_id = picture.picture_buffer_id(); output.bitstream_buffer_id = picture.bitstream_buffer_id(); @@ -216,25 +205,22 @@ void PPB_VideoDecoder_Impl::PictureReady(const media::Picture& picture) { picture.visible_size().height()); output.decoded_size = PP_MakeSize(picture.decoded_size().width(), picture.decoded_size().height()); - ppp_videodecoder_->PictureReady( - instance()->pp_instance(), resource.id, output); + ppp_videodecoder_->PictureReady(instance()->pp_instance(), output); } void PPB_VideoDecoder_Impl::DismissPictureBuffer(int32 picture_buffer_id) { if (!ppp_videodecoder_) return; - ScopedResourceId resource(this); ppp_videodecoder_->DismissPictureBuffer( - instance()->pp_instance(), resource.id, picture_buffer_id); + instance()->pp_instance(), picture_buffer_id); } void PPB_VideoDecoder_Impl::NotifyEndOfStream() { if (!ppp_videodecoder_) return; - ScopedResourceId resource(this); - ppp_videodecoder_->EndOfStream(instance()->pp_instance(), resource.id); + ppp_videodecoder_->EndOfStream(instance()->pp_instance()); } void PPB_VideoDecoder_Impl::NotifyError( @@ -242,13 +228,12 @@ void PPB_VideoDecoder_Impl::NotifyError( if (!ppp_videodecoder_) return; - ScopedResourceId resource(this); // TODO(vrk): This is assuming VideoDecodeAccelerator::Error and // PP_VideoDecodeError_Dev have identical enum values. There is no compiler // assert to guarantee this. We either need to add such asserts or // merge these two enums. - ppp_videodecoder_->NotifyError(instance()->pp_instance(), resource.id, - static_cast<PP_VideoDecodeError_Dev>(error)); + ppp_videodecoder_->NotifyError(instance()->pp_instance(), + static_cast<PP_VideoDecodeError_Dev>(error)); } void PPB_VideoDecoder_Impl::NotifyResetDone() { @@ -259,14 +244,6 @@ void PPB_VideoDecoder_Impl::NotifyResetDone() { PP_RunAndClearCompletionCallback(&reset_callback_, PP_OK); } -void PPB_VideoDecoder_Impl::NotifyDestroyDone() { - if (destroy_callback_.func == NULL) - return; - - // Call the callback that was stored to be called when Destroy is done. - PP_RunAndClearCompletionCallback(&destroy_callback_, PP_OK); -} - void PPB_VideoDecoder_Impl::NotifyEndOfBitstreamBuffer( int32 bitstream_buffer_id) { CallbackById::iterator it = diff --git a/webkit/plugins/ppapi/ppb_video_decoder_impl.h b/webkit/plugins/ppapi/ppb_video_decoder_impl.h index 76c5b6e..b3137b7 100644 --- a/webkit/plugins/ppapi/ppb_video_decoder_impl.h +++ b/webkit/plugins/ppapi/ppb_video_decoder_impl.h @@ -49,7 +49,7 @@ class PPB_VideoDecoder_Impl : public Resource, virtual void ReusePictureBuffer(int32_t picture_buffer_id) OVERRIDE; virtual int32_t Flush(PP_CompletionCallback callback) OVERRIDE; virtual int32_t Reset(PP_CompletionCallback callback) OVERRIDE; - virtual int32_t Destroy(PP_CompletionCallback callback) OVERRIDE; + virtual void Destroy() OVERRIDE; // media::VideoDecodeAccelerator::Client implementation. virtual void ProvidePictureBuffers( @@ -65,7 +65,6 @@ class PPB_VideoDecoder_Impl : public Resource, virtual void NotifyFlushDone() OVERRIDE; virtual void NotifyEndOfBitstreamBuffer(int32 buffer_id) OVERRIDE; virtual void NotifyResetDone() OVERRIDE; - virtual void NotifyDestroyDone() OVERRIDE; private: // Key: bitstream_buffer_id, value: callback to run when bitstream decode is @@ -74,7 +73,7 @@ class PPB_VideoDecoder_Impl : public Resource, // This is NULL before initialization, and if this PPB_VideoDecoder_Impl is // swapped with another. - scoped_ptr<PluginDelegate::PlatformVideoDecoder> platform_video_decoder_; + scoped_refptr<PluginDelegate::PlatformVideoDecoder> platform_video_decoder_; // Factory to produce our callbacks. base::ScopedCallbackFactory<PPB_VideoDecoder_Impl> callback_factory_; @@ -84,7 +83,6 @@ class PPB_VideoDecoder_Impl : public Resource, PP_Resource context3d_id_; PP_CompletionCallback initialization_callback_; - PP_CompletionCallback destroy_callback_; PP_CompletionCallback flush_callback_; PP_CompletionCallback reset_callback_; CallbackById bitstream_buffer_callbacks_; |