Commit 66613bc0 authored by miu@chromium.org's avatar miu@chromium.org

[Cast] Aggressively send sender reports until first receiver report is received.

Identical logic change in both AudioSender and VideoSender: Send a sender
report just before each frame, up to the first 100 frames.  Then, they are
sent at ~1 second intervals thereafter.  However, if a receiver report
shows up, we halt the aggressive report sending immediately since we know
the data dependency (receiver side) has been satisfied.

Also, misc clean-ups:

1. RtpTimestampHelper threw away significant precision in its calculations
   (was truncating to millis).
2. Unit tests were using EXPECT with args in the wrong order (per gtest
   requirements).
3. Got rid of useless LocalRtcpXXXXXSenderFeedback class.

BUG=371482

Review URL: https://codereview.chromium.org/295603004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@272732 0039d316-1c4b-4281-b951-d872f2087c98
parent 0782b14b
......@@ -13,27 +13,8 @@
namespace media {
namespace cast {
const int64 kMinSchedulingDelayMs = 1;
class LocalRtcpAudioSenderFeedback : public RtcpSenderFeedback {
public:
explicit LocalRtcpAudioSenderFeedback(AudioSender* audio_sender)
: audio_sender_(audio_sender) {}
virtual void OnReceivedCastFeedback(const RtcpCastMessage& cast_feedback)
OVERRIDE {
if (!cast_feedback.missing_frames_and_packets_.empty()) {
audio_sender_->ResendPackets(cast_feedback.missing_frames_and_packets_);
}
VLOG(2) << "Received audio ACK "
<< static_cast<int>(cast_feedback.ack_frame_id_);
}
private:
AudioSender* audio_sender_;
DISALLOW_IMPLICIT_CONSTRUCTORS(LocalRtcpAudioSenderFeedback);
};
const int kNumAggressiveReportsSentAtStart = 100;
const int kMinSchedulingDelayMs = 1;
// TODO(mikhal): Reduce heap allocation when not needed.
AudioSender::AudioSender(scoped_refptr<CastEnvironment> cast_environment,
......@@ -42,9 +23,8 @@ AudioSender::AudioSender(scoped_refptr<CastEnvironment> cast_environment,
: cast_environment_(cast_environment),
transport_sender_(transport_sender),
rtp_timestamp_helper_(audio_config.frequency),
rtcp_feedback_(new LocalRtcpAudioSenderFeedback(this)),
rtcp_(cast_environment,
rtcp_feedback_.get(),
this,
transport_sender_,
NULL, // paced sender.
NULL,
......@@ -54,7 +34,7 @@ AudioSender::AudioSender(scoped_refptr<CastEnvironment> cast_environment,
audio_config.incoming_feedback_ssrc,
audio_config.rtcp_c_name,
true),
timers_initialized_(false),
num_aggressive_rtcp_reports_sent_(0),
cast_initialization_cb_(STATUS_AUDIO_UNINITIALIZED),
weak_factory_(this) {
rtcp_.SetCastReceiverEventHistorySize(kReceiverRtcpEventHistorySize);
......@@ -79,14 +59,6 @@ AudioSender::AudioSender(scoped_refptr<CastEnvironment> cast_environment,
AudioSender::~AudioSender() {}
void AudioSender::InitializeTimers() {
DCHECK(cast_environment_->CurrentlyOn(CastEnvironment::MAIN));
if (!timers_initialized_) {
timers_initialized_ = true;
ScheduleNextRtcpReport();
}
}
void AudioSender::InsertAudio(scoped_ptr<AudioBus> audio_bus,
const base::TimeTicks& recorded_time) {
DCHECK(cast_environment_->CurrentlyOn(CastEnvironment::MAIN));
......@@ -100,7 +72,22 @@ void AudioSender::SendEncodedAudioFrame(
DCHECK(!audio_frame->reference_time.is_null());
rtp_timestamp_helper_.StoreLatestTime(audio_frame->reference_time,
audio_frame->rtp_timestamp);
InitializeTimers();
// At the start of the session, it's important to send reports before each
// frame so that the receiver can properly compute playout times. The reason
// more than one report is sent is because transmission is not guaranteed,
// only best effort, so we send enough that one should almost certainly get
// through.
if (num_aggressive_rtcp_reports_sent_ < kNumAggressiveReportsSentAtStart) {
// SendRtcpReport() will schedule future reports to be made if this is the
// last "aggressive report."
++num_aggressive_rtcp_reports_sent_;
const bool is_last_aggressive_report =
(num_aggressive_rtcp_reports_sent_ == kNumAggressiveReportsSentAtStart);
VLOG_IF(1, is_last_aggressive_report) << "Sending last aggressive report.";
SendRtcpReport(is_last_aggressive_report);
}
transport_sender_->InsertCodedAudioFrame(*audio_frame);
}
......@@ -126,19 +113,47 @@ void AudioSender::ScheduleNextRtcpReport() {
cast_environment_->PostDelayedTask(
CastEnvironment::MAIN,
FROM_HERE,
base::Bind(&AudioSender::SendRtcpReport, weak_factory_.GetWeakPtr()),
base::Bind(&AudioSender::SendRtcpReport,
weak_factory_.GetWeakPtr(),
true),
time_to_next);
}
void AudioSender::SendRtcpReport() {
void AudioSender::SendRtcpReport(bool schedule_future_reports) {
DCHECK(cast_environment_->CurrentlyOn(CastEnvironment::MAIN));
const base::TimeTicks now = cast_environment_->Clock()->NowTicks();
uint32 now_as_rtp_timestamp = 0;
if (rtp_timestamp_helper_.GetCurrentTimeAsRtpTimestamp(
now, &now_as_rtp_timestamp)) {
rtcp_.SendRtcpFromRtpSender(now, now_as_rtp_timestamp);
} else {
// |rtp_timestamp_helper_| should have stored a mapping by this point.
NOTREACHED();
}
if (schedule_future_reports)
ScheduleNextRtcpReport();
}
void AudioSender::OnReceivedCastFeedback(const RtcpCastMessage& cast_feedback) {
DCHECK(cast_environment_->CurrentlyOn(CastEnvironment::MAIN));
if (rtcp_.is_rtt_available()) {
// Having the RTT values implies the receiver sent back a receiver report
// based on it having received a report from here. Therefore, ensure this
// sender stops aggressively sending reports.
if (num_aggressive_rtcp_reports_sent_ < kNumAggressiveReportsSentAtStart) {
VLOG(1) << "No longer a need to send reports aggressively (sent "
<< num_aggressive_rtcp_reports_sent_ << ").";
num_aggressive_rtcp_reports_sent_ = kNumAggressiveReportsSentAtStart;
ScheduleNextRtcpReport();
}
}
if (!cast_feedback.missing_frames_and_packets_.empty()) {
ResendPackets(cast_feedback.missing_frames_and_packets_);
}
ScheduleNextRtcpReport();
VLOG(2) << "Received audio ACK "
<< static_cast<int>(cast_feedback.ack_frame_id_);
}
} // namespace cast
......
......@@ -22,11 +22,11 @@ namespace media {
namespace cast {
class AudioEncoder;
class LocalRtcpAudioSenderFeedback;
// This class is not thread safe.
// It's only called from the main cast thread.
class AudioSender : public base::NonThreadSafe,
class AudioSender : public RtcpSenderFeedback,
public base::NonThreadSafe,
public base::SupportsWeakPtr<AudioSender> {
public:
AudioSender(scoped_refptr<CastEnvironment> cast_environment,
......@@ -49,23 +49,21 @@ class AudioSender : public base::NonThreadSafe,
void SendEncodedAudioFrame(scoped_ptr<transport::EncodedFrame> audio_frame);
private:
friend class LocalRtcpAudioSenderFeedback;
void ResendPackets(
const MissingFramesAndPacketsMap& missing_frames_and_packets);
void ScheduleNextRtcpReport();
void SendRtcpReport();
void SendRtcpReport(bool schedule_future_reports);
void InitializeTimers();
virtual void OnReceivedCastFeedback(const RtcpCastMessage& cast_feedback)
OVERRIDE;
scoped_refptr<CastEnvironment> cast_environment_;
transport::CastTransportSender* const transport_sender_;
scoped_ptr<AudioEncoder> audio_encoder_;
RtpTimestampHelper rtp_timestamp_helper_;
scoped_ptr<LocalRtcpAudioSenderFeedback> rtcp_feedback_;
Rtcp rtcp_;
bool timers_initialized_;
int num_aggressive_rtcp_reports_sent_;
CastInitializationStatus cast_initialization_cb_;
// NOTE: Weak pointers must be invalidated before all other member variables.
......
......@@ -33,6 +33,12 @@ class TestPacketSender : public transport::PacketSender {
if (Rtcp::IsRtcpPacket(&packet->data[0], packet->data.size())) {
++number_of_rtcp_packets_;
} else {
// Check that at least one RTCP packet was sent before the first RTP
// packet. This confirms that the receiver will have the necessary lip
// sync info before it has to calculate the playout time of the first
// frame.
if (number_of_rtp_packets_ == 0)
EXPECT_LE(1, number_of_rtcp_packets_);
++number_of_rtp_packets_;
}
return true;
......@@ -88,7 +94,7 @@ class AudioSenderTest : public ::testing::Test {
virtual ~AudioSenderTest() {}
static void UpdateCastTransportStatus(transport::CastTransportStatus status) {
EXPECT_EQ(status, transport::TRANSPORT_AUDIO_INITIALIZED);
EXPECT_EQ(transport::TRANSPORT_AUDIO_INITIALIZED, status);
}
base::SimpleTestTickClock* testing_clock_; // Owned by CastEnvironment.
......@@ -110,9 +116,8 @@ TEST_F(AudioSenderTest, Encode20ms) {
audio_sender_->InsertAudio(bus.Pass(), testing_clock_->NowTicks());
task_runner_->RunTasks();
EXPECT_GE(
transport_.number_of_rtp_packets() + transport_.number_of_rtcp_packets(),
1);
EXPECT_LE(1, transport_.number_of_rtp_packets());
EXPECT_LE(1, transport_.number_of_rtcp_packets());
}
TEST_F(AudioSenderTest, RtcpTimer) {
......@@ -131,8 +136,8 @@ TEST_F(AudioSenderTest, RtcpTimer) {
base::TimeDelta::FromMilliseconds(1 + kDefaultRtcpIntervalMs * 3 / 2);
testing_clock_->Advance(max_rtcp_timeout);
task_runner_->RunTasks();
EXPECT_GE(transport_.number_of_rtp_packets(), 1);
EXPECT_EQ(transport_.number_of_rtcp_packets(), 1);
EXPECT_LE(1, transport_.number_of_rtp_packets());
EXPECT_LE(1, transport_.number_of_rtcp_packets());
}
} // namespace cast
......
......@@ -98,6 +98,7 @@ class Rtcp {
base::TimeDelta* avg_rtt,
base::TimeDelta* min_rtt,
base::TimeDelta* max_rtt) const;
bool is_rtt_available() const { return number_of_rtt_in_avg_ > 0; }
bool RtpTimestampInSenderTime(int frequency,
uint32 rtp_timestamp,
base::TimeTicks* rtp_timestamp_in_ticks) const;
......
......@@ -19,9 +19,10 @@ bool RtpTimestampHelper::GetCurrentTimeAsRtpTimestamp(
const base::TimeTicks& now, uint32* rtp_timestamp) const {
if (last_capture_time_.is_null())
return false;
base::TimeDelta elapsed_time = now - last_capture_time_;
*rtp_timestamp = last_rtp_timestamp_ + elapsed_time.InMilliseconds() *
frequency_ / base::Time::kMillisecondsPerSecond;
const base::TimeDelta elapsed_time = now - last_capture_time_;
const int64 rtp_delta =
elapsed_time * frequency_ / base::TimeDelta::FromSeconds(1);
*rtp_timestamp = last_rtp_timestamp_ + static_cast<uint32>(rtp_delta);
return true;
}
......
......@@ -20,23 +20,8 @@
namespace media {
namespace cast {
const int64 kMinSchedulingDelayMs = 1;
class LocalRtcpVideoSenderFeedback : public RtcpSenderFeedback {
public:
explicit LocalRtcpVideoSenderFeedback(VideoSender* video_sender)
: video_sender_(video_sender) {}
virtual void OnReceivedCastFeedback(const RtcpCastMessage& cast_feedback)
OVERRIDE {
video_sender_->OnReceivedCastFeedback(cast_feedback);
}
private:
VideoSender* video_sender_;
DISALLOW_IMPLICIT_CONSTRUCTORS(LocalRtcpVideoSenderFeedback);
};
const int kNumAggressiveReportsSentAtStart = 100;
const int kMinSchedulingDelayMs = 1;
VideoSender::VideoSender(
scoped_refptr<CastEnvironment> cast_environment,
......@@ -51,7 +36,7 @@ VideoSender::VideoSender(
cast_environment_(cast_environment),
transport_sender_(transport_sender),
rtp_timestamp_helper_(kVideoFrequency),
rtcp_feedback_(new LocalRtcpVideoSenderFeedback(this)),
num_aggressive_rtcp_reports_sent_(0),
last_acked_frame_id_(-1),
last_sent_frame_id_(-1),
frames_in_encoder_(0),
......@@ -91,7 +76,7 @@ VideoSender::VideoSender(
rtcp_.reset(
new Rtcp(cast_environment_,
rtcp_feedback_.get(),
this,
transport_sender_,
NULL, // paced sender.
NULL,
......@@ -189,6 +174,22 @@ void VideoSender::SendEncodedVideoFrameMainThread(
DCHECK(!encoded_frame->reference_time.is_null());
rtp_timestamp_helper_.StoreLatestTime(encoded_frame->reference_time,
encoded_frame->rtp_timestamp);
// At the start of the session, it's important to send reports before each
// frame so that the receiver can properly compute playout times. The reason
// more than one report is sent is because transmission is not guaranteed,
// only best effort, so send enough that one should almost certainly get
// through.
if (num_aggressive_rtcp_reports_sent_ < kNumAggressiveReportsSentAtStart) {
// SendRtcpReport() will schedule future reports to be made if this is the
// last "aggressive report."
++num_aggressive_rtcp_reports_sent_;
const bool is_last_aggressive_report =
(num_aggressive_rtcp_reports_sent_ == kNumAggressiveReportsSentAtStart);
VLOG_IF(1, is_last_aggressive_report) << "Sending last aggressive report.";
SendRtcpReport(is_last_aggressive_report);
}
transport_sender_->InsertCodedVideoFrame(*encoded_frame);
UpdateFramesInFlight();
InitializeTimers();
......@@ -210,19 +211,25 @@ void VideoSender::ScheduleNextRtcpReport() {
cast_environment_->PostDelayedTask(
CastEnvironment::MAIN,
FROM_HERE,
base::Bind(&VideoSender::SendRtcpReport, weak_factory_.GetWeakPtr()),
base::Bind(&VideoSender::SendRtcpReport,
weak_factory_.GetWeakPtr(),
true),
time_to_next);
}
void VideoSender::SendRtcpReport() {
void VideoSender::SendRtcpReport(bool schedule_future_reports) {
DCHECK(cast_environment_->CurrentlyOn(CastEnvironment::MAIN));
const base::TimeTicks now = cast_environment_->Clock()->NowTicks();
uint32 now_as_rtp_timestamp = 0;
if (rtp_timestamp_helper_.GetCurrentTimeAsRtpTimestamp(
now, &now_as_rtp_timestamp)) {
rtcp_->SendRtcpFromRtpSender(now, now_as_rtp_timestamp);
} else {
// |rtp_timestamp_helper_| should have stored a mapping by this point.
NOTREACHED();
}
ScheduleNextRtcpReport();
if (schedule_future_reports)
ScheduleNextRtcpReport();
}
void VideoSender::ScheduleNextResendCheck() {
......@@ -313,6 +320,16 @@ void VideoSender::OnReceivedCastFeedback(const RtcpCastMessage& cast_feedback) {
if (rtcp_->Rtt(&rtt, &avg_rtt, &min_rtt, &max_rtt)) {
// Don't use a RTT lower than our average.
rtt = std::max(rtt, avg_rtt);
// Having the RTT values implies the receiver sent back a receiver report
// based on it having received a report from here. Therefore, ensure this
// sender stops aggressively sending reports.
if (num_aggressive_rtcp_reports_sent_ < kNumAggressiveReportsSentAtStart) {
VLOG(1) << "No longer a need to send reports aggressively (sent "
<< num_aggressive_rtcp_reports_sent_ << ").";
num_aggressive_rtcp_reports_sent_ = kNumAggressiveReportsSentAtStart;
ScheduleNextRtcpReport();
}
} else {
// We have no measured value use default.
rtt = base::TimeDelta::FromMilliseconds(kStartRttMs);
......@@ -368,11 +385,6 @@ void VideoSender::ReceivedAck(uint32 acked_frame_id) {
// be acked. Ignore.
return;
}
// Start sending RTCP packets only after receiving the first ACK, i.e. only
// after establishing that the receiver is active.
if (last_acked_frame_id_ == -1) {
ScheduleNextRtcpReport();
}
last_acked_frame_id_ = static_cast<int>(acked_frame_id);
base::TimeTicks now = cast_environment_->Clock()->NowTicks();
......
......@@ -23,7 +23,6 @@ namespace media {
class VideoFrame;
namespace cast {
class LocalRtcpVideoSenderFeedback;
class LocalVideoEncoderCallback;
class VideoEncoder;
......@@ -37,7 +36,8 @@ class CastTransportSender;
// RTCP packets.
// Additionally it posts a bunch of delayed tasks to the main thread for various
// timeouts.
class VideoSender : public base::NonThreadSafe,
class VideoSender : public RtcpSenderFeedback,
public base::NonThreadSafe,
public base::SupportsWeakPtr<VideoSender> {
public:
VideoSender(scoped_refptr<CastEnvironment> cast_environment,
......@@ -61,7 +61,8 @@ class VideoSender : public base::NonThreadSafe,
protected:
// Protected for testability.
void OnReceivedCastFeedback(const RtcpCastMessage& cast_feedback);
virtual void OnReceivedCastFeedback(const RtcpCastMessage& cast_feedback)
OVERRIDE;
private:
friend class LocalRtcpVideoSenderFeedback;
......@@ -69,7 +70,7 @@ class VideoSender : public base::NonThreadSafe,
// Schedule when we should send the next RTPC report,
// via a PostDelayedTask to the main cast thread.
void ScheduleNextRtcpReport();
void SendRtcpReport();
void SendRtcpReport(bool schedule_future_reports);
// Schedule when we should check that we have received an acknowledgment, or a
// loss report from our remote peer. If we have not heard back from our remote
......@@ -104,9 +105,9 @@ class VideoSender : public base::NonThreadSafe,
transport::CastTransportSender* const transport_sender_;
RtpTimestampHelper rtp_timestamp_helper_;
scoped_ptr<LocalRtcpVideoSenderFeedback> rtcp_feedback_;
scoped_ptr<VideoEncoder> video_encoder_;
scoped_ptr<Rtcp> rtcp_;
int num_aggressive_rtcp_reports_sent_;
uint8 max_unacked_frames_;
int last_acked_frame_id_;
int last_sent_frame_id_;
......
......@@ -62,6 +62,12 @@ class TestPacketSender : public transport::PacketSender {
if (Rtcp::IsRtcpPacket(&packet->data[0], packet->data.size())) {
++number_of_rtcp_packets_;
} else {
// Check that at least one RTCP packet was sent before the first RTP
// packet. This confirms that the receiver will have the necessary lip
// sync info before it has to calculate the playout time of the first
// frame.
if (number_of_rtp_packets_ == 0)
EXPECT_LE(1, number_of_rtcp_packets_);
++number_of_rtp_packets_;
}
return true;
......@@ -129,7 +135,7 @@ class VideoSenderTest : public ::testing::Test {
}
static void UpdateCastTransportStatus(transport::CastTransportStatus status) {
EXPECT_EQ(status, transport::TRANSPORT_VIDEO_INITIALIZED);
EXPECT_EQ(transport::TRANSPORT_VIDEO_INITIALIZED, status);
}
void InitEncoder(bool external) {
......@@ -193,7 +199,7 @@ class VideoSenderTest : public ::testing::Test {
}
void InitializationResult(CastInitializationStatus result) {
EXPECT_EQ(result, STATUS_VIDEO_INITIALIZED);
EXPECT_EQ(STATUS_VIDEO_INITIALIZED, result);
}
base::SimpleTestTickClock* testing_clock_; // Owned by CastEnvironment.
......@@ -214,9 +220,8 @@ TEST_F(VideoSenderTest, BuiltInEncoder) {
video_sender_->InsertRawVideoFrame(video_frame, capture_time);
task_runner_->RunTasks();
EXPECT_GE(
transport_.number_of_rtp_packets() + transport_.number_of_rtcp_packets(),
1);
EXPECT_LE(1, transport_.number_of_rtp_packets());
EXPECT_LE(1, transport_.number_of_rtcp_packets());
}
TEST_F(VideoSenderTest, ExternalEncoder) {
......@@ -248,16 +253,15 @@ TEST_F(VideoSenderTest, RtcpTimer) {
base::TimeDelta::FromMilliseconds(1 + kDefaultRtcpIntervalMs * 3 / 2);
RunTasks(max_rtcp_timeout.InMilliseconds());
EXPECT_GE(transport_.number_of_rtp_packets(), 1);
// Don't send RTCP prior to receiving an ACK.
EXPECT_GE(transport_.number_of_rtcp_packets(), 0);
EXPECT_LE(1, transport_.number_of_rtp_packets());
EXPECT_LE(1, transport_.number_of_rtcp_packets());
// Build Cast msg and expect RTCP packet.
RtcpCastMessage cast_feedback(1);
cast_feedback.media_ssrc_ = 2;
cast_feedback.ack_frame_id_ = 0;
video_sender_->OnReceivedCastFeedback(cast_feedback);
RunTasks(max_rtcp_timeout.InMilliseconds());
EXPECT_GE(transport_.number_of_rtcp_packets(), 1);
EXPECT_LE(1, transport_.number_of_rtcp_packets());
}
TEST_F(VideoSenderTest, ResendTimer) {
......@@ -283,9 +287,9 @@ TEST_F(VideoSenderTest, ResendTimer) {
// Make sure that we do a re-send.
RunTasks(max_resend_timeout.InMilliseconds());
// Should have sent at least 3 packets.
EXPECT_GE(
transport_.number_of_rtp_packets() + transport_.number_of_rtcp_packets(),
3);
EXPECT_LE(
3,
transport_.number_of_rtp_packets() + transport_.number_of_rtcp_packets());
}
TEST_F(VideoSenderTest, LogAckReceivedEvent) {
......@@ -353,16 +357,16 @@ TEST_F(VideoSenderTest, StopSendingIntheAbsenceOfAck) {
cast_feedback.media_ssrc_ = 2;
cast_feedback.ack_frame_id_ = 0;
video_sender_->OnReceivedCastFeedback(cast_feedback);
EXPECT_GE(
transport_.number_of_rtp_packets() + transport_.number_of_rtcp_packets(),
4);
EXPECT_LE(
4,
transport_.number_of_rtp_packets() + transport_.number_of_rtcp_packets());
// Empty the pipeline.
RunTasks(100);
// Should have sent at least 7 packets.
EXPECT_GE(
transport_.number_of_rtp_packets() + transport_.number_of_rtcp_packets(),
7);
EXPECT_LE(
7,
transport_.number_of_rtp_packets() + transport_.number_of_rtcp_packets());
}
} // namespace cast
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment