Skip to content

Commit

Permalink
Check that MediaCodec sends FORMAT_CHANGED before decoded buffers.
Browse files Browse the repository at this point in the history
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}
  • Loading branch information
liberato-at-chromium authored and Commit bot committed Mar 1, 2016
1 parent 542f9e5 commit 46e95bb
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 1 deletion.
24 changes: 24 additions & 0 deletions content/common/gpu/media/android_video_decode_accelerator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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.";
Expand Down
22 changes: 21 additions & 1 deletion gpu/config/software_rendering_list_json.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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"
]
}
]
}
Expand Down
15 changes: 15 additions & 0 deletions tools/metrics/histograms/histograms.xml
Original file line number Diff line number Diff line change
Expand Up @@ -19372,6 +19372,16 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
</summary>
</histogram>

<histogram name="Media.AVDA.MissingFormatChanged" enum="BooleanFormatChanged">
<owner>[email protected]</owner>
<summary>
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.
</summary>
</histogram>

<histogram name="Media.AvdaCodecImage.WaitTimeForFrame" units="ms">
<owner>[email protected]</owner>
<summary>
Expand Down Expand Up @@ -59755,6 +59765,11 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
<int value="1" label="Force Disabled"/>
</enum>

<enum name="BooleanFormatChanged" type="int">
<int value="0" label="Any Codec Initialized"/>
<int value="1" label="Missing FORMAT_CHANGED message"/>
</enum>

<enum name="BooleanFormManager" type="int">
<int value="0" label="Has Form Manager"/>
<int value="1" label="Lacks Form Manager"/>
Expand Down

0 comments on commit 46e95bb

Please sign in to comment.