From 0121ff40dac41e149adae17774b57f8690d1706b Mon Sep 17 00:00:00 2001 From: Manashi Sarkar Date: Mon, 27 May 2024 13:22:22 +0000 Subject: [PATCH] Revert "Propagate arrival time inside NetEq" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 5237cbbe6851a6728c5d78add47e3d0cb80142ec. Reason for revert: Breaks build. Original change's description: > Propagate arrival time inside NetEq > > Bug: webrtc:341266986 > Change-Id: I313ded76b884e9ee0f00f43541c8e9aebc406001 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/351340 > Reviewed-by: Jakob Ivarsson‎ > Commit-Queue: Lionel Koenig > Cr-Commit-Position: refs/heads/main@{#42381} Bug: webrtc:341266986 Change-Id: I3c067b95055a8b3e7208cc6e45a5b581f8d65be6 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/351541 Commit-Queue: Manashi Sarkar Reviewed-by: Lionel Koenig Bot-Commit: rubber-stamper@appspot.gserviceaccount.com Reviewed-by: Lionel Koenig Gélas Reviewed-by: Jakob Ivarsson‎ Cr-Commit-Position: refs/heads/main@{#42387} --- api/neteq/BUILD.gn | 1 - api/neteq/neteq.h | 7 ++--- audio/channel_receive.cc | 31 ++++++++----------- modules/audio_coding/BUILD.gn | 2 -- modules/audio_coding/acm2/acm_receiver.cc | 6 ++-- modules/audio_coding/acm2/acm_receiver.h | 7 ++--- modules/audio_coding/neteq/neteq_impl.cc | 10 +++--- modules/audio_coding/neteq/neteq_impl.h | 11 ++----- .../audio_coding/neteq/neteq_impl_unittest.cc | 15 +++------ 9 files changed, 31 insertions(+), 59 deletions(-) diff --git a/api/neteq/BUILD.gn b/api/neteq/BUILD.gn index c29660d3a0..f73085affd 100644 --- a/api/neteq/BUILD.gn +++ b/api/neteq/BUILD.gn @@ -23,7 +23,6 @@ rtc_source_set("neteq_api") { "../../rtc_base:stringutils", "../../system_wrappers:system_wrappers", "../audio_codecs:audio_codecs_api", - "../units:timestamp", "//third_party/abseil-cpp/absl/types:optional", ] } diff --git a/api/neteq/neteq.h b/api/neteq/neteq.h index 8f0fc16447..927bd62111 100644 --- a/api/neteq/neteq.h +++ b/api/neteq/neteq.h @@ -23,7 +23,6 @@ #include "api/audio_codecs/audio_format.h" #include "api/rtp_headers.h" #include "api/scoped_refptr.h" -#include "api/units/timestamp.h" namespace webrtc { @@ -186,10 +185,8 @@ class NetEq { // Inserts a new packet into NetEq. // Returns 0 on success, -1 on failure. - virtual int InsertPacket( - const RTPHeader& rtp_header, - rtc::ArrayView payload, - Timestamp receive_time = Timestamp::MinusInfinity()) = 0; + virtual int InsertPacket(const RTPHeader& rtp_header, + rtc::ArrayView payload) = 0; // Lets NetEq know that a packet arrived with an empty payload. This typically // happens when empty packets are used for probing the network channel, and diff --git a/audio/channel_receive.cc b/audio/channel_receive.cc index b2fa0bf3cc..1fa396e482 100644 --- a/audio/channel_receive.cc +++ b/audio/channel_receive.cc @@ -196,8 +196,8 @@ class ChannelReceive : public ChannelReceiveInterface, private: void ReceivePacket(const uint8_t* packet, size_t packet_length, - const RTPHeader& header, - Timestamp receive_time) RTC_RUN_ON(worker_thread_checker_); + const RTPHeader& header) + RTC_RUN_ON(worker_thread_checker_); int ResendPackets(const uint16_t* sequence_numbers, int length); void UpdatePlayoutTimestamp(bool rtcp, int64_t now_ms) RTC_RUN_ON(worker_thread_checker_); @@ -205,8 +205,7 @@ class ChannelReceive : public ChannelReceiveInterface, int GetRtpTimestampRateHz() const; void OnReceivedPayloadData(rtc::ArrayView payload, - const RTPHeader& rtpHeader, - Timestamp receive_time) + const RTPHeader& rtpHeader) RTC_RUN_ON(worker_thread_checker_); void InitFrameTransformerDelegate( @@ -255,6 +254,7 @@ class ChannelReceive : public ChannelReceiveInterface, AudioSinkInterface* audio_sink_ = nullptr; AudioLevel _outputAudioLevel; + Clock* const clock_; RemoteNtpTimeEstimator ntp_estimator_ RTC_GUARDED_BY(ts_stats_lock_); // Timestamp of the audio pulled from NetEq. @@ -320,8 +320,7 @@ class ChannelReceive : public ChannelReceiveInterface, void ChannelReceive::OnReceivedPayloadData( rtc::ArrayView payload, - const RTPHeader& rtpHeader, - Timestamp receive_time) { + const RTPHeader& rtpHeader) { if (!playing_) { // Avoid inserting into NetEQ when we are not playing. Count the // packet as discarded. @@ -336,7 +335,7 @@ void ChannelReceive::OnReceivedPayloadData( // the SourceTracker from updating RtpSource information. if (source_tracker_) { RtpPacketInfos::vector_type packet_vector = { - RtpPacketInfo(rtpHeader, receive_time)}; + RtpPacketInfo(rtpHeader, clock_->CurrentTime())}; source_tracker_->OnFrameDelivered(RtpPacketInfos(packet_vector)); } @@ -344,7 +343,7 @@ void ChannelReceive::OnReceivedPayloadData( } // Push the incoming payload (parsed and ready for decoding) into the ACM - if (acm_receiver_.InsertPacket(rtpHeader, payload, receive_time) != 0) { + if (acm_receiver_.InsertPacket(rtpHeader, payload) != 0) { RTC_DLOG(LS_ERROR) << "ChannelReceive::OnReceivedPayloadData() unable to " "push data to the ACM"; return; @@ -373,9 +372,7 @@ void ChannelReceive::InitFrameTransformerDelegate( receive_audio_callback = [this](rtc::ArrayView packet, const RTPHeader& header) { RTC_DCHECK_RUN_ON(&worker_thread_checker_); - // TODO(lionelk): Get the receive time. - OnReceivedPayloadData(packet, header, - /*receive_time=*/Timestamp::MinusInfinity()); + OnReceivedPayloadData(packet, header); }; frame_transformer_delegate_ = rtc::make_ref_counted( @@ -548,6 +545,7 @@ ChannelReceive::ChannelReceive( jitter_buffer_fast_playout, jitter_buffer_min_delay_ms)), _outputAudioLevel(), + clock_(clock), ntp_estimator_(clock), playout_timestamp_rtp_(0), playout_delay_ms_(0), @@ -665,14 +663,12 @@ void ChannelReceive::OnRtpPacket(const RtpPacketReceived& packet) { rtc::saturated_cast(packet_copy.payload_type_frequency()), header.extension.absolute_capture_time); - ReceivePacket(packet_copy.data(), packet_copy.size(), header, - packet.arrival_time()); + ReceivePacket(packet_copy.data(), packet_copy.size(), header); } void ChannelReceive::ReceivePacket(const uint8_t* packet, size_t packet_length, - const RTPHeader& header, - Timestamp receive_time) { + const RTPHeader& header) { const uint8_t* payload = packet + header.headerLength; RTC_DCHECK_GE(packet_length, header.headerLength); size_t payload_length = packet_length - header.headerLength; @@ -692,8 +688,7 @@ void ChannelReceive::ReceivePacket(const uint8_t* packet, const FrameDecryptorInterface::Result decrypt_result = frame_decryptor_->Decrypt( cricket::MEDIA_TYPE_AUDIO, csrcs, - /*additional_data=*/ - nullptr, + /*additional_data=*/nullptr, rtc::ArrayView(payload, payload_data_length), decrypted_audio_payload); @@ -725,7 +720,7 @@ void ChannelReceive::ReceivePacket(const uint8_t* packet, frame_transformer_delegate_->Transform(payload_data, header, remote_ssrc_, mime_type.str()); } else { - OnReceivedPayloadData(payload_data, header, receive_time); + OnReceivedPayloadData(payload_data, header); } } diff --git a/modules/audio_coding/BUILD.gn b/modules/audio_coding/BUILD.gn index f750a3874d..0ae4e34afb 100644 --- a/modules/audio_coding/BUILD.gn +++ b/modules/audio_coding/BUILD.gn @@ -47,7 +47,6 @@ rtc_library("audio_coding") { "../../api/audio:audio_frame_api", "../../api/audio_codecs:audio_codecs_api", "../../api/neteq:neteq_api", - "../../api/units:timestamp", "../../common_audio", "../../common_audio:common_audio_c", "../../rtc_base:audio_format_to_string", @@ -713,7 +712,6 @@ rtc_library("neteq") { "../../api/neteq:neteq_api", "../../api/neteq:neteq_controller_api", "../../api/neteq:tick_timer", - "../../api/units:timestamp", "../../common_audio", "../../common_audio:common_audio_c", "../../rtc_base:audio_format_to_string", diff --git a/modules/audio_coding/acm2/acm_receiver.cc b/modules/audio_coding/acm2/acm_receiver.cc index 9181443421..3474229f7f 100644 --- a/modules/audio_coding/acm2/acm_receiver.cc +++ b/modules/audio_coding/acm2/acm_receiver.cc @@ -20,7 +20,6 @@ #include "api/audio/audio_frame.h" #include "api/audio_codecs/audio_decoder.h" #include "api/neteq/neteq.h" -#include "api/units/timestamp.h" #include "modules/audio_coding/acm2/acm_resampler.h" #include "modules/audio_coding/acm2/call_statistics.h" #include "modules/audio_coding/neteq/default_neteq_factory.h" @@ -105,8 +104,7 @@ int AcmReceiver::last_output_sample_rate_hz() const { } int AcmReceiver::InsertPacket(const RTPHeader& rtp_header, - rtc::ArrayView incoming_payload, - Timestamp receive_time) { + rtc::ArrayView incoming_payload) { if (incoming_payload.empty()) { neteq_->InsertEmptyPacket(rtp_header); return 0; @@ -141,7 +139,7 @@ int AcmReceiver::InsertPacket(const RTPHeader& rtp_header, } } // `mutex_` is released. - if (neteq_->InsertPacket(rtp_header, incoming_payload, receive_time) < 0) { + if (neteq_->InsertPacket(rtp_header, incoming_payload) < 0) { RTC_LOG(LS_ERROR) << "AcmReceiver::InsertPacket " << static_cast(rtp_header.payloadType) << " Failed to insert packet"; diff --git a/modules/audio_coding/acm2/acm_receiver.h b/modules/audio_coding/acm2/acm_receiver.h index 624a67047e..6393a866f6 100644 --- a/modules/audio_coding/acm2/acm_receiver.h +++ b/modules/audio_coding/acm2/acm_receiver.h @@ -26,7 +26,6 @@ #include "api/audio_codecs/audio_format.h" #include "api/neteq/neteq.h" #include "api/neteq/neteq_factory.h" -#include "api/units/timestamp.h" #include "modules/audio_coding/acm2/acm_resampler.h" #include "modules/audio_coding/acm2/call_statistics.h" #include "modules/audio_coding/include/audio_coding_module_typedefs.h" @@ -69,15 +68,13 @@ class AcmReceiver { // information about payload type, sequence number, // timestamp, SSRC and marker bit. // - incoming_payload : Incoming audio payload. - // - receive_time : Timestamp when the packet has been seen on the - // network card. + // - length_payload : Length of incoming audio payload in bytes. // // Return value : 0 if OK. // <0 if NetEq returned an error. // int InsertPacket(const RTPHeader& rtp_header, - rtc::ArrayView incoming_payload, - Timestamp receive_time = Timestamp::MinusInfinity()); + rtc::ArrayView incoming_payload); // // Asks NetEq for 10 milliseconds of decoded audio. diff --git a/modules/audio_coding/neteq/neteq_impl.cc b/modules/audio_coding/neteq/neteq_impl.cc index dcdac980b2..1ee4dc4eb7 100644 --- a/modules/audio_coding/neteq/neteq_impl.cc +++ b/modules/audio_coding/neteq/neteq_impl.cc @@ -193,12 +193,11 @@ NetEqImpl::NetEqImpl(const NetEq::Config& config, NetEqImpl::~NetEqImpl() = default; int NetEqImpl::InsertPacket(const RTPHeader& rtp_header, - rtc::ArrayView payload, - Timestamp receive_time) { + rtc::ArrayView payload) { rtc::MsanCheckInitialized(payload); TRACE_EVENT0("webrtc", "NetEqImpl::InsertPacket"); MutexLock lock(&mutex_); - if (InsertPacketInternal(rtp_header, payload, receive_time) != 0) { + if (InsertPacketInternal(rtp_header, payload) != 0) { return kFail; } return kOK; @@ -465,12 +464,13 @@ NetEq::Operation NetEqImpl::last_operation_for_test() const { // Methods below this line are private. int NetEqImpl::InsertPacketInternal(const RTPHeader& rtp_header, - rtc::ArrayView payload, - Timestamp receive_time) { + rtc::ArrayView payload) { if (payload.empty()) { RTC_LOG_F(LS_ERROR) << "payload is empty"; return kInvalidPointer; } + + Timestamp receive_time = clock_->CurrentTime(); stats_->ReceivedPacket(); PacketList packet_list; diff --git a/modules/audio_coding/neteq/neteq_impl.h b/modules/audio_coding/neteq/neteq_impl.h index 9fa2af8e1e..f808c9ed8b 100644 --- a/modules/audio_coding/neteq/neteq_impl.h +++ b/modules/audio_coding/neteq/neteq_impl.h @@ -13,7 +13,6 @@ #include #include -#include #include #include #include @@ -25,7 +24,6 @@ #include "api/neteq/neteq_controller_factory.h" #include "api/neteq/tick_timer.h" #include "api/rtp_packet_info.h" -#include "api/units/timestamp.h" #include "modules/audio_coding/neteq/audio_multi_vector.h" #include "modules/audio_coding/neteq/expand_uma_logger.h" #include "modules/audio_coding/neteq/packet.h" @@ -128,10 +126,8 @@ class NetEqImpl : public webrtc::NetEq { NetEqImpl& operator=(const NetEqImpl&) = delete; // Inserts a new packet into NetEq. Returns 0 on success, -1 on failure. - int InsertPacket( - const RTPHeader& rtp_header, - rtc::ArrayView payload, - Timestamp receive_time = Timestamp::MinusInfinity()) override; + int InsertPacket(const RTPHeader& rtp_header, + rtc::ArrayView payload) override; void InsertEmptyPacket(const RTPHeader& rtp_header) override; @@ -208,8 +204,7 @@ class NetEqImpl : public webrtc::NetEq { // above. Returns 0 on success, otherwise an error code. // TODO(hlundin): Merge this with InsertPacket above? int InsertPacketInternal(const RTPHeader& rtp_header, - rtc::ArrayView payload, - Timestamp receive_time) + rtc::ArrayView payload) RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_); // Returns true if the payload type changed (this should be followed by diff --git a/modules/audio_coding/neteq/neteq_impl_unittest.cc b/modules/audio_coding/neteq/neteq_impl_unittest.cc index 8283694ae2..07c924c2f2 100644 --- a/modules/audio_coding/neteq/neteq_impl_unittest.cc +++ b/modules/audio_coding/neteq/neteq_impl_unittest.cc @@ -494,8 +494,7 @@ TEST_F(NetEqImplTest, VerifyTimestampPropagation) { // Insert one packet. clock_.AdvanceTimeMilliseconds(123456); Timestamp expected_receive_time = clock_.CurrentTime(); - EXPECT_EQ(NetEq::kOK, - neteq_->InsertPacket(rtp_header, payload, expected_receive_time)); + EXPECT_EQ(NetEq::kOK, neteq_->InsertPacket(rtp_header, payload)); // Pull audio once. const size_t kMaxOutputSize = static_cast(10 * kSampleRateHz / 1000); @@ -589,9 +588,7 @@ TEST_F(NetEqImplTest, ReorderedPacket) { // Insert one packet. clock_.AdvanceTimeMilliseconds(123456); Timestamp expected_receive_time = clock_.CurrentTime(); - EXPECT_EQ(NetEq::kOK, - neteq_->InsertPacket(rtp_header, payload, - /*receive_time=*/clock_.CurrentTime())); + EXPECT_EQ(NetEq::kOK, neteq_->InsertPacket(rtp_header, payload)); // Pull audio once. const size_t kMaxOutputSize = static_cast(10 * kSampleRateHz / 1000); @@ -621,18 +618,14 @@ TEST_F(NetEqImplTest, ReorderedPacket) { rtp_header.extension.set_audio_level(AudioLevel(/*voice_activity=*/false, 1)); payload[0] = 1; clock_.AdvanceTimeMilliseconds(1000); - EXPECT_EQ(NetEq::kOK, - neteq_->InsertPacket(rtp_header, payload, - /*receive_time=*/clock_.CurrentTime())); + EXPECT_EQ(NetEq::kOK, neteq_->InsertPacket(rtp_header, payload)); rtp_header.sequenceNumber += 2; rtp_header.timestamp += 2 * kPayloadLengthSamples; rtp_header.extension.set_audio_level(AudioLevel(/*voice_activity=*/false, 2)); payload[0] = 2; clock_.AdvanceTimeMilliseconds(2000); expected_receive_time = clock_.CurrentTime(); - EXPECT_EQ(NetEq::kOK, - neteq_->InsertPacket(rtp_header, payload, - /*receive_time=*/clock_.CurrentTime())); + EXPECT_EQ(NetEq::kOK, neteq_->InsertPacket(rtp_header, payload)); // Expect only the second packet to be decoded (the one with "2" as the first // payload byte).