GNOME Bugzilla – Bug 760242
rtmpsink: should get streamheaders from caps instead of first buffer.
Last modified: 2016-02-10 00:41:47 UTC
Created attachment 318388 [details] [review] Implement a setcaps function where we get the headers, and use them. This would allow for dynamic adding of rtmpsink to a running pipeline.
Review of attachment 318388 [details] [review]: Looks good to me. Is it possible someone would have it's own muxer that does not set this into caps ? Maybe I'm mistaken, but this does not seems backward compatible if streamheader is missing, shall be make sure it still workaround it by assuming first buffer is the header ?
Well if the streamheader is not set in the caps there are a lot more plugins that will break like multifdsink right ? The whole concept of the streamheaders is so that you can start a new connection while the element has been running for a while and have all the necessary pieces to start decoding. My use case here is that I have a FLV muxer running and connected with a rtmpsink. At some point I need to remove/add the sink as something has changed. The current implementation fails as it is not sending the headers at all (just concatenating the 2 first buffers). The only way to maintain the current behaviour while implementing streamheaders would be to look at the buffers timestamps / offsets to figure if the first buffer is actually the first buffer produced by the upstream muxer or not but that's hackish :)
Nicolas, are you aware of any third party muxer that does not produce streamheaders in the caps, or was that just a general backwards compat consideration?
I'm raising the concern that this is not backward compatible (and could be made so will a little more effort). Though, I'm not aware of third party FLV muxer (maybe Pexip ?). The streamheader purpose is clear to me, but it wasn't fully implemented until recently if I remember well. If nobody cares about third parties, just merge it, but at least be aware.
So, what do we do here, go ahead with that ?
I'm fine with it. I think the benefits of the change outweigh the as-far-as-we-know-mostly-hypothetical risk of breakage with third party muxers, especially seeing that this is in -bad; and if this ever materialises it can be fixed easily. However, if you think we should block merging this on a fallback code path being implemented, that reasonable too :)
Let's solve this problem then :-D
Since git bz didn't do that this time: commit 2b457a46a06c9ada62942aa1b1773fc59530523a Author: Julien MOUTTE <julien@moutte.net> Date: Wed Jan 6 21:39:00 2016 +0000 rtpmsink: Implement setcaps that uses streamheader