GNOME Bugzilla – Bug 773895
encodebin: enable splitmuxsink use
Last modified: 2018-11-03 11:51:13 UTC
At the moment, it's not possible to use splitmuxsink with encodebin, because encodebin outputs already-muxed content to a single ALWAYS src pad. To work with splitmuxsink, we need to delegate both the muxing and output to an internal element. Attaching patches that create a new encodebin which has no source pad and does the required delegation. I've called it splitencodebin because that's the primary use case, but really it could be used for other purposes too. If anyone has a better more-generic name, speak up. Or, we could call it 'encodebin2' and give it a SOMETIMES src pad that makes it work as the current encodebin does.
Created attachment 339026 [details] [review] encodebin: Split implementation into a base class Create EncodeBaseBin as a base class for the existing encodebin and to allow other implementations.
Created attachment 339027 [details] [review] splitencodebin: Add new element A new element which is a copy of encodebin that doesn't have a source pad - instead it outputs to splitmuxsink or other app-provided renderer bin Modify encodebasebin to be cleverer about which sink pads to link to on the renderer - inspecting actual runtime caps and not just pad templates, since splitmuxsink necessarily publishes ANY pad templates and only knows actual caps at runtime.
Hi, Needing this to add file rolling capability to camerabin, I rebased these patches onto master at 1b6eed694ce2bcde8e4b903901d8d2ccd8db01d6, moving all subsequent EncodeBin changes to EncodeBaseBin. I also moved the initial plugin.c to the first patch as it's needed there, and added those files to the meson build for each patch. I've tested all this (as a 1.14.1 version as that's where we are) with a preliminary camerabin patch and they seem to work well with SplitMuxSink. As the first one involves moving most of EncodeBin to EncodeBaseBin, they're vulnerable to any changes in the former, so it'll need rebasing again if many more changes happen. Hope this helps, and that this can be merged soon. It'll be tough to maintain as a separate patch.
Created attachment 373353 [details] [review] encodebin: Split implementation into a base class Create EncodeBaseBin as a base class for the existing encodebin and to allow other implementations.
Created attachment 373354 [details] [review] splitencodebin: Add new element A new element which is a copy of encodebin that doesn't have a source pad - instead it outputs to splitmuxsink or other app-provided renderer bin Modify encodebasebin to be cleverer about which sink pads to link to on the renderer - inspecting actual runtime caps and not just pad templates, since splitmuxsink necessarily publishes ANY pad templates and only knows actual caps at runtime.
Thanks for updating the patches. I was canvassing some opinions on this approach before landing it and got a mixed reception, so it got a bit blocked. In the absence of any concrete suggestions for a better way to achieve the functionality, I'm tempted to land something like this patch anyway.
I think we agreed we didn't really have a way to do it without creating a baseclass and have 2 different classes, which is not optimal, but well :-) Jan did you check validate tests still pass after that? Looks like the code is basically the same, but the patch is huge, so reviewing in depth doesn't make so much sense.
This is difficult to review as it involves a large movement of code out of a frequently updated component. If it stalls again, it might be a good idea to refactor this into a rename patch followed by smaller changes from there, as that would be easier to rebase.
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/issues/304.