After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 554839 - [rtpbin] Automaticaly remove pads
[rtpbin] Automaticaly remove pads
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal normal
: 0.10.18
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2008-10-03 11:58 UTC by Stian Selnes (stianse)
Modified: 2009-12-21 14:09 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Test case checking if GstRtpBin releases pad and is ready for new stream with same ssrc. (4.88 KB, text/plain)
2008-10-05 22:19 UTC, Stian Selnes (stianse)
  Details
Same test case, C code (6.79 KB, application/x-gzip)
2008-10-05 22:26 UTC, Stian Selnes (stianse)
  Details
Quick and dirty patch to handle SSRC timeout pad removal on the gstrtpbin (7.99 KB, patch)
2008-12-04 13:42 UTC, Ali Sabil
committed Details | Review

Description Stian Selnes (stianse) 2008-10-03 11:58:02 UTC
As I understand, the release request pad functionality in GstRtpBin is not implemented yet. Thus, it's not possible to clean up the GstRtpBin properly when on-bye-timeout or on-bye-ssrc is emitted; the dynamic pad recv_rtp_src_%d_%d_%d is not removed.

The consequence is that if there is a new incoming RTP stream with the same ssrc as the one before, no new pad will be created and the "pad-added" signal is never triggered. This may seem like a corner case, but it is quite important in my application.
Comment 1 Stian Selnes (stianse) 2008-10-05 22:19:44 UTC
Created attachment 119991 [details]
Test case checking if GstRtpBin releases pad and is ready for new stream with same ssrc.

A simple test case written in Vala. It creates a receiver that contains a GstRtpBin. gst_element_release_request_pad() is called after an RTP stream has timed out. After time out the test script also sends a new stream with same ssrc to the receiver. Currently, the pad-added signal is not emitted the second time. I believe this should work.

The output of the test case is now:
"""
  pad_added (recv_rtp_sink_0)
Sending stream #1
  on_new_stream (0, 1000000000)
  pad_added (recv_rtp_src_0_1000000000_96)
  Setting up playback of incoming stream
  Stopped sending. Waiting for timeout...
  on_timeout (0, 1000000000)
  Tearing down playback of stream
SUCCESS on stream #1
Sending stream #2
  on_new_stream (0, 1000000000)
  Stopped sending. Waiting for timeout...
  on_timeout (0, 1000000000)
FAILURE on stream #2
"""

I hope that after implementing release of these pads in GstRtpBin this test case would print "SUCCESS on stream #2" on that last line.
Comment 2 Stian Selnes (stianse) 2008-10-05 22:26:39 UTC
Created attachment 119992 [details]
Same test case, C code

Archive with the Vala-generated C code for they who are not using Vala.
Comment 3 Wim Taymans 2008-10-07 10:18:46 UTC
The example is wrong, a rtpbin.release_request_pad() is called on a sometimes pad.
Comment 4 Wim Taymans 2008-10-07 11:33:15 UTC
Not quite done yet but a start:

        * gst/rtpmanager/gstrtpbin.c: (find_session_by_pad),
        (free_session), (gst_rtp_bin_dispose), (remove_recv_rtp),
        (remove_recv_rtcp), (remove_send_rtp), (remove_rtcp),
        (gst_rtp_bin_release_pad):
        Release pads of the session manager.
        Start implementing releasing pads of gstrtpbin.

        * gst/rtpmanager/gstrtpsession.c: (remove_recv_rtp_sink),
        (remove_recv_rtcp_sink), (remove_send_rtp_sink),
        (remove_send_rtcp_src), (gst_rtp_session_release_pad):
        Implement releasing pads in gstrtpsession.
Comment 5 Ali Sabil 2008-12-04 13:42:09 UTC
Created attachment 123949 [details] [review]
Quick and dirty patch to handle SSRC timeout pad removal on the gstrtpbin

Attached is a quick and dirty patch clearing the source pads from the gstrtpbin, and adding an action signal to the gstrtpssrcdemux to make it "forget" about a specific SSRC.

This patch works, but obviously it needs a lot of cleaning and improvement, and only handles the SSRC timeout case.
Comment 6 Wim Taymans 2009-05-22 14:37:14 UTC
commit 0d014baaa4c61010c98c6899cd2338da1423014b
Author: Ali Sabil <ali.sabil at gmail.com>
Date:   Fri May 22 16:35:20 2009 +0200

    ssrcdemux: emit signal when pads are removed
    
    Add action signal to clear an SSRC in the ssrc demuxer.
    Add signal to notify of removed ssrc.
    
    See #554839
Comment 7 Chris Shoemaker 2009-05-24 23:41:49 UTC
Shouldn't the commit referenced in comment #6 mean that this bug can be resolved as FIXED?
Comment 8 Wim Taymans 2009-05-25 06:58:24 UTC
No, it needs more work. I will close the bug when it's fixed.
Comment 9 Wim Taymans 2009-05-25 14:02:19 UTC
Releasing the pads should now clean up everything with git versions. Unit test is added as well to test reuse of SSRCs in rtpbin.

Comment #5 is however suggesting the additional feature of automatically removing the SSRC pads when a bye is received from a source. This not yet working.
Comment 10 Wim Taymans 2009-12-21 13:03:48 UTC
Thinking about this some more, we don't usually remove pads just because the stream on them goes EOS, this would break quite a bit of applications that perform a flushing seek back to the beginning after getting EOS.

Since those dynamic pads were created by the element when it found an SSRC with a pt, there is also no way for the application to make the pads go away on EOS.

I would propose to add a property to rtpbin (disabled by default) to enable the removal of the EOS pads.
Comment 11 Wim Taymans 2009-12-21 14:09:05 UTC
Something like this:

commit 973469978865673c3b5b2dd79a180db5b4b169e6
Author: Wim Taymans <wim.taymans@collabora.co.uk>
Date:   Mon Dec 21 15:05:09 2009 +0100

    rtpbin: add property to remove pads automatically
    
    Add a property called autoremove to automatically remove the pads of sources
    that timed out.
    
    Fixes #554839