From e04d0fa1b280f56796782b2bea6afcb928c06173 Mon Sep 17 00:00:00 2001 From: Brett Hebert Date: Tue, 9 Aug 2022 18:25:04 +0000 Subject: [PATCH] Fix Event Log For Video Receiver MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves an issue where, in Chrome, WebRTC event logs do not capture outgoing packets for video receivers because no reference to the event log was passed to the video receiver. Bug: webrtc:14338 Change-Id: Ia33ce6f2d69a0e341530648b10a08516dc53abf3 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/271080 Reviewed-by: Magnus Flodman Reviewed-by: Björn Terelius Commit-Queue: Björn Terelius Cr-Commit-Position: refs/heads/main@{#37746} --- AUTHORS | 1 + call/call.cc | 2 +- video/rtp_video_stream_receiver2.cc | 10 +++++++--- video/rtp_video_stream_receiver2.h | 3 ++- video/rtp_video_stream_receiver2_unittest.cc | 4 ++-- video/video_receive_stream2.cc | 6 ++++-- video/video_receive_stream2.h | 3 ++- video/video_receive_stream2_unittest.cc | 2 +- 8 files changed, 20 insertions(+), 11 deletions(-) diff --git a/AUTHORS b/AUTHORS index 8f1df31929..831ee0dc25 100644 --- a/AUTHORS +++ b/AUTHORS @@ -24,6 +24,7 @@ Anil Kumar Ben Strong Berthold Herrmann Bob Withers +Brett Hebert Bridger Maxwell Bruno Pitrus Cheng Qian diff --git a/call/call.cc b/call/call.cc index 09bc902370..959d028241 100644 --- a/call/call.cc +++ b/call/call.cc @@ -1042,7 +1042,7 @@ webrtc::VideoReceiveStreamInterface* Call::CreateVideoReceiveStream( task_queue_factory_, this, num_cpu_cores_, transport_send_->packet_router(), std::move(configuration), call_stats_.get(), clock_, std::make_unique(clock_, trials()), - &nack_periodic_processor_, decode_sync_.get()); + &nack_periodic_processor_, decode_sync_.get(), event_log_); // TODO(bugs.webrtc.org/11993): Set this up asynchronously on the network // thread. receive_stream->RegisterWithTransport(&video_receiver_controller_); diff --git a/video/rtp_video_stream_receiver2.cc b/video/rtp_video_stream_receiver2.cc index 284a50185e..ac12224c75 100644 --- a/video/rtp_video_stream_receiver2.cc +++ b/video/rtp_video_stream_receiver2.cc @@ -83,7 +83,8 @@ std::unique_ptr CreateRtpRtcpModule( RtcpPacketTypeCounterObserver* rtcp_packet_type_counter_observer, RtcpCnameCallback* rtcp_cname_callback, bool non_sender_rtt_measurement, - uint32_t local_ssrc) { + uint32_t local_ssrc, + RtcEventLog* rtc_event_log) { RtpRtcpInterface::Configuration configuration; configuration.clock = clock; configuration.audio = false; @@ -96,6 +97,7 @@ std::unique_ptr CreateRtpRtcpModule( configuration.rtcp_cname_callback = rtcp_cname_callback; configuration.local_media_ssrc = local_ssrc; configuration.non_sender_rtt_measurement = non_sender_rtt_measurement; + configuration.event_log = rtc_event_log; std::unique_ptr rtp_rtcp = ModuleRtpRtcpImpl2::Create(configuration); @@ -223,7 +225,8 @@ RtpVideoStreamReceiver2::RtpVideoStreamReceiver2( OnCompleteFrameCallback* complete_frame_callback, rtc::scoped_refptr frame_decryptor, rtc::scoped_refptr frame_transformer, - const FieldTrialsView& field_trials) + const FieldTrialsView& field_trials, + RtcEventLog* event_log) : field_trials_(field_trials), worker_queue_(current_queue), clock_(clock), @@ -251,7 +254,8 @@ RtpVideoStreamReceiver2::RtpVideoStreamReceiver2( rtcp_packet_type_counter_observer, rtcp_cname_callback, config_.rtp.rtcp_xr.receiver_reference_time_report, - config_.rtp.local_ssrc)), + config_.rtp.local_ssrc, + event_log)), nack_periodic_processor_(nack_periodic_processor), complete_frame_callback_(complete_frame_callback), keyframe_request_method_(config_.rtp.keyframe_method), diff --git a/video/rtp_video_stream_receiver2.h b/video/rtp_video_stream_receiver2.h index 73ed80e0ed..c341536d3f 100644 --- a/video/rtp_video_stream_receiver2.h +++ b/video/rtp_video_stream_receiver2.h @@ -96,7 +96,8 @@ class RtpVideoStreamReceiver2 : public LossNotificationSender, OnCompleteFrameCallback* complete_frame_callback, rtc::scoped_refptr frame_decryptor, rtc::scoped_refptr frame_transformer, - const FieldTrialsView& field_trials); + const FieldTrialsView& field_trials, + RtcEventLog* event_log); ~RtpVideoStreamReceiver2() override; void AddReceiveCodec(uint8_t payload_type, diff --git a/video/rtp_video_stream_receiver2_unittest.cc b/video/rtp_video_stream_receiver2_unittest.cc index 4eb0406db6..325188e93c 100644 --- a/video/rtp_video_stream_receiver2_unittest.cc +++ b/video/rtp_video_stream_receiver2_unittest.cc @@ -161,7 +161,7 @@ class RtpVideoStreamReceiver2Test : public ::testing::Test, TaskQueueBase::Current(), Clock::GetRealTimeClock(), &mock_transport_, nullptr, nullptr, &config_, rtp_receive_statistics_.get(), nullptr, nullptr, &nack_periodic_processor_, &mock_on_complete_frame_callback_, - nullptr, nullptr, field_trials_); + nullptr, nullptr, field_trials_, nullptr); rtp_video_stream_receiver_->AddReceiveCodec(kPayloadType, kVideoCodecGeneric, {}, /*raw_payload=*/false); @@ -1143,7 +1143,7 @@ TEST_F(RtpVideoStreamReceiver2Test, TransformFrame) { TaskQueueBase::Current(), Clock::GetRealTimeClock(), &mock_transport_, nullptr, nullptr, &config_, rtp_receive_statistics_.get(), nullptr, nullptr, &nack_periodic_processor_, &mock_on_complete_frame_callback_, - nullptr, mock_frame_transformer, field_trials_); + nullptr, mock_frame_transformer, field_trials_, nullptr); receiver->AddReceiveCodec(kPayloadType, kVideoCodecGeneric, {}, /*raw_payload=*/false); diff --git a/video/video_receive_stream2.cc b/video/video_receive_stream2.cc index 84a9bb3543..413c45a8eb 100644 --- a/video/video_receive_stream2.cc +++ b/video/video_receive_stream2.cc @@ -200,7 +200,8 @@ VideoReceiveStream2::VideoReceiveStream2( Clock* clock, std::unique_ptr timing, NackPeriodicProcessor* nack_periodic_processor, - DecodeSynchronizer* decode_sync) + DecodeSynchronizer* decode_sync, + RtcEventLog* event_log) : task_queue_factory_(task_queue_factory), transport_adapter_(config.rtcp_send_transport), config_(std::move(config)), @@ -226,7 +227,8 @@ VideoReceiveStream2::VideoReceiveStream2( this, // OnCompleteFrameCallback std::move(config_.frame_decryptor), std::move(config_.frame_transformer), - call->trials()), + call->trials(), + event_log), rtp_stream_sync_(call->worker_thread(), this), max_wait_for_keyframe_(DetermineMaxWaitForFrame( TimeDelta::Millis(config_.rtp.nack.rtp_history_ms), diff --git a/video/video_receive_stream2.h b/video/video_receive_stream2.h index 6410434a36..39ed2c3a68 100644 --- a/video/video_receive_stream2.h +++ b/video/video_receive_stream2.h @@ -102,7 +102,8 @@ class VideoReceiveStream2 Clock* clock, std::unique_ptr timing, NackPeriodicProcessor* nack_periodic_processor, - DecodeSynchronizer* decode_sync); + DecodeSynchronizer* decode_sync, + RtcEventLog* event_log); // Destruction happens on the worker thread. Prior to destruction the caller // must ensure that a registration with the transport has been cleared. See // `RegisterWithTransport` for details. diff --git a/video/video_receive_stream2_unittest.cc b/video/video_receive_stream2_unittest.cc index d8dcb18b02..f6b7df33fd 100644 --- a/video/video_receive_stream2_unittest.cc +++ b/video/video_receive_stream2_unittest.cc @@ -266,7 +266,7 @@ class VideoReceiveStream2Test : public ::testing::TestWithParam { time_controller_.GetTaskQueueFactory(), &fake_call_, kDefaultNumCpuCores, &packet_router_, config_.Copy(), &call_stats_, clock_, absl::WrapUnique(timing_), &nack_periodic_processor_, - GetParam() ? &decode_sync_ : nullptr); + GetParam() ? &decode_sync_ : nullptr, nullptr); video_receive_stream_->RegisterWithTransport( &rtp_stream_receiver_controller_); if (state)