[Bug 790661] rtpsession: fix allow_early flag for RTCP early handling

[GStreamer (GNOME Bugzilla)]

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

Sebastian Dröge (slomo) <slomo at coaxion.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #364281|none                        |reviewed
             status|                            |

--- Comment #8 from Sebastian Dröge (slomo) <slomo at coaxion.net> ---
Review of attachment 364281:
 --> (https://bugzilla.gnome.org/review?bug=790661&attachment=364281)

Various typos in the commit message.

"... where all RTCP packet *are* early and *r*egular RTCP packets are sent
again after minutes"
"making this complex algorithm"

::: gst/rtpmanager/rtpsession.c
@@ -4284,3 @@
-     * max_delay */
-    if (sess->last_rtcp_check_time + T_rr > current_time)
-      offset = (sess->last_rtcp_check_time + T_rr) - current_time;

After reading all the code again, I'm not sure why this code existed at all. If
last time was a regular RTCP packet, we should've just reset allow_early. The
other case is already covered by the code below, and a regular RTCP packet
would be sent in time that covers this early request. And this case here would
actually prevent a regular RTCP packet to be sent while that's exactly what
should happen.

Your changes simplify the code and seem more correct.

::: tests/check/elements/rtpsession.c
@@ +738,3 @@
+
+  GST_DEBUG ("Wait for RTCP...");
+  g_usleep (2000000);

Check the gst_test_clock_advance_time() / crank_rtcp_thread() code in the other
tests. You can do this without a sleep and deterministically.

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