Skip to content

Commit

Permalink
Migrate MediaMetricsProvider to new Mojo types
Browse files Browse the repository at this point in the history
This CL applies pending_receiver to AcquireWatchTimeRecorder
and AcquireVideoDecodeStatsRecorder in MediaMetricsProvider
interface.

  - Convert FooPtr to mojo::PendingRemote or mojo::Remote.
  - Convert FooRequest to mojo::PendingReceiver.
  - Convert MakeStrongBinding to MakeSelfOwnedReceiver.

Bug: 955171
Change-Id: Ife29eb0e123324305d316b1ec0b9ec3754488e41
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1885851
Reviewed-by: Chrome Cunningham <chcunningham@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Sam McNally <sammc@chromium.org>
Commit-Queue: Gyuyoung Kim <gyuyoung@igalia.com>
Cr-Commit-Position: refs/heads/master@{#710570}
  • Loading branch information
Gyuyoung authored and Commit Bot committed Oct 30, 2019
1 parent fe29640 commit a040bc5
Show file tree
Hide file tree
Showing 8 changed files with 36 additions and 34 deletions.
12 changes: 6 additions & 6 deletions media/blink/video_decode_stats_reporter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
namespace media {

VideoDecodeStatsReporter::VideoDecodeStatsReporter(
mojom::VideoDecodeStatsRecorderPtr recorder_ptr,
mojo::PendingRemote<mojom::VideoDecodeStatsRecorder> recorder_remote,
GetPipelineStatsCB get_pipeline_stats_cb,
VideoCodecProfile codec_profile,
const gfx::Size& natural_size,
Expand All @@ -28,7 +28,7 @@ VideoDecodeStatsReporter::VideoDecodeStatsReporter(
base::TimeDelta::FromMilliseconds(kRecordingIntervalMs)),
kTinyFpsWindowDuration(
base::TimeDelta::FromMilliseconds(kTinyFpsWindowMs)),
recorder_ptr_(std::move(recorder_ptr)),
recorder_remote_(std::move(recorder_remote)),
get_pipeline_stats_cb_(std::move(get_pipeline_stats_cb)),
codec_profile_(codec_profile),
natural_size_(GetSizeBucket(natural_size)),
Expand All @@ -37,12 +37,12 @@ VideoDecodeStatsReporter::VideoDecodeStatsReporter(
: false),
tick_clock_(tick_clock),
stats_cb_timer_(tick_clock_) {
DCHECK(recorder_ptr_.is_bound());
DCHECK(recorder_remote_.is_bound());
DCHECK(get_pipeline_stats_cb_);
DCHECK_NE(VIDEO_CODEC_PROFILE_UNKNOWN, codec_profile_);
DCHECK(!cdm_config || !key_system_.empty());

recorder_ptr_.set_connection_error_handler(base::BindRepeating(
recorder_remote_.set_disconnect_handler(base::BindRepeating(
&VideoDecodeStatsReporter::OnIpcConnectionError, base::Unretained(this)));
stats_cb_timer_.SetTaskRunner(task_runner);
}
Expand Down Expand Up @@ -157,7 +157,7 @@ void VideoDecodeStatsReporter::StartNewRecord(
codec_profile_, natural_size_, last_observed_fps_, key_system_,
use_hw_secure_codecs);

recorder_ptr_->StartNewRecord(std::move(features));
recorder_remote_->StartNewRecord(std::move(features));
}

void VideoDecodeStatsReporter::ResetFrameRateState() {
Expand Down Expand Up @@ -327,7 +327,7 @@ void VideoDecodeStatsReporter::UpdateStats() {
<< "/" << targets->frames_decoded
<< " power efficient:" << targets->frames_power_efficient << "/"
<< targets->frames_decoded;
recorder_ptr_->UpdateRecord(std::move(targets));
recorder_remote_->UpdateRecord(std::move(targets));
}

} // namespace media
6 changes: 4 additions & 2 deletions media/blink/video_decode_stats_reporter.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
#include "media/base/video_codecs.h"
#include "media/blink/media_blink_export.h"
#include "media/mojo/mojom/video_decode_stats_recorder.mojom.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/remote.h"

namespace media {

Expand All @@ -33,7 +35,7 @@ class MEDIA_BLINK_EXPORT VideoDecodeStatsReporter {
using GetPipelineStatsCB = base::Callback<PipelineStatistics(void)>;

VideoDecodeStatsReporter(
mojom::VideoDecodeStatsRecorderPtr recorder_ptr,
mojo::PendingRemote<mojom::VideoDecodeStatsRecorder> recorder_remote,
GetPipelineStatsCB get_pipeline_stats_cb,
VideoCodecProfile codec_profile,
const gfx::Size& natural_size,
Expand Down Expand Up @@ -146,7 +148,7 @@ class MEDIA_BLINK_EXPORT VideoDecodeStatsReporter {

// Pointer to the remote recorder. The recorder runs in the browser process
// and finalizes the record in the event of fast render process tear down.
mojom::VideoDecodeStatsRecorderPtr recorder_ptr_;
mojo::Remote<mojom::VideoDecodeStatsRecorder> recorder_remote_;

// Callback for retrieving playback statistics.
GetPipelineStatsCB get_pipeline_stats_cb_;
Expand Down
32 changes: 15 additions & 17 deletions media/blink/video_decode_stats_reporter_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@
#include "media/capabilities/bucket_utility.h"
#include "media/mojo/mojom/media_types.mojom.h"
#include "media/mojo/mojom/video_decode_stats_recorder.mojom.h"
#include "mojo/public/cpp/bindings/strong_binding.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/self_owned_receiver.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "ui/gfx/geometry/rect.h"
Expand Down Expand Up @@ -163,21 +164,21 @@ class VideoDecodeStatsReporterTest : public ::testing::Test {
SLOW_STABILIZE_FPS,
};

// Bind the RecordInterceptor to the request for a VideoDecodeStatsRecorder.
// The interceptor serves as a mock recorder to verify reporter/recorder
// interactions.
void SetupRecordInterceptor(mojom::VideoDecodeStatsRecorderPtr* recorder_ptr,
RecordInterceptor** interceptor) {
// Return mojo::PendingRemote<mojom::VideoDecodeStatsRecorder>
// after binding the RecordInterceptor to the receiver for a
// VideoDecodeStatsRecorder. The interceptor serves as a mock recorder to
// verify reporter/recorder interactions.
mojo::PendingRemote<mojom::VideoDecodeStatsRecorder> SetupRecordInterceptor(
RecordInterceptor** interceptor) {
// Capture a the interceptor pointer for verifying recorder calls. Lifetime
// will be managed by the |recorder_ptr|.
*interceptor = new RecordInterceptor();

// Bind interceptor as the VideoDecodeStatsRecorder.
mojom::VideoDecodeStatsRecorderRequest request =
mojo::MakeRequest(recorder_ptr);
mojo::MakeStrongBinding(base::WrapUnique(*interceptor),
mojo::MakeRequest(recorder_ptr));
EXPECT_TRUE(recorder_ptr->is_bound());
mojo::PendingRemote<mojom::VideoDecodeStatsRecorder> recorder_remote;
mojo::MakeSelfOwnedReceiver(
base::WrapUnique(*interceptor),
recorder_remote.InitWithNewPipeAndPassReceiver());
EXPECT_TRUE(recorder_remote.is_valid());
return recorder_remote;
}

// Inject mock objects and create a new |reporter_| to test.
Expand All @@ -186,11 +187,8 @@ class VideoDecodeStatsReporterTest : public ::testing::Test {
const gfx::Size& natural_size = gfx::Size(kDefaultWidth, kDefaultHeight),
const std::string key_system = kDefaultKeySystem,
const base::Optional<CdmConfig> cdm_config = kDefaultCdmConfig) {
mojom::VideoDecodeStatsRecorderPtr recorder_ptr;
SetupRecordInterceptor(&recorder_ptr, &interceptor_);

reporter_ = std::make_unique<VideoDecodeStatsReporter>(
std::move(recorder_ptr),
SetupRecordInterceptor(&interceptor_),
base::Bind(&VideoDecodeStatsReporterTest::GetPipelineStatsCB,
base::Unretained(this)),
profile, natural_size, key_system, cdm_config, task_runner_,
Expand Down
3 changes: 2 additions & 1 deletion media/blink/watch_time_reporter_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,8 @@ class WatchTimeReporterTest
std::make_unique<WatchTimeInterceptor>(parent_), std::move(receiver));
}
void AcquireVideoDecodeStatsRecorder(
mojom::VideoDecodeStatsRecorderRequest request) override {
mojo::PendingReceiver<mojom::VideoDecodeStatsRecorder> receiver)
override {
FAIL();
}
void AcquireLearningTaskController(
Expand Down
5 changes: 3 additions & 2 deletions media/blink/webmediaplayer_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
#include "media/filters/ffmpeg_demuxer.h"
#include "media/filters/memory_data_source.h"
#include "media/media_buildflags.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "net/base/data_url.h"
#include "third_party/blink/public/platform/web_encrypted_media_types.h"
#include "third_party/blink/public/platform/web_media_player_client.h"
Expand Down Expand Up @@ -1977,9 +1978,9 @@ void WebMediaPlayerImpl::CreateVideoDecodeStatsReporter() {
DCHECK(!key_system_.empty());
}

mojom::VideoDecodeStatsRecorderPtr recorder;
mojo::PendingRemote<mojom::VideoDecodeStatsRecorder> recorder;
media_metrics_provider_->AcquireVideoDecodeStatsRecorder(
mojo::MakeRequest(&recorder));
recorder.InitWithNewPipeAndPassReceiver());

// Create capabilities reporter and synchronize its initial state.
video_decode_stats_reporter_.reset(new VideoDecodeStatsReporter(
Expand Down
3 changes: 2 additions & 1 deletion media/mojo/mojom/media_metrics_provider.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ interface MediaMetricsProvider {
pending_receiver<WatchTimeRecorder> recorder);

// Creates a VideoDecodeStatsRecorder instance.
AcquireVideoDecodeStatsRecorder(VideoDecodeStatsRecorder& recorder);
AcquireVideoDecodeStatsRecorder(
pending_receiver<VideoDecodeStatsRecorder> recorder);

// Returns a LearningTaskController for the given |taskName|.
AcquireLearningTaskController(
Expand Down
7 changes: 3 additions & 4 deletions media/mojo/services/media_metrics_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
#include "media/mojo/services/video_decode_stats_recorder.h"
#include "media/mojo/services/watch_time_recorder.h"
#include "mojo/public/cpp/bindings/self_owned_receiver.h"
#include "mojo/public/cpp/bindings/strong_binding.h"
#include "services/metrics/public/cpp/ukm_builders.h"
#include "services/metrics/public/cpp/ukm_recorder.h"

Expand Down Expand Up @@ -268,7 +267,7 @@ void MediaMetricsProvider::AcquireWatchTimeRecorder(
}

void MediaMetricsProvider::AcquireVideoDecodeStatsRecorder(
mojom::VideoDecodeStatsRecorderRequest request) {
mojo::PendingReceiver<mojom::VideoDecodeStatsRecorder> receiver) {
if (!initialized_) {
mojo::ReportBadMessage(kInvalidInitialize);
return;
Expand All @@ -279,10 +278,10 @@ void MediaMetricsProvider::AcquireVideoDecodeStatsRecorder(
return;
}

mojo::MakeStrongBinding(
mojo::MakeSelfOwnedReceiver(
std::make_unique<VideoDecodeStatsRecorder>(save_cb_, source_id_, origin_,
is_top_frame_, player_id_),
std::move(request));
std::move(receiver));
}

void MediaMetricsProvider::AcquireLearningTaskController(
Expand Down
2 changes: 1 addition & 1 deletion media/mojo/services/media_metrics_provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ class MEDIA_MOJO_EXPORT MediaMetricsProvider
mojom::PlaybackPropertiesPtr properties,
mojo::PendingReceiver<mojom::WatchTimeRecorder> receiver) override;
void AcquireVideoDecodeStatsRecorder(
mojom::VideoDecodeStatsRecorderRequest request) override;
mojo::PendingReceiver<mojom::VideoDecodeStatsRecorder> receiver) override;
void AcquireLearningTaskController(
const std::string& taskName,
mojo::PendingReceiver<media::learning::mojom::LearningTaskController>
Expand Down

0 comments on commit a040bc5

Please sign in to comment.