Commit eede381b authored by liberato@chromium.org's avatar liberato@chromium.org Committed by Commit Bot

CodecImage should fail rather than dcheck if it's been recycled.

Previously, CodecImage::Render...Buffer() methods would dcheck if
there was no wait coordinator; this would indicate that it has
been recycled before all uses were done.  It turns out, however,
that it doesn't quite always imply that.

During teardown of VideoFrameSubmitter (possibly when the renderer
is destroyed, or just the video element), the VideoFrames held by
the renderer are dropped.  This triggers recycling on the gpu side,
and calls CodecImage::NotifyUnused.  Unfortunately, nobody ever
got any returned resources from viz, so viz might still try to
draw with the CodecImage.

This CL just makes those calls fail, so we'll just use whatever
was in the texture already.

Bug: 1054528
Change-Id: Iabe40d92e48f972381145a83e20ce35d1e0bf443
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2078800
Commit-Queue: Frank Liberato <liberato@chromium.org>
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: default avatarDale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#745295}
parent 344208da
......@@ -248,13 +248,20 @@ bool CodecImage::RenderToFrontBuffer() {
}
bool CodecImage::RenderToTextureOwnerBackBuffer(BlockingMode blocking_mode) {
DCHECK(codec_buffer_wait_coordinator_);
DCHECK_NE(phase_, Phase::kInFrontBuffer);
if (phase_ == Phase::kInBackBuffer)
return true;
if (phase_ == Phase::kInvalidated)
return false;
// Normally, we should have a wait coordinator if we're called. However, if
// the renderer is torn down (either VideoFrameSubmitter or the whole process)
// before we get returns back from viz, then we can be notified that we're
// no longer in use (erroneously) when the VideoFrame is destroyed. So, if
// we don't have a wait coordinator, then just fail.
if (!codec_buffer_wait_coordinator_)
return false;
// Wait for a previous frame available so we don't confuse it with the one
// we're about to release.
if (codec_buffer_wait_coordinator_->IsExpectingFrameAvailable()) {
......@@ -272,7 +279,13 @@ bool CodecImage::RenderToTextureOwnerBackBuffer(BlockingMode blocking_mode) {
}
bool CodecImage::RenderToTextureOwnerFrontBuffer(BindingsMode bindings_mode) {
DCHECK(codec_buffer_wait_coordinator_);
// Normally, we should have a wait coordinator if we're called. However, if
// the renderer is torn down (either VideoFrameSubmitter or the whole process)
// before we get returns back from viz, then we can be notified that we're
// no longer in use (erroneously) when the VideoFrame is destroyed. So, if
// we don't have a wait coordinator, then just fail.
if (!codec_buffer_wait_coordinator_)
return false;
if (phase_ == Phase::kInFrontBuffer) {
EnsureBoundIfNeeded(bindings_mode);
......
......@@ -171,6 +171,8 @@ class MEDIA_GPU_EXPORT CodecImage
~CodecImage() override;
private:
FRIEND_TEST_ALL_PREFIXES(CodecImageTest, RenderAfterUnusedDoesntCrash);
// The lifecycle phases of an image.
// The only possible transitions are from left to right. Both
// kInFrontBuffer and kInvalidated are terminal.
......
......@@ -422,4 +422,12 @@ TEST_F(CodecImageTest, GetCropRect) {
codec_buffer_wait_coordinator_->texture_owner()->get_crop_rect_count, 1);
}
TEST_F(CodecImageTest, RenderAfterUnusedDoesntCrash) {
auto i = NewImage(kTextureOwner);
i->NotifyUnused();
EXPECT_FALSE(i->RenderToTextureOwnerBackBuffer());
EXPECT_FALSE(i->RenderToTextureOwnerFrontBuffer(
CodecImage::BindingsMode::kEnsureTexImageBound));
}
} // namespace media
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