GNOME Bugzilla – Bug 726192
srtpenc: send caps event after stream-start
Last modified: 2014-05-14 19:58:54 UTC
Currently, srtpenc pushes a new caps event in the chain function when the key is changed. This means that some times the caps event might be misordered if the segment event comes first. Also, the chain function uses gst_pad_get_current_caps. I am using the new rtpbin functionality to add an auxiliary sender (a FEC element in my case). I also add an encoder element to rtpbin (srtpenc). gst_pad_get_current_caps doesn't work in that case.
Created attachment 271637 [details] [review] fix caps misordering From the commit log: we initialize key_changed to FALSE as no key has been set yet. we also use gst_pad_push_event instead of the old gst_pad_set_caps, and gst_pad_get_current_caps instead of gst_pad_query_caps. finally, we send a caps event after the stream-start event, otherwise the caps event is only sent when a buffer is chained. this behavior is still kept to allow changing the key in the middle of the stream. I have to say that I still don't fully understand when to use gst_pad_get_current_caps and gst_pad_query_caps. I think I understand the documentation but I always end up using gst_pad_query_caps, which might mean I don't understand the documentation very well.
Ping.
Review of attachment 271637 [details] [review]: I'm not sure this patch fixes the problem correctly, after an incoming STREAM_START, the caps should be pushed in response to the incoming CAPS event already. And as the stream-start comes before everything else, the ordering should be fine? Do you have a test case ? ::: ext/srtp/gstsrtpenc.c @@ +985,3 @@ GST_OBJECT_UNLOCK (filter); + caps = gst_pad_query_caps (pad, NULL); the query is for allow possible caps the pipeline can handle through that pad, current_caps gets the caps from the currently flowing media. So in here, current_caps is what we want. @@ +1167,3 @@ + case GST_EVENT_STREAM_START: + { + // Send stream-start first. Please use /* */ C-style comments.
I changed a couple things in srtpenc. In particular, we no longer parse the SSRC, instead we just let libsrtp do it for us. Let me know if you bug still persists. What symptoms do you have exactly ?
(In reply to comment #4) > I changed a couple things in srtpenc. In particular, we no longer parse the > SSRC, instead we just let libsrtp do it for us. Let me know if you bug still > persists. What symptoms do you have exactly ? Sorry for not getting back to you (specially after my Ping). I hope I can try it this or next week at most.
(In reply to comment #4) > I changed a couple things in srtpenc. In particular, we no longer parse the > SSRC, instead we just let libsrtp do it for us. Let me know if you bug still > persists. What symptoms do you have exactly ? One of my initial problems was that I was using an AUX sender and the sender needs to propagate the query caps, otherwise rtpmanager elements will not create the pt map and so on. So, on the AUX sender sink pad I added: GST_PAD_SET_PROXY_CAPS (self->sinkpad); About the stream-start. I think the proxy caps also solved the problem. I also tried your latest changes (which are great by the way) and everything keeps working. However, I couldn't see anything that would fix the stream-start issue. My guess is that because I had to do the query caps in my initial patch I had to workaround the implications of it.