Skip to content

Commit

Permalink
media/mojo/VideoEncoderMetricsProvider: Add encoder id
Browse files Browse the repository at this point in the history
Only a javascript main thread can establish mojo connection with a
browser process with a URL key. In the webrtc scenario, however, a
thread of creating a video encoder is different from a javascript
main thread.
This CL adds the encoder id to the VideoEncoderMetricsProvider mojo
calls so that one VideoEncoderMetricsProvider can handle multiple
video encoders simultaneously. With it, we enable deal with the
webrtc case by passing the class of owning VideoEncoderMetricsProvider
to the webrtc encoders.

Bug: b:275663480
Test: chrome://ukm in Cast and MediaRecorder
Change-Id: I0b07b9575798e5e1ebc5052c8f9e369bfb8c9a6c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4729246
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: Mark Foltz <mfoltz@chromium.org>
Commit-Queue: Hirokazu Honda <hiroh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1187007}
  • Loading branch information
Hirokazu Honda authored and Chromium LUCI CQ committed Aug 23, 2023
1 parent 19e20fb commit a5d3e6d
Show file tree
Hide file tree
Showing 6 changed files with 376 additions and 135 deletions.
25 changes: 19 additions & 6 deletions media/mojo/clients/mojo_video_encoder_metrics_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,17 @@ class MojoVideoEncoderMetricsProvider : public VideoEncoderMetricsProvider {
public:
MojoVideoEncoderMetricsProvider(
mojom::VideoEncoderUseCase use_case,
mojo::PendingRemote<mojom::VideoEncoderMetricsProvider> pending_remote)
: use_case_(use_case), pending_remote_(std::move(pending_remote)) {
mojo::PendingRemote<mojom::VideoEncoderMetricsProvider> pending_remote,
uint64_t encoder_id)
: use_case_(use_case),
encoder_id_(encoder_id),
pending_remote_(std::move(pending_remote)) {
DETACH_FROM_SEQUENCE(sequence_checker_);
}

~MojoVideoEncoderMetricsProvider() override {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
Complete();
}

void Initialize(VideoCodecProfile codec_profile,
Expand All @@ -36,7 +40,7 @@ class MojoVideoEncoderMetricsProvider : public VideoEncoderMetricsProvider {
}
initialized_ = true;
num_encoded_frames_ = 0;
remote_->Initialize(use_case_, codec_profile, encode_size,
remote_->Initialize(encoder_id_, use_case_, codec_profile, encode_size,
is_hardware_encoder, svc_mode);
}

Expand All @@ -53,7 +57,7 @@ class MojoVideoEncoderMetricsProvider : public VideoEncoderMetricsProvider {
// as it is important to represent whether the encoding actually starts.
if (num_encoded_frames_ % kEncodedFrameCountBucketSize == 0 ||
num_encoded_frames_ == 1u) {
remote_->SetEncodedFrameCount(num_encoded_frames_);
remote_->SetEncodedFrameCount(encoder_id_, num_encoded_frames_);
}
}

Expand All @@ -64,12 +68,21 @@ class MojoVideoEncoderMetricsProvider : public VideoEncoderMetricsProvider {
return;
}
CHECK(!status.is_ok());
remote_->SetError(status);
remote_->SetError(encoder_id_, status);
}

private:
void Complete() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (initialized_) {
remote_->Complete(encoder_id_);
}
remote_.reset();
}

const mojom::VideoEncoderUseCase use_case_
GUARDED_BY_CONTEXT(sequence_checker_);
const uint64_t encoder_id_ GUARDED_BY_CONTEXT(sequence_checker_);

mojo::PendingRemote<mojom::VideoEncoderMetricsProvider> pending_remote_;
mojo::Remote<mojom::VideoEncoderMetricsProvider> remote_
Expand All @@ -87,6 +100,6 @@ CreateMojoVideoEncoderMetricsProvider(
mojom::VideoEncoderUseCase use_case,
mojo::PendingRemote<mojom::VideoEncoderMetricsProvider> pending_remote) {
return std::make_unique<MojoVideoEncoderMetricsProvider>(
use_case, std::move(pending_remote));
use_case, std::move(pending_remote), /*encoder_id=*/0u);
}
} // namespace media
31 changes: 20 additions & 11 deletions media/mojo/clients/mojo_video_encoder_metrics_provider_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ using ::testing::InSequence;

namespace media {

constexpr uint64_t kEncoderId = 0u;

class MockMojomVideoEncoderMetricsProvider
: public mojom::VideoEncoderMetricsProvider {
public:
Expand All @@ -24,14 +26,19 @@ class MockMojomVideoEncoderMetricsProvider
// mojom::VideoEncoderMetricsProvider implementation.
MOCK_METHOD(void,
Initialize,
(mojom::VideoEncoderUseCase,
(uint64_t,
mojom::VideoEncoderUseCase,
VideoCodecProfile,
const gfx::Size&,
bool,
SVCScalabilityMode),
(override));
MOCK_METHOD(void, SetEncodedFrameCount, (uint64_t), (override));
MOCK_METHOD(void, SetError, (const media::EncoderStatus&), (override));
MOCK_METHOD(void, SetEncodedFrameCount, (uint64_t, uint64_t), (override));
MOCK_METHOD(void,
SetError,
(uint64_t, const media::EncoderStatus&),
(override));
MOCK_METHOD(void, Complete, (uint64_t), (override));
};

class MojoVideoEncoderMetricsProviderTest : public ::testing::Test {
Expand Down Expand Up @@ -83,10 +90,12 @@ TEST_F(MojoVideoEncoderMetricsProviderTest, CreateAndBoundAndInitialize) {

InSequence s;
EXPECT_CALL(*mock_mojo_receiver(),
Initialize(kUseCase, kCodecProfile, kEncodeSize,
Initialize(kEncoderId, kUseCase, kCodecProfile, kEncodeSize,
kIsHardwareEncoder, kSVCMode));
mojo_encoder_metrics_provider_->Initialize(kCodecProfile, kEncodeSize,
kIsHardwareEncoder, kSVCMode);
EXPECT_CALL(*mock_mojo_receiver(), Complete(kEncoderId));
mojo_encoder_metrics_provider_.reset();
base::RunLoop().RunUntilIdle();
}

Expand All @@ -99,26 +108,26 @@ TEST_F(MojoVideoEncoderMetricsProviderTest,

InSequence s;
EXPECT_CALL(*mock_mojo_receiver(),
Initialize(kUseCase, kCodecProfile, kEncodeSize,
Initialize(kEncoderId, kUseCase, kCodecProfile, kEncodeSize,
kIsHardwareEncoder, kSVCMode));
mojo_encoder_metrics_provider_->Initialize(kCodecProfile, kEncodeSize,
kIsHardwareEncoder, kSVCMode);
base::RunLoop().RunUntilIdle();
EXPECT_CALL(*mock_mojo_receiver(), SetEncodedFrameCount(1u));
EXPECT_CALL(*mock_mojo_receiver(), SetEncodedFrameCount(kEncoderId, 1u));
mojo_encoder_metrics_provider_->IncrementEncodedFrameCount();
base::RunLoop().RunUntilIdle();
for (size_t i = 0; i < 98; i++) {
mojo_encoder_metrics_provider_->IncrementEncodedFrameCount();
}
base::RunLoop().RunUntilIdle();
EXPECT_CALL(*mock_mojo_receiver(), SetEncodedFrameCount(100u));
EXPECT_CALL(*mock_mojo_receiver(), SetEncodedFrameCount(kEncoderId, 100u));
mojo_encoder_metrics_provider_->IncrementEncodedFrameCount();
base::RunLoop().RunUntilIdle();
for (size_t i = 0; i < 99; i++) {
mojo_encoder_metrics_provider_->IncrementEncodedFrameCount();
}
base::RunLoop().RunUntilIdle();
EXPECT_CALL(*mock_mojo_receiver(), SetEncodedFrameCount(200u));
EXPECT_CALL(*mock_mojo_receiver(), SetEncodedFrameCount(kEncoderId, 200u));
mojo_encoder_metrics_provider_->IncrementEncodedFrameCount();
base::RunLoop().RunUntilIdle();
}
Expand All @@ -132,20 +141,20 @@ TEST_F(MojoVideoEncoderMetricsProviderTest,

InSequence s;
EXPECT_CALL(*mock_mojo_receiver(),
Initialize(kUseCase, kCodecProfile, kEncodeSize,
Initialize(kEncoderId, kUseCase, kCodecProfile, kEncodeSize,
kIsHardwareEncoder, kSVCMode));
mojo_encoder_metrics_provider_->Initialize(kCodecProfile, kEncodeSize,
kIsHardwareEncoder, kSVCMode);
base::RunLoop().RunUntilIdle();
EXPECT_CALL(*mock_mojo_receiver(), SetEncodedFrameCount(1u));
EXPECT_CALL(*mock_mojo_receiver(), SetEncodedFrameCount(kEncoderId, 1u));
mojo_encoder_metrics_provider_->IncrementEncodedFrameCount();
mojo_encoder_metrics_provider_->IncrementEncodedFrameCount();
mojo_encoder_metrics_provider_->IncrementEncodedFrameCount();
base::RunLoop().RunUntilIdle();
const media::EncoderStatus kErrorStatus(
media::EncoderStatus::Codes::kEncoderFailedEncode, "Encoder failed");
mojo_encoder_metrics_provider_->SetError(kErrorStatus);
EXPECT_CALL(*mock_mojo_receiver(), SetError(kErrorStatus));
EXPECT_CALL(*mock_mojo_receiver(), SetError(kEncoderId, kErrorStatus));
base::RunLoop().RunUntilIdle();
}

Expand Down
20 changes: 14 additions & 6 deletions media/mojo/mojom/video_encoder_metrics_provider.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,28 @@ enum VideoEncoderUseCase {

// Provider interface to collect video encoder metrics.
interface VideoEncoderMetricsProvider {
// Initialize() is called whenever the encoder configuration is changed.
// Initialize() is called whenever the encoder configuration is changed for
// the encoder whose id is |encoder_id|. This must be called for a new encoder
// before other calls. Otherwise, the other functions for the encoder is
// ignored.
// The UKM about the encoding represented by the previous Initialize() is
// reported if SetEncodedFrameCount() or SetError() is invoked. See
// Media.VideoEncoderMetrics in ukm.xml for the detail about the recorded UKM.
Initialize(VideoEncoderUseCase encoder_use_case, VideoCodecProfile profile,
gfx.mojom.Size encode_size, bool is_hardware_encoder,
SVCScalabilityMode svc_mode);
Initialize(uint64 encoder_id, VideoEncoderUseCase encoder_use_case,
VideoCodecProfile profile, gfx.mojom.Size encode_size,
bool is_hardware_encoder, SVCScalabilityMode svc_mode);

// SetEncodedFramesCount() updates the number of successfully encoded frames.
// |num_encoded_frames| can be any value, but it is bucket by 100 when UKM
// is recorded.
SetEncodedFrameCount(uint64 num_encoded_frames);
SetEncodedFrameCount(uint64 encoder_id, uint64 num_encoded_frames);

// SetError() should be called when the encoder becomes in the error state. In
// other words, |status| must not be kOk.
SetError(EncoderStatus status);
SetError(uint64 encoder_id, EncoderStatus status);

// Complete() is called when if the encoder whose id |encoder_id| is no longer
// used. This may not be called if the encoder is destroyed by destroying
// the renderer process (e.g. closing the tab).
Complete(uint64 encoder_id);
};

0 comments on commit a5d3e6d

Please sign in to comment.