GNOME Bugzilla – Bug 719734
gst-rtsp-server: allow setting a different rtp manager
Last modified: 2014-02-25 22:28:12 UTC
Currently, rtsp-media uses a hardcoded rtpbin element. It would be good if rtpbin could be constructed by the user (in the media factory) and pass it to the media as with gst_rtsp_media_take_pipeline. Right now, the rtpbin can be configured if rtsp-media is subclassed by implementing setup_rtpbin.
Created attachment 263356 [details] [review] add create_rtpbin vmethod Add a create_rtpbin vmethod in the media factory. This makes it easy to create a different rtpbin. With this the setup_rtpbin vmethod might not make more sense as the rtpbin can now be setup when creating it.
Created attachment 263420 [details] [review] add create_rtpbin vmethod (fix) Better to keep the state_lock while the rtpbin is updated.
Is this for your srtpbin? Will it still be needed once you modify rtpbin?
Yes, srtpbin won't exist anymore. It's for the modified rtpbin so I can connect signals to it and don't need to subclass media. I could do it in the setup_rtpbin vmethod from rtsp-media, but I think the the create_rtpbin vmethod in the factory is more flexible. But right now I'm thinking this is wrong, as you can prepare and unprepare the media and prepare it again.
I already have a modified rtpbin which adds the following signals: request-rtp-encoder request-rtp-decoder request-rtcp-encoder request-rtcp-decoder link-rtp-encoder link-rtp-decoder link-rtcp-encoder link-rtcp-decoder It works, but I'm still playing with it. Too many signals for my taste, but couldn't find any other way to do it. Still working on it.
(In reply to comment #4) > But right now I'm thinking this is wrong, as you can prepare and unprepare the > media and prepare it again. What about changing the vmethod setup_rtpbin in the media for create_rtpbin? Or simply adding create_rtpbin?
Why the link signals? And the RTP and RTCP encoder need to be the same element if they share the same master key.
Created attachment 263434 [details] [review] add create_rtpbin in media not factory This one solves the prepare/unprepare/prepare. It adds a create_rtpbin to media so one can change the rtp manager. It is not necessary for the srtpbin (as it won't exist) but who knows if one wants to use a different manager. Which means I will need to subclass rtsp-media for my purposes.
(In reply to comment #7) > Why the link signals? And the RTP and RTCP encoder need to be the same element > if they share the same master key. Yes, I guess we don't need two signal and the linking can be also done in the request signal. About RTP and RTCP encoder, I thought it was possible to have separate master keys (in the srtpenc case it would need two elements), I think the RFC allows this. But may be I am wrong.
(In reply to comment #9) > (In reply to comment #7) > > Why the link signals? And the RTP and RTCP encoder need to be the same element > > if they share the same master key. > > Yes, I guess we don't need two signal and the linking can be also done in the > request signal. > Oh, wait, now I remember why two signals. The first one (request) will return the element (so it can be freed later), and the second one (link) will return the pad that needs to be ghosted.
(In reply to comment #3) > Is this for your srtpbin? Will it still be needed once you modify rtpbin? I am now subclassing rtsp-media and use setup_rtpbin, but I think giving the option of creating a different rtpbin (via create_rtpbin) it's not bad thing.
To make sure this is the last comment and it doesn't get lost, patch in comment 8 would still be valid if you find it useful.
commit ab3651d3393d3771975e6f1180ea9adf1fde0605 Author: Aleix Conchillo Flaqué <aleix@oblong.com> Date: Tue Dec 3 11:54:42 2013 -0800 media: add new create_rtpbin vmethod * gst/rtsp-server/rtsp-media.[ch]: add new create_rtpbin vmethod. https://bugzilla.gnome.org/show_bug.cgi?id=719734
Aleix, just in case you weren't aware - adding the new vfunc 'in the middle' of the GstRTSPMediaClass structure breaks ABI technically. If there had been a 1.x release yet, that would have been a problem.
(In reply to comment #14) > Aleix, just in case you weren't aware - adding the new vfunc 'in the middle' of > the GstRTSPMediaClass structure breaks ABI technically. If there had been a 1.x > release yet, that would have been a problem. Right. I knew, but I really didn't think about it when I made the patch. Thanks for the heads up!