From 9a0662ac7e4a3bc6b3a316397a7fdf25f0025d35 Mon Sep 17 00:00:00 2001 From: Ilya Nikolaevskiy Date: Tue, 30 Oct 2018 09:49:39 +0100 Subject: [PATCH] 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} --- audio/channel_send.cc | 1 + modules/rtp_rtcp/source/rtcp_sender.cc | 9 +++++++++ 2 files changed, 10 insertions(+) diff --git a/audio/channel_send.cc b/audio/channel_send.cc index cc8b1f7842..e3d34727f6 100644 --- a/audio/channel_send.cc +++ b/audio/channel_send.cc @@ -333,6 +333,7 @@ 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 67fdf34d6f..b2aad1e7cb 100644 --- a/modules/rtp_rtcp/source/rtcp_sender.cc +++ b/modules/rtp_rtcp/source/rtcp_sender.cc @@ -258,6 +258,15 @@ 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) {