New pacer: make FlexFEC and Video priority equal

Perf dashboard show a very minor change with the new pacer, for tests
that use flexfec. I have found that previously fec was in fact
prioritized at the same level as video, see eg PacketTypeToPriority()
in RTPSender.

With the new pacer we put fec in between video and padding.
Not sure if this is in fact an actual problem. In the non-loss case
the frame latency should actually be slighly lower, but on the other
hand if we have loss fec won't be applied until after the full frame
has been sent and so we may end up sending NACK before we apply the
FEC and recover a packet.

Just to avoid any problems let's revert to the old behavior.

Bug: webrtc:10633
Change-Id: I9a4210a64165a6e376c0c70ccaa07b0688cc58a5
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/146714
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28678}
This commit is contained in:
Erik Språng 2019-07-25 09:16:34 +02:00 committed by Commit Bot
parent b79f564b53
commit 97b6c757bc
2 changed files with 12 additions and 7 deletions

View File

@ -57,12 +57,13 @@ int GetPriorityForType(RtpPacketToSend::Type type) {
// Video has "normal" priority, in the old speak.
return 2;
case RtpPacketToSend::Type::kForwardErrorCorrection:
// Redundancy is OK to drop, but the content is hopefully not useless.
return 3;
// Send redundancy concurrently to video. If it is delayed it might have a
// lower chance of being useful.
return 2;
case RtpPacketToSend::Type::kPadding:
// Packets that are in themselves likely useless, only sent to keep the
// BWE high.
return 4;
return 3;
}
}

View File

@ -1479,7 +1479,7 @@ TEST_P(PacedSenderTest, OwnedPacketPrioritizedOnType) {
// Insert a packet of each type, from low to high priority. Since priority
// is weighted higher than insert order, these should come out of the pacer
// in backwards order.
// in backwards order with the exception of FEC and Video.
for (RtpPacketToSend::Type type :
{RtpPacketToSend::Type::kPadding,
RtpPacketToSend::Type::kForwardErrorCorrection,
@ -1495,12 +1495,16 @@ TEST_P(PacedSenderTest, OwnedPacketPrioritizedOnType) {
EXPECT_CALL(
callback,
SendPacket(Pointee(Property(&RtpPacketToSend::Ssrc, kVideoRtxSsrc)), _));
EXPECT_CALL(
callback,
SendPacket(Pointee(Property(&RtpPacketToSend::Ssrc, kVideoSsrc)), _));
// FEC and video actually have the same priority, so will come out in
// insertion order.
EXPECT_CALL(
callback,
SendPacket(Pointee(Property(&RtpPacketToSend::Ssrc, kFlexFecSsrc)), _));
EXPECT_CALL(
callback,
SendPacket(Pointee(Property(&RtpPacketToSend::Ssrc, kVideoSsrc)), _));
EXPECT_CALL(
callback,
SendPacket(Pointee(Property(&RtpPacketToSend::Ssrc, kVideoRtxSsrc)), _));