[Bug 731780] rtpjitterbuffer: free item if it is a duplicated one

[GStreamer (GNOME Bugzilla)]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #278598|none                        |needs-work
             status|                            |

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

::: gst/rtpmanager/gstrtpjitterbuffer.c
@@ +1393,3 @@
+  if (G_UNLIKELY (!rtp_jitter_buffer_insert (priv->jbuf, item, &head, NULL)))
{
+    free_item (item);
+  }

This one should go away, events have no seqnum and as such will never be
duplicates

@@ +2803,3 @@
   item = alloc_item (event, ITEM_TYPE_LOST, -1, -1, seqnum, lost_packets, -1);
+  if (G_UNLIKELY (!rtp_jitter_buffer_insert (priv->jbuf, item, &head, NULL)))
{
+    free_item (item);

I can't reproduce the leak with the unit test here (even after disabling the
sticky event misordering warnings). From looking at the code, if this happens
it is a bug in a higher layer already. There shouldn't be an item with the same
seqnum in the queue at this point.

The only way how I can see this happen is either
a) we received the packet with the seqnum now... in which case we shouldn't be
in this code at all (nothing is lost!)
b) we queued a lost event already in the jitterbuffer but did not push it out
yet (this should not happen unless the jitterbuffer is blocked downstream for
too long)

Miguel, can you provide a testcase for this problem?


But from looking at the code, we don't seem to check in the chain function if
the currently received packet currently has a lost item in the queue. If there
is a lost item for it, we would discard the packet unless I'm missing
something... just to request retransmission of that packet later.
Wim, is that correct?

@@ +3252,3 @@
           item = alloc_item (query, ITEM_TYPE_QUERY, -1, -1, -1, 0, -1);
+          if (G_UNLIKELY (!rtp_jitter_buffer_insert (priv->jbuf, item, &head,
+                      NULL))) {

Should go away, some as for events

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