[Bug 746543] rtpsession: Properly implement T_rr_interval and allow sending multiple early feedback packets in a row

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=746543

Magnus Westerlund <magnus.westerlund at ericsson.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |magnus.westerlund at ericsson.
                   |                            |com

--- Comment #3 from Magnus Westerlund <magnus.westerlund at ericsson.com> ---

Quotes from comment 2

"..., unless the last non-early RTCP packet is more than rtcp-min-interval in
the past (in which case we would schedule earlier)."

Is this referring to the case when there are no feedback messages, but more
than one T_rr_int has past since T_rr_last? 

Also, is that rtcp-min-interval the randomized value [0.5,1.5] times T_rr_int,
i.e. the equivalent to T_rr_interval in the RTCP spec. That randomization is
there to prevent clock synchronization in multi-party cases. 

"Exception to this is for the very first RTCP packet, which would be at 1s if
feedback is pending and otherwise at the time the current code calculates
(rtcp-min-interval/2)."

How are this dealing with the unicast rule? The RFC allows the initial delay to
be zero in case the session is unicast. Note that there are some clarifications
in an IETF draft that is fairly mature. I expect WG last call occur in the next
three months. Please see: Section 5.2 of
https://datatracker.ietf.org/doc/draft-ietf-avtcore-rtp-multi-stream/

"- on every timeout, we send RTCP if the last non-early RTCP packet is more
than rtcp-min-interval in the past, or feedback is pending (in which case we
consider this RTCP packet as early feedback). Otherwise we suppress (and allow
early feedback again)."

This appear correct, assuming that one has calculated the transmission timeout
when sending the early packet according to bullet 6 in Section 3.5.2 of RFC
4585: 

"R then MUST set allow_early = FALSE, MUST recalculate tn = tp + 2*T_rr, and
MUST set tp to the previous tn."

"Also this would be a first step to reduced-size RTCP, as we would now already
have to store the time since the last non-early RTCP packet. Which with
reduced-size RTCP would be exactly the time since the last compound
(non-reduced size) RTCP packet. And whenever we send a non-early RTCP packet,
we would have to make sure that we really send a compound RTCP packet.
Additionally we would have to take IP/UDP headers into account and not consider
all RTCP packets of approximately equal size as Olivier said in comment 1."

Yes, you do need to introduce the T_rr_last parameter and use that correctly. I
do note that already today early sent minimal Feedback Packets, even without
reduced size RTCP is supposed to not update T_rr_last. But, I guess current
implementation send full compounds with all SDES items etc. 

I would note that if you already today are not sampling the actual incoming and
outgoing RTCP packet sizes to form the value of avg_rtcp_size the
implementation will not maintain bandwidth targets when there are non-symmetric
number of SSRCs between the local and remote endpoint(s).

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.