From 8bffba71072577ef8e81f48b615f231880de25a0 Mon Sep 17 00:00:00 2001 From: Stefan Holmer Date: Wed, 23 Sep 2015 15:53:52 +0200 Subject: [PATCH] Fix BWE bug where audio has timestamps in us. The BWE expects arrival timestamps in ms, while the audio path delivered them in us, causing the BWE to break down under the combined audio/video BWE experiment. This was introduced in r9892 (https://chromium.googlesource.com/external/webrtc/+/68786d20400f1f3744ad83549325665c18ea9e5b). BUG=webrtc:4758 R=mflodman@webrtc.org, sprang@webrtc.org Review URL: https://codereview.webrtc.org/1360913004 . Cr-Commit-Position: refs/heads/master@{#10032} --- webrtc/video/audio_receive_stream.cc | 2 +- webrtc/video/audio_receive_stream_unittest.cc | 77 +++++++++++++++++++ webrtc/webrtc_tests.gypi | 1 + 3 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 webrtc/video/audio_receive_stream_unittest.cc diff --git a/webrtc/video/audio_receive_stream.cc b/webrtc/video/audio_receive_stream.cc index b8da1bb6c3..a1cf2ca33e 100644 --- a/webrtc/video/audio_receive_stream.cc +++ b/webrtc/video/audio_receive_stream.cc @@ -102,7 +102,7 @@ bool AudioReceiveStream::DeliverRtp(const uint8_t* packet, header.extension.hasAbsoluteSendTime) { int64_t arrival_time_ms = TickTime::MillisecondTimestamp(); if (packet_time.timestamp >= 0) - arrival_time_ms = packet_time.timestamp; + arrival_time_ms = (packet_time.timestamp + 500) / 1000; size_t payload_size = length - header.headerLength; remote_bitrate_estimator_->IncomingPacket(arrival_time_ms, payload_size, header, false); diff --git a/webrtc/video/audio_receive_stream_unittest.cc b/webrtc/video/audio_receive_stream_unittest.cc new file mode 100644 index 0000000000..cf5314cea1 --- /dev/null +++ b/webrtc/video/audio_receive_stream_unittest.cc @@ -0,0 +1,77 @@ +/* + * Copyright (c) 2015 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#include "testing/gtest/include/gtest/gtest.h" + +#include "webrtc/modules/remote_bitrate_estimator/include/mock/mock_remote_bitrate_estimator.h" +#include "webrtc/modules/rtp_rtcp/source/byte_io.h" +#include "webrtc/video/audio_receive_stream.h" + +namespace webrtc { + +const size_t kAbsoluteSendTimeLength = 4; + +void BuildAbsoluteSendTimeExtension(uint8_t* buffer, + int id, + uint32_t abs_send_time) { + const size_t kRtpOneByteHeaderLength = 4; + const uint16_t kRtpOneByteHeaderExtensionId = 0xBEDE; + ByteWriter::WriteBigEndian(buffer, kRtpOneByteHeaderExtensionId); + + const uint32_t kPosLength = 2; + ByteWriter::WriteBigEndian(buffer + kPosLength, + kAbsoluteSendTimeLength / 4); + + const uint8_t kLengthOfData = 3; + buffer[kRtpOneByteHeaderLength] = (id << 4) + (kLengthOfData - 1); + ByteWriter::WriteBigEndian( + buffer + kRtpOneByteHeaderLength + 1, abs_send_time); +} + +size_t CreateRtpHeaderWithAbsSendTime(uint8_t* header, + int extension_id, + uint32_t abs_send_time) { + header[0] = 0x80; // Version 2. + header[0] |= 0x10; // Set extension bit. + header[1] = 100; // Payload type. + header[1] |= 0x80; // Marker bit is set. + ByteWriter::WriteBigEndian(header + 2, 0x1234); // Sequence number. + ByteWriter::WriteBigEndian(header + 4, 0x5678); // Timestamp. + ByteWriter::WriteBigEndian(header + 8, 0x4321); // SSRC. + int32_t rtp_header_length = kRtpHeaderSize; + + BuildAbsoluteSendTimeExtension(header + rtp_header_length, extension_id, + abs_send_time); + rtp_header_length += kAbsoluteSendTimeLength; + return rtp_header_length; +} + +TEST(AudioReceiveStreamTest, AudioPacketUpdatesBweWithTimestamp) { + MockRemoteBitrateEstimator rbe; + AudioReceiveStream::Config config; + config.combined_audio_video_bwe = true; + config.voe_channel_id = 1; + const int kAbsSendTimeId = 3; + config.rtp.extensions.push_back( + RtpExtension(RtpExtension::kAbsSendTime, kAbsSendTimeId)); + internal::AudioReceiveStream recv_stream(&rbe, config); + uint8_t rtp_packet[30]; + const int kAbsSendTimeValue = 1234; + CreateRtpHeaderWithAbsSendTime(rtp_packet, kAbsSendTimeId, kAbsSendTimeValue); + PacketTime packet_time(5678000, 0); + const size_t kExpectedHeaderLength = 20; + EXPECT_CALL(rbe, IncomingPacket(packet_time.timestamp / 1000, + sizeof(rtp_packet) - kExpectedHeaderLength, + testing::_, false)) + .Times(1); + EXPECT_TRUE( + recv_stream.DeliverRtp(rtp_packet, sizeof(rtp_packet), packet_time)); +} +} // namespace webrtc diff --git a/webrtc/webrtc_tests.gypi b/webrtc/webrtc_tests.gypi index 2515ebd101..06bb25fdde 100644 --- a/webrtc/webrtc_tests.gypi +++ b/webrtc/webrtc_tests.gypi @@ -154,6 +154,7 @@ 'test/common_unittest.cc', 'test/testsupport/metrics/video_metrics_unittest.cc', 'tools/agc/agc_manager_unittest.cc', + 'video/audio_receive_stream_unittest.cc', 'video/bitrate_estimator_tests.cc', 'video/end_to_end_tests.cc', 'video/packet_injection_tests.cc',