admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

WebRTCVideoChannel::OnPacketReceived: avoid PostTasks.

Browse Source 
Under the combined network/worker thread project, tasks
are unnecessarily posted to the same thread. Avoid this
by posting only if invoked on a diffferent sequence.

TESTED=presubmit + local Meet calls.

Bug: webrtc:137439
Change-Id: Ic5dd99e5fbb843ad4c54d4466138135ae81596cf
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/295867
Commit-Queue: Markus Handell <handellm@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39471}
This commit is contained in:
Markus Handell 2023-03-03 12:08:18 +01:00 committed by WebRTC LUCI CQ
parent 764e89c09d
commit 28c4986e1b
3 changed files with 51 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

67
media/engine/webrtc_video_engine.cc
View File

@ -1741,34 +1741,23 @@ void WebRtcVideoChannel::FillReceiveCodecStats(
void WebRtcVideoChannel::OnPacketReceived(
const webrtc::RtpPacketReceived& packet) {
// Note: the network_thread_checker may refer to the worker thread if the two
// threads are combined, but this is either always true or always false
// depending on configuration set at object initialization.
RTC_DCHECK_RUN_ON(&network_thread_checker_);
// TODO(bugs.webrtc.org/11993): This code is very similar to what
// WebRtcVoiceMediaChannel::OnPacketReceived does. For maintainability and
// consistency it would be good to move the interaction with call_->Receiver()
// to a common implementation and provide a callback on the worker thread
// for the exception case (DELIVERY_UNKNOWN_SSRC) and how retry is attempted.
worker_thread_->PostTask(
SafeTask(task_safety_.flag(), [this, packet = packet]() mutable {
RTC_DCHECK_RUN_ON(&thread_checker_);
// TODO(bugs.webrtc.org/7135): extensions in `packet` is currently set
// in RtpTransport and does not neccessarily include extensions specific
// to this channel/MID. Also see comment in
// BaseChannel::MaybeUpdateDemuxerAndRtpExtensions_w.
// It would likely be good if extensions where merged per BUNDLE and
// applied directly in RtpTransport::DemuxPacket;
packet.IdentifyExtensions(recv_rtp_extension_map_);
packet.set_payload_type_frequency(webrtc::kVideoPayloadTypeFrequency);
if (!packet.arrival_time().IsFinite()) {
packet.set_arrival_time(webrtc::Timestamp::Micros(rtc::TimeMicros()));
}
call_->Receiver()->DeliverRtpPacket(
webrtc::MediaType::VIDEO, std::move(packet),
absl::bind_front(
&WebRtcVideoChannel::MaybeCreateDefaultReceiveStream, this));
}));
// TODO(bugs.webrtc.org/137439): Stop posting to the worker thread when the
// combined network/worker project launches.
if (webrtc::TaskQueueBase::Current() != worker_thread_) {
worker_thread_->PostTask(
SafeTask(task_safety_.flag(), [this, packet = packet]() mutable {
RTC_DCHECK_RUN_ON(&thread_checker_);
ProcessReceivedPacket(std::move(packet));
}));
} else {
RTC_DCHECK_RUN_ON(&thread_checker_);
ProcessReceivedPacket(packet);
}
}
bool WebRtcVideoChannel::MaybeCreateDefaultReceiveStream(
@ -3555,6 +3544,32 @@ WebRtcVideoChannel::FindReceiveStream(uint32_t ssrc) {
return nullptr;
}
// RTC_RUN_ON(worker_thread_)
void WebRtcVideoChannel::ProcessReceivedPacket(
webrtc::RtpPacketReceived packet) {
// TODO(bugs.webrtc.org/11993): This code is very similar to what
// WebRtcVoiceMediaChannel::OnPacketReceived does. For maintainability and
// consistency it would be good to move the interaction with call_->Receiver()
// to a common implementation and provide a callback on the worker thread
// for the exception case (DELIVERY_UNKNOWN_SSRC) and how retry is attempted.
// TODO(bugs.webrtc.org/7135): extensions in `packet` is currently set
// in RtpTransport and does not neccessarily include extensions specific
// to this channel/MID. Also see comment in
// BaseChannel::MaybeUpdateDemuxerAndRtpExtensions_w.
// It would likely be good if extensions where merged per BUNDLE and
// applied directly in RtpTransport::DemuxPacket;
packet.IdentifyExtensions(recv_rtp_extension_map_);
packet.set_payload_type_frequency(webrtc::kVideoPayloadTypeFrequency);
if (!packet.arrival_time().IsFinite()) {
packet.set_arrival_time(webrtc::Timestamp::Micros(rtc::TimeMicros()));
}
call_->Receiver()->DeliverRtpPacket(
webrtc::MediaType::VIDEO, std::move(packet),
absl::bind_front(&WebRtcVideoChannel::MaybeCreateDefaultReceiveStream,
this));
}
void WebRtcVideoChannel::SetRecordableEncodedFrameCallback(
uint32_t ssrc,
std::function<void(const webrtc::RecordableEncodedFrame&)> callback) {

13
media/engine/webrtc_video_engine.h
View File

@ -234,11 +234,6 @@ class WebRtcVideoChannel : public VideoMediaChannel,
private:
class WebRtcVideoReceiveStream;
// Finds VideoReceiveStreamInterface corresponding to ssrc. Aware of
// unsignalled ssrc handling.
WebRtcVideoReceiveStream* FindReceiveStream(uint32_t ssrc)
RTC_EXCLUSIVE_LOCKS_REQUIRED(thread_checker_);
struct VideoCodecSettings {
VideoCodecSettings();
@ -281,6 +276,14 @@ class WebRtcVideoChannel : public VideoMediaChannel,
absl::optional<int> flexfec_payload_type;
};
// Finds VideoReceiveStreamInterface corresponding to ssrc. Aware of
// unsignalled ssrc handling.
WebRtcVideoReceiveStream* FindReceiveStream(uint32_t ssrc)
RTC_EXCLUSIVE_LOCKS_REQUIRED(thread_checker_);
void ProcessReceivedPacket(webrtc::RtpPacketReceived packet)
RTC_RUN_ON(thread_checker_);
bool GetChangedSendParameters(const VideoSendParameters& params,
ChangedSendParameters* changed_params) const
RTC_EXCLUSIVE_LOCKS_REQUIRED(thread_checker_);

4
media/engine/webrtc_video_engine_unittest.cc
View File

@ -1715,8 +1715,8 @@ TEST_F(WebRtcVideoChannelEncodedFrameCallbackTest,
/*is_default_stream=*/true));
EXPECT_TRUE(channel_->SetSink(kSsrc + 1, &renderer_));
channel_->SetRecordableEncodedFrameCallback(kSsrc, callback.AsStdFunction());
DeliverKeyFrame(kSsrc); // Expected to not cause function to fire.
channel_->SetDefaultSink(&renderer_);
DeliverKeyFrame(kSsrc); // Expected to not cause function to fire.
DeliverKeyFrameAndWait(kSsrc + 1);
receive_channel_->RemoveRecvStream(kSsrc + 1);
}
@ -2217,8 +2217,8 @@ TEST_F(WebRtcVideoChannelBaseTest, SetSink) {
EXPECT_TRUE(SetDefaultCodec());
EXPECT_TRUE(SetSend(true));
EXPECT_EQ(0, renderer_.num_rendered_frames());
channel_->OnPacketReceived(packet);
channel_->SetDefaultSink(&renderer_);
channel_->OnPacketReceived(packet);
SendFrame();
EXPECT_FRAME_WAIT(1, kVideoWidth, kVideoHeight, kTimeout);
}