[Bug 711084] rtpmanager: add new rtprtxsend and rtprtxreceive elements for retransmission

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=711084
  GStreamer | gst-plugins-good | git

Wim Taymans <wim.taymans> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED
   Target Milestone|HEAD                        |1.3.1

--- Comment #68 from Wim Taymans <wim.taymans at gmail.com> 2014-01-03 20:16:57 UTC ---
This is partially pushed now with the following changes:

 - only pushed the RTX element commits
 - modified for the rtpbin AUX element handling
 - Did not commit the SSRC-seqnum association timeout, this does not need a
jitterbuffer event and can be done from the RTX element itself because it knows
the timeout and can use the incomming timestamps to see elapsed time.
 - did not commit GstTask on srcpad, it failed the unit test for me and the
task should only push the RTX packets and should push the original stream in
the same thread.

I have the following comments:

 - I kept the src/sink pads, if we later implement session-multiplex we need to
add a request rtx pad or add a property to make such a pad. You will need to
have a bin to make an element with the right padnames for rtpbin. I tried with
request pads with session numbers (src_%u) etc but that was overly complicated.

about the RTXSender:

 - a hashtable lookup is done for each RTP packet to get the SSRC specific
info. I considered removing this but kept it in the end, we can optimize this
with a simple 1 item cache if needed. I'm not sure why exactly we would need to
support SSRC multiplexing in the RTXsender..
 - a second hashtable lookup is done to get the payload type.  pt -> RTX pt
mapping should be done with a simple array (there are only 127 pt-s possible)
 - for each RTP packet g_sequence_append is done, which does a rebalance of a
tree(!) In case the queue exceeds its limits, a couple more rebalances are
done(..). A GSequence is the wrong datastructure for this, you need a simple
dynamic ringbuffer to keep packets around. If you need packet with seqnum M,
you look at the seqnum (N) of the head of the ringbuffer and packet M is (M -
N) positions in the ringbuffer. O(1) insertion and O(1) lookup that can be done
without dynamic allocations when the ringbuffer reached its correct size. 
 - making the RTX packets looks very inefficient and seems to do a copy of all
the packet parts separately, surely doing one memcpy of everything is faster.
Better would even be to not copy anything and _ref (or share) the parts that
don't change...
 - limiting the size of the queue in time does not need the rtptime fields of
the packets, just check the timestamps on the buffers.

Similar remarks about the hashtables in rtxreceive.

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.