Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(563)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: media/gpu/android_video_decode_accelerator.cc

Issue 2487813002: Remove delayed error posting from AndroidVideoDecodeAccelerator (Closed)
Patch Set: Rebase Created 4 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « media/gpu/android_video_decode_accelerator.h ('k') | media/gpu/avda_picture_buffer_manager.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: media/gpu/android_video_decode_accelerator.cc
diff --git a/media/gpu/android_video_decode_accelerator.cc b/media/gpu/android_video_decode_accelerator.cc
index 3bec30326f5dc36493da0e8e4ba7865c61b7bc9c..de67510f8e0cf7ef6798dc8bd661536951cb5bc5 100644
--- a/media/gpu/android_video_decode_accelerator.cc
+++ b/media/gpu/android_video_decode_accelerator.cc
@@ -46,10 +46,10 @@
#include "media/mojo/services/mojo_cdm_service.h"
#endif
-#define POST_ERROR(error_code, error_message) \
- do { \
- DLOG(ERROR) << error_message; \
- PostError(FROM_HERE, VideoDecodeAccelerator::error_code); \
+#define NOTIFY_ERROR(error_code, error_message) \
+ do { \
+ DLOG(ERROR) << error_message; \
+ NotifyError(VideoDecodeAccelerator::error_code); \
} while (0)
namespace media {
@@ -106,11 +106,6 @@ constexpr base::TimeDelta NoWaitTimeOut = base::TimeDelta::FromMicroseconds(0);
constexpr base::TimeDelta IdleTimerTimeOut = base::TimeDelta::FromSeconds(1);
-// Time between when we notice an error, and when we actually notify somebody.
-// This is to prevent codec errors caused by SurfaceView fullscreen transitions
-// from breaking the pipeline, if we're about to be reset anyway.
-constexpr base::TimeDelta ErrorPostingDelay = base::TimeDelta::FromSeconds(2);
-
} // namespace
static base::LazyInstance<AVDACodecAllocator>::Leaky g_avda_codec_allocator =
@@ -313,8 +308,6 @@ AndroidVideoDecodeAccelerator::AndroidVideoDecodeAccelerator(
media_drm_bridge_cdm_context_(nullptr),
cdm_registration_id_(0),
pending_input_buf_index_(-1),
- error_sequence_token_(0),
- defer_errors_(false),
deferred_initialization_pending_(false),
codec_needs_reset_(false),
defer_surface_creation_(false),
@@ -509,7 +502,6 @@ void AndroidVideoDecodeAccelerator::DoIOTask(bool start_timer) {
bool AndroidVideoDecodeAccelerator::QueueInput() {
DCHECK(thread_checker_.CalledOnValidThread());
TRACE_EVENT0("media", "AVDA::QueueInput");
- base::AutoReset<bool> auto_reset(&defer_errors_, true);
if (state_ == ERROR || state_ == WAITING_FOR_CODEC ||
state_ == WAITING_FOR_KEY) {
return false;
@@ -531,12 +523,12 @@ bool AndroidVideoDecodeAccelerator::QueueInput() {
case MEDIA_CODEC_DEQUEUE_INPUT_AGAIN_LATER:
return false;
case MEDIA_CODEC_ERROR:
- POST_ERROR(PLATFORM_FAILURE, "Failed to DequeueInputBuffer");
+ NOTIFY_ERROR(PLATFORM_FAILURE, "DequeueInputBuffer failed");
return false;
case MEDIA_CODEC_OK:
break;
default:
- NOTREACHED() << "Unknown DequeueInputBuffer status " << status;
+ NOTREACHED();
return false;
}
}
@@ -563,7 +555,7 @@ bool AndroidVideoDecodeAccelerator::QueueInput() {
shm = std::move(pending_bitstream_records_.front().memory);
if (!shm->Map()) {
- POST_ERROR(UNREADABLE_INPUT, "Failed to SharedMemoryRegion::Map()");
+ NOTIFY_ERROR(UNREADABLE_INPUT, "SharedMemoryRegion::Map() failed");
return false;
}
}
@@ -631,7 +623,7 @@ bool AndroidVideoDecodeAccelerator::QueueInput() {
bitstreams_notified_in_advance_.push_back(bitstream_buffer.id());
if (status != MEDIA_CODEC_OK) {
- POST_ERROR(PLATFORM_FAILURE, "Failed to QueueInputBuffer: " << status);
+ NOTIFY_ERROR(PLATFORM_FAILURE, "QueueInputBuffer failed:" << status);
return false;
}
@@ -641,7 +633,6 @@ bool AndroidVideoDecodeAccelerator::QueueInput() {
bool AndroidVideoDecodeAccelerator::DequeueOutput() {
DCHECK(thread_checker_.CalledOnValidThread());
TRACE_EVENT0("media", "AVDA::DequeueOutput");
- base::AutoReset<bool> auto_reset(&defer_errors_, true);
if (state_ == ERROR || state_ == WAITING_FOR_CODEC)
return false;
// If we're draining for reset or destroy, then we don't need picture buffers
@@ -693,7 +684,7 @@ bool AndroidVideoDecodeAccelerator::DequeueOutput() {
state_ = ERROR;
OnDrainCompleted();
} else {
- POST_ERROR(PLATFORM_FAILURE, "DequeueOutputBuffer failed.");
+ NOTIFY_ERROR(PLATFORM_FAILURE, "DequeueOutputBuffer failed.");
}
return false;
@@ -708,7 +699,7 @@ bool AndroidVideoDecodeAccelerator::DequeueOutput() {
return true; // ignore
if (media_codec_->GetOutputSize(&size_) != MEDIA_CODEC_OK) {
- POST_ERROR(PLATFORM_FAILURE, "GetOutputSize failed.");
+ NOTIFY_ERROR(PLATFORM_FAILURE, "GetOutputSize failed.");
return false;
}
@@ -764,7 +755,7 @@ bool AndroidVideoDecodeAccelerator::DequeueOutput() {
// In 0.01% of playbacks MediaCodec returns a frame before FORMAT_CHANGED.
// Occurs on JB and M. (See the Media.AVDA.MissingFormatChanged histogram.)
media_codec_->ReleaseOutputBuffer(buf_index, false);
- POST_ERROR(PLATFORM_FAILURE, "Dequeued buffers before FORMAT_CHANGED.");
+ NOTIFY_ERROR(PLATFORM_FAILURE, "Dequeued buffers before FORMAT_CHANGED.");
return false;
}
@@ -814,7 +805,7 @@ void AndroidVideoDecodeAccelerator::SendDecodedFrameToClient(
TRACE_EVENT0("media", "AVDA::SendDecodedFrameToClient");
if (!make_context_current_cb_.Run()) {
- POST_ERROR(PLATFORM_FAILURE, "Failed to make the GL context current.");
+ NOTIFY_ERROR(PLATFORM_FAILURE, "Failed to make the GL context current.");
return;
}
@@ -824,8 +815,8 @@ void AndroidVideoDecodeAccelerator::SendDecodedFrameToClient(
const auto it = output_picture_buffers_.find(picture_buffer_id);
if (it == output_picture_buffers_.end()) {
- POST_ERROR(PLATFORM_FAILURE,
- "Can't find PictureBuffer id: " << picture_buffer_id);
+ NOTIFY_ERROR(PLATFORM_FAILURE,
+ "Can't find PictureBuffer id: " << picture_buffer_id);
return;
}
@@ -857,8 +848,8 @@ void AndroidVideoDecodeAccelerator::Decode(
DCHECK(thread_checker_.CalledOnValidThread());
if (defer_surface_creation_ && !InitializePictureBufferManager()) {
- POST_ERROR(PLATFORM_FAILURE,
- "Failed deferred surface and MediaCodec initialization.");
+ NOTIFY_ERROR(PLATFORM_FAILURE,
+ "Failed deferred surface and MediaCodec initialization.");
return;
}
@@ -878,8 +869,8 @@ void AndroidVideoDecodeAccelerator::Decode(
base::SharedMemory::CloseHandle(bitstream_buffer.handle());
if (bitstream_buffer.id() < 0) {
- POST_ERROR(INVALID_ARGUMENT,
- "Invalid bistream_buffer, id: " << bitstream_buffer.id());
+ NOTIFY_ERROR(INVALID_ARGUMENT,
+ "Invalid bistream_buffer, id: " << bitstream_buffer.id());
} else {
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE,
@@ -917,7 +908,7 @@ void AndroidVideoDecodeAccelerator::AssignPictureBuffers(
DCHECK(free_picture_ids_.empty());
if (buffers.size() < kNumPictureBuffers) {
- POST_ERROR(INVALID_ARGUMENT, "Not enough picture buffers assigned.");
+ NOTIFY_ERROR(INVALID_ARGUMENT, "Not enough picture buffers assigned.");
return;
}
@@ -946,8 +937,8 @@ void AndroidVideoDecodeAccelerator::ReusePictureBuffer(
auto it = output_picture_buffers_.find(picture_buffer_id);
if (it == output_picture_buffers_.end()) {
- POST_ERROR(PLATFORM_FAILURE, "Can't find PictureBuffer id "
- << picture_buffer_id);
+ NOTIFY_ERROR(PLATFORM_FAILURE, "Can't find PictureBuffer id "
+ << picture_buffer_id);
return;
}
@@ -1079,7 +1070,7 @@ void AndroidVideoDecodeAccelerator::OnCodecConfigured(
media_codec_ = std::move(media_codec);
picture_buffer_manager_.CodecChanged(media_codec_.get());
if (!media_codec_) {
- POST_ERROR(PLATFORM_FAILURE, "Failed to create MediaCodec.");
+ NOTIFY_ERROR(PLATFORM_FAILURE, "Failed to create MediaCodec");
return;
}
@@ -1181,14 +1172,6 @@ void AndroidVideoDecodeAccelerator::ResetCodecState() {
return;
}
- // We might increment error_sequence_token here to cancel any delayed errors,
- // but right now it's unclear that it's safe to do so. If we are in an error
- // state because of a codec error, then it would be okay. Otherwise, it's
- // less obvious that we are exiting the error state. Since deferred errors
- // are only intended for fullscreen transitions right now, we take the more
- // conservative approach and let the errors post.
- // TODO(liberato): revisit this once we sort out the error state a bit more.
-
// Flush the codec if possible, or create a new one if not.
if (!did_codec_error_happen &&
!MediaCodecUtil::CodecNeedsFlushWorkaround(media_codec_.get())) {
@@ -1235,9 +1218,6 @@ void AndroidVideoDecodeAccelerator::Reset() {
TRACE_COUNTER1("media", "AVDA::PendingBitstreamBufferCount", 0);
bitstreams_notified_in_advance_.clear();
- // Any error that is waiting to post can be ignored.
- error_sequence_token_++;
-
picture_buffer_manager_.ReleaseCodecBuffers(output_picture_buffers_);
// Some VP8 files require complete MediaCodec drain before we can call
@@ -1370,17 +1350,6 @@ void AndroidVideoDecodeAccelerator::OnDestroyingSurface(int surface_id) {
OnDrainCompleted();
}
-void AndroidVideoDecodeAccelerator::PostError(
- const ::tracked_objects::Location& from_here,
- VideoDecodeAccelerator::Error error) {
- base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
- from_here,
- base::Bind(&AndroidVideoDecodeAccelerator::NotifyError,
- weak_this_factory_.GetWeakPtr(), error, error_sequence_token_),
- (defer_errors_ ? ErrorPostingDelay : base::TimeDelta()));
- state_ = ERROR;
-}
-
void AndroidVideoDecodeAccelerator::InitializeCdm() {
DVLOG(2) << __FUNCTION__ << ": " << config_.cdm_id;
@@ -1479,14 +1448,8 @@ void AndroidVideoDecodeAccelerator::NotifyResetDone() {
client_->NotifyResetDone();
}
-void AndroidVideoDecodeAccelerator::NotifyError(
- VideoDecodeAccelerator::Error error,
- int token) {
- DVLOG(1) << __FUNCTION__ << ": error: " << error << " token: " << token
- << " current: " << error_sequence_token_;
- if (token != error_sequence_token_)
- return;
-
+void AndroidVideoDecodeAccelerator::NotifyError(Error error) {
+ state_ = ERROR;
if (client_)
client_->NotifyError(error);
}
@@ -1636,9 +1599,9 @@ bool AndroidVideoDecodeAccelerator::UpdateSurface() {
// Ensure the current context is active when switching surfaces; we may need
// to create a new texture.
if (!make_context_current_cb_.Run()) {
- POST_ERROR(PLATFORM_FAILURE,
- "Failed to make this decoder's GL context current when "
- "switching surfaces.");
+ NOTIFY_ERROR(PLATFORM_FAILURE,
+ "Failed to make this decoder's GL context current when "
+ "switching surfaces.");
return false;
}
@@ -1646,13 +1609,13 @@ bool AndroidVideoDecodeAccelerator::UpdateSurface() {
codec_config_->surface_ =
picture_buffer_manager_.Initialize(pending_surface_id_.value());
if (codec_config_->surface_.IsEmpty()) {
- POST_ERROR(PLATFORM_FAILURE, "An error occurred while switching surfaces.");
+ NOTIFY_ERROR(PLATFORM_FAILURE, "Failed to switch surfaces.");
return false;
}
if (media_codec_ &&
!media_codec_->SetSurface(codec_config_->surface_.j_surface().obj())) {
- POST_ERROR(PLATFORM_FAILURE, "MediaCodec failed to switch surfaces.");
+ NOTIFY_ERROR(PLATFORM_FAILURE, "MediaCodec failed to switch surfaces.");
return false;
}
« no previous file with comments | « media/gpu/android_video_decode_accelerator.h ('k') | media/gpu/avda_picture_buffer_manager.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698