Commit 6f002a06 authored by Scott Violet's avatar Scott Violet Committed by Commit Bot

Revert "MojoVideoDecoder: Add histogram to track initialization and error."

This reverts commit 6aac2af3.

Reason for revert: Caused breakage of mash test. See https://crbug.com/902870.

Original change's description:
> MojoVideoDecoder: Add histogram to track initialization and error.
> 
> The autotest video_ChromeHWDecodeUsed verifies Chrome used the HW
> decoding when playing a video stream by checking the histogram value,
> which is reported from GpuVideoDecoder.
> In order to keep the test pass during switching from GpuVideoDecoder
> to MojoVideoDecoder, we have to report the same histogram value
> temporarily from MojoVideoDecoder.
> 
> With this CL, we can enable MojoVideoDecoder at ChromeOS platform.
> 
> BUG=chromium:902968
> TEST=Pass video_ChromeHWDecodeUsed autotest on Eve
> TEST=Play video at Youtube on Eve and confirm the successfull option
>      of Media.GpuVideoDecoderInitializeStatus are increased.
> 
> Change-Id: I26fc53704843e9b153bbee304e8f60ab3a7b0417
> Reviewed-on: https://chromium-review.googlesource.com/c/1328634
> Commit-Queue: Chih-Yu Huang <akahuang@chromium.org>
> Reviewed-by: Dan Sanders <sandersd@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#607890}

TBR=posciak@chromium.org,sandersd@chromium.org,akahuang@chromium.org,hiroh@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: chromium:902968
Change-Id: I3b74c752b6cc34dcbcba203eeb39707f4921d405
Reviewed-on: https://chromium-review.googlesource.com/c/1340378Reviewed-by: default avatarScott Violet <sky@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608874}
parent 9d0b9fe7
...@@ -260,8 +260,16 @@ const base::Feature kMemoryPressureBasedSourceBufferGC{ ...@@ -260,8 +260,16 @@ const base::Feature kMemoryPressureBasedSourceBufferGC{
"MemoryPressureBasedSourceBufferGC", base::FEATURE_DISABLED_BY_DEFAULT}; "MemoryPressureBasedSourceBufferGC", base::FEATURE_DISABLED_BY_DEFAULT};
// Enable MojoVideoDecoder, replacing GpuVideoDecoder. // Enable MojoVideoDecoder, replacing GpuVideoDecoder.
const base::Feature kMojoVideoDecoder{"MojoVideoDecoder", const base::Feature kMojoVideoDecoder {
base::FEATURE_ENABLED_BY_DEFAULT}; "MojoVideoDecoder",
#if defined(OS_CHROMEOS)
// TODO(posciak): Re-enable once the feature is verified on CrOS.
// https://crbug.com/902968.
base::FEATURE_DISABLED_BY_DEFAULT
#else
base::FEATURE_ENABLED_BY_DEFAULT
#endif
};
// Enable The D3D11 Video decoder. Must also enable MojoVideoDecoder for // Enable The D3D11 Video decoder. Must also enable MojoVideoDecoder for
// this to have any effect. // this to have any effect.
......
...@@ -12,7 +12,6 @@ ...@@ -12,7 +12,6 @@
#include "base/logging.h" #include "base/logging.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/scoped_refptr.h" #include "base/memory/scoped_refptr.h"
#include "base/metrics/histogram_macros.h"
#include "base/single_thread_task_runner.h" #include "base/single_thread_task_runner.h"
#include "base/unguessable_token.h" #include "base/unguessable_token.h"
#include "build/build_config.h" #include "build/build_config.h"
...@@ -30,37 +29,6 @@ ...@@ -30,37 +29,6 @@
#include "mojo/public/cpp/bindings/interface_request.h" #include "mojo/public/cpp/bindings/interface_request.h"
namespace media { namespace media {
namespace {
void ReportMojoVideoDecoderInitializeStatusToUMAAndRunCB(
const VideoDecoder::InitCB& init_cb,
bool success) {
// Send the same histogram as GpuVideoDecoder to avoid breaking the existing
// tests.
// TODO(crbug.com/902968): Remove it after deprecating GpuVideoDecoder.
PipelineStatus status = success ? PIPELINE_OK : DECODER_ERROR_NOT_SUPPORTED;
UMA_HISTOGRAM_ENUMERATION("Media.GpuVideoDecoderInitializeStatus", status,
PIPELINE_STATUS_MAX + 1);
init_cb.Run(success);
}
void ReportMojoVideoDecoderErrorStatusToUMAAndRunCB(
const VideoDecoder::DecodeCB& decode_cb,
DecodeStatus status) {
// Send the same histogram as GpuVideoDecoder to avoid breaking the existing
// tests.
// TODO(crbug.com/902968): Remove it after deprecating GpuVideoDecoder.
if (status == DecodeStatus::DECODE_ERROR) {
UMA_HISTOGRAM_ENUMERATION("Media.GpuVideoDecoderError",
media::VideoDecodeAccelerator::PLATFORM_FAILURE,
media::VideoDecodeAccelerator::ERROR_MAX + 1);
}
decode_cb.Run(status);
}
} // namespace
// Provides a thread-safe channel for VideoFrame destruction events. // Provides a thread-safe channel for VideoFrame destruction events.
class MojoVideoFrameHandleReleaser class MojoVideoFrameHandleReleaser
...@@ -153,16 +121,12 @@ void MojoVideoDecoder::Initialize( ...@@ -153,16 +121,12 @@ void MojoVideoDecoder::Initialize(
DVLOG(1) << __func__; DVLOG(1) << __func__;
DCHECK(task_runner_->BelongsToCurrentThread()); DCHECK(task_runner_->BelongsToCurrentThread());
InitCB bound_init_cb =
base::Bind(&ReportMojoVideoDecoderInitializeStatusToUMAAndRunCB, init_cb);
// Fail immediately if we know that the remote side cannot support |config|. // Fail immediately if we know that the remote side cannot support |config|.
if (gpu_factories_ && !gpu_factories_->IsDecoderConfigSupported(config)) { if (gpu_factories_ && !gpu_factories_->IsDecoderConfigSupported(config)) {
// TODO(liberato): Remove bypass once D3D11VideoDecoder provides // TODO(liberato): Remove bypass once D3D11VideoDecoder provides
// SupportedVideoDecoderConfigs. // SupportedVideoDecoderConfigs.
if (!base::FeatureList::IsEnabled(kD3D11VideoDecoder)) { if (!base::FeatureList::IsEnabled(kD3D11VideoDecoder)) {
task_runner_->PostTask(FROM_HERE, task_runner_->PostTask(FROM_HERE, base::BindRepeating(init_cb, false));
base::BindRepeating(bound_init_cb, false));
return; return;
} }
} }
...@@ -177,7 +141,7 @@ void MojoVideoDecoder::Initialize( ...@@ -177,7 +141,7 @@ void MojoVideoDecoder::Initialize(
// is passed for reinitialization. // is passed for reinitialization.
if (config.is_encrypted() && CdmContext::kInvalidCdmId == cdm_id) { if (config.is_encrypted() && CdmContext::kInvalidCdmId == cdm_id) {
DVLOG(1) << __func__ << ": Invalid CdmContext."; DVLOG(1) << __func__ << ": Invalid CdmContext.";
task_runner_->PostTask(FROM_HERE, base::BindOnce(bound_init_cb, false)); task_runner_->PostTask(FROM_HERE, base::BindOnce(init_cb, false));
return; return;
} }
...@@ -185,13 +149,12 @@ void MojoVideoDecoder::Initialize( ...@@ -185,13 +149,12 @@ void MojoVideoDecoder::Initialize(
BindRemoteDecoder(); BindRemoteDecoder();
if (has_connection_error_) { if (has_connection_error_) {
task_runner_->PostTask(FROM_HERE, task_runner_->PostTask(FROM_HERE, base::BindRepeating(init_cb, false));
base::BindRepeating(bound_init_cb, false));
return; return;
} }
initialized_ = false; initialized_ = false;
init_cb_ = bound_init_cb; init_cb_ = init_cb;
output_cb_ = output_cb; output_cb_ = output_cb;
remote_decoder_->Initialize( remote_decoder_->Initialize(
config, low_delay, cdm_id, config, low_delay, cdm_id,
...@@ -214,25 +177,22 @@ void MojoVideoDecoder::Decode(scoped_refptr<DecoderBuffer> buffer, ...@@ -214,25 +177,22 @@ void MojoVideoDecoder::Decode(scoped_refptr<DecoderBuffer> buffer,
DVLOG(3) << __func__ << ": " << buffer->AsHumanReadableString(); DVLOG(3) << __func__ << ": " << buffer->AsHumanReadableString();
DCHECK(task_runner_->BelongsToCurrentThread()); DCHECK(task_runner_->BelongsToCurrentThread());
DecodeCB bound_decode_cb =
base::Bind(&ReportMojoVideoDecoderErrorStatusToUMAAndRunCB, decode_cb);
if (has_connection_error_) { if (has_connection_error_) {
task_runner_->PostTask( task_runner_->PostTask(FROM_HERE,
FROM_HERE, base::Bind(bound_decode_cb, DecodeStatus::DECODE_ERROR)); base::Bind(decode_cb, DecodeStatus::DECODE_ERROR));
return; return;
} }
mojom::DecoderBufferPtr mojo_buffer = mojom::DecoderBufferPtr mojo_buffer =
mojo_decoder_buffer_writer_->WriteDecoderBuffer(std::move(buffer)); mojo_decoder_buffer_writer_->WriteDecoderBuffer(std::move(buffer));
if (!mojo_buffer) { if (!mojo_buffer) {
task_runner_->PostTask( task_runner_->PostTask(FROM_HERE,
FROM_HERE, base::Bind(bound_decode_cb, DecodeStatus::DECODE_ERROR)); base::Bind(decode_cb, DecodeStatus::DECODE_ERROR));
return; return;
} }
uint64_t decode_id = decode_counter_++; uint64_t decode_id = decode_counter_++;
pending_decodes_[decode_id] = bound_decode_cb; pending_decodes_[decode_id] = decode_cb;
remote_decoder_->Decode(std::move(mojo_buffer), remote_decoder_->Decode(std::move(mojo_buffer),
base::Bind(&MojoVideoDecoder::OnDecodeDone, base::Bind(&MojoVideoDecoder::OnDecodeDone,
base::Unretained(this), decode_id)); base::Unretained(this), decode_id));
......
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