After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 654583 - Immediate RTCP in rtpsession
Immediate RTCP in rtpsession
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal normal
: 0.10.31
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2011-07-14 00:24 UTC by Olivier Crête
Modified: 2011-07-25 15:23 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
rtpsession: Don't let the computed RTP bandwidth fall too low (1.01 KB, patch)
2011-07-14 00:25 UTC, Olivier Crête
none Details | Review
rtpsession: Always send application requested feedback in immediate mode (3.94 KB, patch)
2011-07-14 00:25 UTC, Olivier Crête
none Details | Review
rtpsession: Don't let the computed RTP bandwidth fall too low (1.01 KB, patch)
2011-07-14 20:25 UTC, Olivier Crête
committed Details | Review
rtpsession: Always send application requested feedback in immediate mode (4.16 KB, patch)
2011-07-25 15:04 UTC, Olivier Crête
committed Details | Review

Description Olivier Crête 2011-07-14 00:24:59 UTC
RFC 4585 describes 3 modes for sending RTCP: Regular, Early and Immediate. We currently implement the first two, the attached patches implement the 3rd. In immediate mode, RTCP feedback is sent immediately at the application's request in addition to regular RTCP packets.
Comment 1 Olivier Crête 2011-07-14 00:25:17 UTC
Created attachment 191933 [details] [review]
rtpsession: Don't let the computed RTP bandwidth fall too low

If it falls too low, the computer RTCP bandwidth will be near zero and
the RTCP thread will be stopped.
Comment 2 Olivier Crête 2011-07-14 00:25:20 UTC
Created attachment 191934 [details] [review]
rtpsession: Always send application requested feedback in immediate mode

Send as many application requested feedback messages in immediate mode, even if they
have already been sent.
Comment 3 David Schleef 2011-07-14 20:17:22 UTC
Review of attachment 191933 [details] [review]:

Typo in the commit message.
Comment 4 Olivier Crête 2011-07-14 20:25:39 UTC
Created attachment 191998 [details] [review]
rtpsession: Don't let the computed RTP bandwidth fall too low

If it falls too low, the computed RTCP bandwidth will be near zero and
the RTCP thread will be stopped.
Comment 5 Olivier Crête 2011-07-14 20:26:15 UTC
Comment on attachment 191933 [details] [review]
rtpsession: Don't let the computed RTP bandwidth fall too low

Ooops, updated the patch
Comment 6 Mark Nauwelaerts 2011-07-25 13:58:41 UTC
(In reply to comment #2)
> Created an attachment (id=191934) [details] [review]
> rtpsession: Always send application requested feedback in immediate mode
> 
> Send as many application requested feedback messages in immediate mode, even if
> they
> have already been sent.

> -  /*  RFC 4585 section 3.5.2 step 4 */
> -  if (sess->allow_early == FALSE)
> +  /*  RFC 4585 section 3.5.2 step 4
> +   * But allow feedback anyway if we are below the immediate feedback threshold
> +   */
> +  if (sess->total_sources < sess->rtcp_immediate_feedback_threshold ||
> +      sess->allow_early == FALSE)
>      goto dont_send;

Seems odd here; the intention is apparently to allow feedback/sending if below threshold, but the code path will go to dont_send (which does not sound promising ...)

There is also a stray empty line added somewhere, and the default value for the new property is not set in _init (afaics).
Comment 7 Mark Nauwelaerts 2011-07-25 14:22:30 UTC
Committed other patch:

commit 354faabda02f84d68ae24fe3d96242e8a242f456
Author: Olivier Crête <olivier.crete@collabora.com>
Date:   Wed Jun 8 14:48:01 2011 -0400

    rtpsession: Don't let the computed RTP bandwidth fall too low
    
    If it falls too low, the computed RTCP bandwidth will be near zero and
    the RTCP thread will be stopped.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=654583
Comment 8 Olivier Crête 2011-07-25 15:04:08 UTC
Created attachment 192619 [details] [review]
rtpsession: Always send application requested feedback in immediate mode

Thanks for the feedback, patch updated.
Comment 9 Mark Nauwelaerts 2011-07-25 15:23:18 UTC
Thanks.

commit 6095d2a3f00e1cb14736648fed42751b5f8a7832
Author: Olivier Crête <olivier.crete@collabora.com>
Date:   Wed Jun 8 15:57:37 2011 -0400

    rtpsession: Always send application requested feedback in immediate mode
    
    Send as many application requested feedback messages in immediate mode, even if they
    have already been sent.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=654583