GNOME Bugzilla – Bug 719829
rtp: Add RFC4571 framing/de-framing element
Last modified: 2013-12-04 20:59:26 UTC
The attached patches add RFC4571 framing/de-framing elements: https://tools.ietf.org/html/rfc4571 Any suggestions about a better element name, and the caps handling would be great :)
Created attachment 263488 [details] [review] rtptcpframe: Add RFC4571 RTP TCP framing element
Created attachment 263489 [details] [review] rtptcpdeframe: Add RFC4571 RTP TCP de-framing element
Created attachment 263493 [details] [review] rtptcpframe: Don't copy RTP memory but just append the existing ones without copying
So, question is how to call the element and the caps. This is not just for TCP but for any connection-based transport protocol. And is "framing" a good word here? The element could also be called payloader, but just putting two bytes at the front called payloading...? Or a muxer/demuxer? For the caps and the first part of the name... instead of TCP we could also just call it like the RFC maybe? Any other ideas? Support for this could also be added to sdpdemux.
Review of attachment 263493 [details] [review]: ::: gst/rtp/gstrtptcpframe.c @@ +170,3 @@ + /* Resize to 2 bytes and append all RTP memory without copying */ + gst_buffer_resize (outbuf, 0, 2); Resize a buffer here doesn't make sense. Possibly not a good use-case for basetransform.
Even though the RFC defines this as RTP/RTCP specific, the framing really isn't. I'd call them rfc4571frame and rfc4571deframe with caps application/x-rfc4571-stream and "ANY" on the other side. Possibly having application/x-rfc4571-stream, original-caps=(caps)"original caps...." for the output of the encoder. And I think they probably fit as payloader/depayloader
(In reply to comment #5) > Review of attachment 263493 [details] [review]: > > ::: gst/rtp/gstrtptcpframe.c > @@ +170,3 @@ > > + /* Resize to 2 bytes and append all RTP memory without copying */ > + gst_buffer_resize (outbuf, 0, 2); > > Resize a buffer here doesn't make sense. Possibly not a good use-case for > basetransform. True, but all other base classes don't fit at all and just going from GstElement will require a lot of code :) I'm fine with changing this to a plain GstElement if everybody agrees that this is a good idea, the cost of downsizing the buffer there and allocating it larger should be minimal though. (In reply to comment #6) > Even though the RFC defines this as RTP/RTCP specific, the framing really > isn't. > I'd call them rfc4571frame and rfc4571deframe with caps > application/x-rfc4571-stream and "ANY" on the other side. Possibly having > application/x-rfc4571-stream, original-caps=(caps)"original caps...." for the > output of the encoder. And I think they probably fit as payloader/depayloader While it is usable for anything else too, I don't think we should allow that as it won't interoperate with anything and would not comply with the RFC at all. We could call that application/x-uint16be-prefixed-packets then ;) Having the original RTP/RTCP caps with just a different structure name inside the RFC4571 caps has the advantage that the element can automatically transform them and put them on the srcpad. Otherwise with ANY caps on the srcpad you'll need something else to set the caps there again, instead of just between tcp*src and the depayloader.
What about application/x-rtp-stream then ? You also need to add application/x-srt[c]p in there. And application/x-dtls-rt[c]p, etc
(In reply to comment #8) > What about application/x-rtp-stream then ? You also need to add > application/x-srt[c]p in there. And application/x-dtls-rt[c]p, etc That's a good suggestion, thanks. So rtpstream(de)pay, application/x-rt(c)p-stream. srt(c)p and dtls-rt(c)p are not listed in that RFC, is there another one that allows usage of these packets here? Or are they just normal RT(C)P packets?
It's not defined in any RFC, but it's still used in practice.
Ok, I'll add those then :) Any other caps to add?
wait for the dtls one, as the elements arent finalized yet, we can all add them at the same time.
commit d87f6cf48386af4a62097b1ddc6f315e908eba52 Author: Sebastian Dröge <sebastian@centricular.com> Date: Wed Dec 4 21:17:03 2013 +0100 rtpstreamdepay: Add RFC4571 RTP stream depayloading element https://bugzilla.gnome.org/show_bug.cgi?id=719829 commit c5284dc0475701aa3df7749e29a965c684727be2 Author: Sebastian Dröge <sebastian@centricular.com> Date: Wed Dec 4 10:12:46 2013 +0100 rtpstreampay: Add RFC4571 RTP stream payloading element https://bugzilla.gnome.org/show_bug.cgi?id=719829