From fcc398163304ba0c26df5ff270905e349d3accc3 Mon Sep 17 00:00:00 2001 From: Ilya Nikolaevskiy Date: Tue, 30 Oct 2018 15:47:48 +0000 Subject: [PATCH] Revert "Use only first payload timestamp for RTCP SR generation for audio" This reverts commit 9a0662ac7e4a3bc6b3a316397a7fdf25f0025d35. Reason for revert: breaks some av sync perf tests Original change's description: > Use only first payload timestamp for RTCP SR generation for audio > > Since now RTP rate is set correctly for audio, there's no need to > use the very last data packet rtp/capture timestamps for generating > RTCP SR packets. > > Using only one (first) packet timestamp eliminates the jitter between > rtp and capture timestamps for audio. This jitter comes from the fact > that capture timestamp for audio is unknown and we generate bogus > timestamp at arbitrary, non-constant offset from the real capture time. > > Bug: webrtc:9905 > Change-Id: I855556184cfe994be39ab7780836a050f5a38c35 > Reviewed-on: https://webrtc-review.googlesource.com/c/108580 > Reviewed-by: Oskar Sundbom > Reviewed-by: Danil Chapovalov > Commit-Queue: Ilya Nikolaevskiy > Cr-Commit-Position: refs/heads/master@{#25430} TBR=danilchap@webrtc.org,ilnik@webrtc.org,ossu@webrtc.org Change-Id: I208a659379b1075258ee94613e42afd9aebe4754 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: webrtc:9905 Reviewed-on: https://webrtc-review.googlesource.com/c/108623 Reviewed-by: Ilya Nikolaevskiy Commit-Queue: Ilya Nikolaevskiy Cr-Commit-Position: refs/heads/master@{#25435} --- audio/channel_send.cc | 1 - modules/rtp_rtcp/source/rtcp_sender.cc | 9 --------- 2 files changed, 10 deletions(-) diff --git a/audio/channel_send.cc b/audio/channel_send.cc index e3d34727f6..cc8b1f7842 100644 --- a/audio/channel_send.cc +++ b/audio/channel_send.cc @@ -333,7 +333,6 @@ int32_t ChannelSend::SendRtpAudio(FrameType frameType, // This call will trigger Transport::SendPacket() from the RTP/RTCP module. if (!_rtpRtcpModule->SendOutgoingData((FrameType&)frameType, payloadType, timeStamp, - // TODO(https://bugs.webrtc.org/9905): // Leaving the time when this frame was // received from the capture device as // undefined for voice for now. diff --git a/modules/rtp_rtcp/source/rtcp_sender.cc b/modules/rtp_rtcp/source/rtcp_sender.cc index b2aad1e7cb..67fdf34d6f 100644 --- a/modules/rtp_rtcp/source/rtcp_sender.cc +++ b/modules/rtp_rtcp/source/rtcp_sender.cc @@ -258,15 +258,6 @@ void RTCPSender::SetLastRtpTime(uint32_t rtp_timestamp, int64_t capture_time_ms, int8_t payload_type) { rtc::CritScope lock(&critical_section_rtcp_sender_); - // Workaround for https://bugs.webrtc.org/9905 - // Only very first SetLastRtpTime for audio should update - // last_frame_capture_time_ms_ and last_payload_type_. - // This eliminates jitter between last rtp and capture timestamps. - // TODO(https://bugs.webrtc.org/9905): remove once the bug is fixed. - if (capture_time_ms < 0 && last_frame_capture_time_ms_ > 0 && - payload_type != -1 && last_payload_type_ == payload_type) { - return; - } // For compatibility with clients who don't set payload type correctly on all // calls. if (payload_type != -1) {