Commit 3ba8f31f authored by Frank Liberato's avatar Frank Liberato Committed by Commit Bot

Check for output buffers more often in MediaCodecVideoDecoder.

When checking for output buffers while a surface transition is
pending, MCVD will choose to wait if there are still unrendered
output buffers.  However, once the last buffer is rendererd,
MCVD didn't always realize to try again; it did so only if it
ran the codec pump for some reason (e.g., a new DecoderBuffer or
a new codec buffer became available).

This CL causes it to try again whenever an output buffer is
rendered, in case it can make progress because of it.

Bug: 1148995
Change-Id: Ife1ba993424491af4a29c71a979e5bb0c7b79eb9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2547227Reviewed-by: default avatarDale Curtis <dalecurtis@chromium.org>
Commit-Queue: Frank Liberato <liberato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#829315}
parent d8cfe1a5
...@@ -125,7 +125,10 @@ CodecOutputBuffer::~CodecOutputBuffer() { ...@@ -125,7 +125,10 @@ CodecOutputBuffer::~CodecOutputBuffer() {
bool CodecOutputBuffer::ReleaseToSurface() { bool CodecOutputBuffer::ReleaseToSurface() {
was_rendered_ = true; was_rendered_ = true;
return codec_->ReleaseCodecOutputBuffer(id_, true); auto result = codec_->ReleaseCodecOutputBuffer(id_, true);
if (render_cb_)
std::move(render_cb_).Run();
return result;
} }
CodecWrapperImpl::CodecWrapperImpl( CodecWrapperImpl::CodecWrapperImpl(
......
...@@ -44,6 +44,12 @@ class MEDIA_GPU_EXPORT CodecOutputBuffer { ...@@ -44,6 +44,12 @@ class MEDIA_GPU_EXPORT CodecOutputBuffer {
// The size of the image. // The size of the image.
gfx::Size size() const { return size_; } gfx::Size size() const { return size_; }
// Sets a callback that will be called when we're released to the surface.
// Will not be called if we're dropped.
void set_render_cb(base::OnceClosure render_cb) {
render_cb_ = std::move(render_cb);
}
// Note that you can't use the first ctor, since CodecWrapperImpl isn't // Note that you can't use the first ctor, since CodecWrapperImpl isn't
// defined here. Use the second, and it'll be nullptr. // defined here. Use the second, and it'll be nullptr.
template <typename... Args> template <typename... Args>
...@@ -67,6 +73,7 @@ class MEDIA_GPU_EXPORT CodecOutputBuffer { ...@@ -67,6 +73,7 @@ class MEDIA_GPU_EXPORT CodecOutputBuffer {
int64_t id_; int64_t id_;
bool was_rendered_ = false; bool was_rendered_ = false;
gfx::Size size_; gfx::Size size_;
base::OnceClosure render_cb_;
DISALLOW_COPY_AND_ASSIGN(CodecOutputBuffer); DISALLOW_COPY_AND_ASSIGN(CodecOutputBuffer);
}; };
......
...@@ -355,4 +355,22 @@ TEST_F(CodecWrapperTest, CodecWrapperPostsReleaseToProvidedThread) { ...@@ -355,4 +355,22 @@ TEST_F(CodecWrapperTest, CodecWrapperPostsReleaseToProvidedThread) {
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
} }
TEST_F(CodecWrapperTest, RenderCallbackCalledIfRendered) {
auto codec_buffer = DequeueCodecOutputBuffer();
bool flag = false;
codec_buffer->set_render_cb(base::BindOnce([](bool* flag) { *flag = true; },
base::Unretained(&flag)));
codec_buffer->ReleaseToSurface();
EXPECT_TRUE(flag);
}
TEST_F(CodecWrapperTest, RenderCallbackIsNotCalledIfNotRendered) {
auto codec_buffer = DequeueCodecOutputBuffer();
bool flag = false;
codec_buffer->set_render_cb(base::BindOnce([](bool* flag) { *flag = true; },
base::Unretained(&flag)));
codec_buffer.reset();
EXPECT_FALSE(flag);
}
} // namespace media } // namespace media
...@@ -966,6 +966,16 @@ bool MediaCodecVideoDecoder::DequeueOutput() { ...@@ -966,6 +966,16 @@ bool MediaCodecVideoDecoder::DequeueOutput() {
std::unique_ptr<ScopedAsyncTrace> async_trace = std::unique_ptr<ScopedAsyncTrace> async_trace =
ScopedAsyncTrace::CreateIfEnabled( ScopedAsyncTrace::CreateIfEnabled(
"MediaCodecVideoDecoder::CreateVideoFrame"); "MediaCodecVideoDecoder::CreateVideoFrame");
// Make sure that we're notified when this is rendered. Otherwise, if we're
// waiting for all output buffers to drain so that we can swap the output
// surface, we might not realize that we may continue. If we're using
// SurfaceControl overlays, then this isn't needed; there is never a surface
// transition anyway.
if (!is_surface_control_enabled_) {
output_buffer->set_render_cb(BindToCurrentLoop(
base::BindOnce(&MediaCodecVideoDecoder::StartTimerOrPumpCodec,
weak_factory_.GetWeakPtr())));
}
video_frame_factory_->CreateVideoFrame( video_frame_factory_->CreateVideoFrame(
std::move(output_buffer), presentation_time, std::move(output_buffer), presentation_time,
GetNaturalSize(visible_rect, decoder_config_.GetPixelAspectRatio()), GetNaturalSize(visible_rect, decoder_config_.GetPixelAspectRatio()),
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment