From 46e95bb6168b76d8664d3dc9bd971d06be4a2fc0 Mon Sep 17 00:00:00 2001 From: liberato Date: Mon, 29 Feb 2016 16:25:55 -0800 Subject: [PATCH] Check that MediaCodec sends FORMAT_CHANGED before decoded buffers. It looks like MediaCodec erroneously returns decoded frames before sending an OUTPUT_FORMAT_CHANGED notification. This will cause us to SendDecodedFrameToClient, then crashes when it tries to get a picture buffer to use. MediaCodec is not supposed to do that, see EncodeDecodeTest.java line 617 in the android cts tests. Once it does, it's unclear what state it's in. We post an error and stop decoding. I do not have a local repro, so this is my best guess about what is causing some reported crashes. They seem to happen only on JB with Adreno 330 GPUs (Snapdragon 800 series), driver version 45. This combination has been added to the blacklist for accelerated video decoding (AVDA). This CL also adds a metric for how often this occurs, to help to identify other affected devices. BUG=585963 CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel Review URL: https://codereview.chromium.org/1685303004 Cr-Commit-Position: refs/heads/master@{#378340} --- .../media/android_video_decode_accelerator.cc | 24 +++++++++++++++++++ gpu/config/software_rendering_list_json.cc | 22 ++++++++++++++++- tools/metrics/histograms/histograms.xml | 15 ++++++++++++ 3 files changed, 60 insertions(+), 1 deletion(-) diff --git a/content/common/gpu/media/android_video_decode_accelerator.cc b/content/common/gpu/media/android_video_decode_accelerator.cc index 228473ceeebe2..3942a80ff3b4b 100644 --- a/content/common/gpu/media/android_video_decode_accelerator.cc +++ b/content/common/gpu/media/android_video_decode_accelerator.cc @@ -106,6 +106,16 @@ static inline const base::TimeDelta ErrorPostingDelay() { return base::TimeDelta::FromSeconds(2); } +// For RecordFormatChangedMetric. +enum FormatChangedValue { + CodecInitialized = false, + MissingFormatChanged = true +}; + +static inline void RecordFormatChangedMetric(FormatChangedValue value) { + UMA_HISTOGRAM_BOOLEAN("Media.AVDA.MissingFormatChanged", !!value); +} + // Handle OnFrameAvailable callbacks safely. Since they occur asynchronously, // we take care that the AVDA that wants them still exists. A WeakPtr to // the AVDA would be preferable, except that OnFrameAvailable callbacks can @@ -631,6 +641,17 @@ bool AndroidVideoDecodeAccelerator::DequeueOutput() { return false; } + if (!picturebuffers_requested_) { + // If, somehow, we get a decoded frame back before a FORMAT_CHANGED + // message, then we might not have any picture buffers to use. This + // isn't supposed to happen (see EncodeDecodeTest.java#617). + // Log a metric to see how common this is. + RecordFormatChangedMetric(FormatChangedValue::MissingFormatChanged); + media_codec_->ReleaseOutputBuffer(buf_index, false); + POST_ERROR(PLATFORM_FAILURE, "Dequeued buffers before FORMAT_CHANGED."); + return false; + } + // Get the bitstream buffer id from the timestamp. auto it = bitstream_buffers_in_decoder_.find(presentation_timestamp); @@ -825,6 +846,9 @@ bool AndroidVideoDecodeAccelerator::ConfigureMediaCodec() { codec_, needs_protected_surface_, gfx::Size(320, 240), surface_.j_surface().obj(), media_crypto, false)); + // Record one instance of the codec being initialized. + RecordFormatChangedMetric(FormatChangedValue::CodecInitialized); + strategy_->CodecChanged(media_codec_.get(), output_picture_buffers_); if (!media_codec_) { LOG(ERROR) << "Failed to create MediaCodec instance."; diff --git a/gpu/config/software_rendering_list_json.cc b/gpu/config/software_rendering_list_json.cc index 484467d0cd597..fb8b84c29789b 100644 --- a/gpu/config/software_rendering_list_json.cc +++ b/gpu/config/software_rendering_list_json.cc @@ -18,7 +18,7 @@ const char kSoftwareRenderingListJson[] = LONG_STRING_CONST( { "name": "software rendering list", // Please update the version number whenever you change this file. - "version": "10.17", + "version": "10.18", "entries": [ { "id": 1, @@ -1189,6 +1189,26 @@ LONG_STRING_CONST( "gpu_rasterization", "accelerated_2d_canvas" ] + }, + { + "id": 109, + "description": "MediaCodec on Adreno 330 / 4.2.2 doesn't always send FORMAT_CHANGED", + "cr_bugs": [585963], + "os": { + "type": "android", + "version": { + "op": "=", + "value": "4.2.2" + } + }, + "gl_renderer": "Adreno \\(TM\\) 330", + "driver_version": { + "op": "=", + "value": "45.0" + }, + "features": [ + "accelerated_video_decode" + ] } ] } diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index f47f6a8feb182..8a6d9bf6e7afc 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml @@ -19372,6 +19372,16 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries. + + liberato@chromium.org + + Number of times that AVDA stopped decoding because MediaCodec failed to + provide a FORMAT_CHANGED message before sending decoded frames back. True + counts indicate instances where FORMAT_CHANGE was missed, while false + instances indicate any MediaCodec initialization by AVDA. + + + liberato@chromium.org @@ -59755,6 +59765,11 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries. + + + + +