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 773895 - encodebin: enable splitmuxsink use
encodebin: enable splitmuxsink use
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal enhancement
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks: 773896
 
 
Reported: 2016-11-03 13:44 UTC by Jan Schmidt
Modified: 2018-11-03 11:51 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
encodebin: Split implementation into a base class (143.10 KB, patch)
2016-11-03 13:45 UTC, Jan Schmidt
none Details | Review
splitencodebin: Add new element (23.28 KB, patch)
2016-11-03 13:45 UTC, Jan Schmidt
none Details | Review
encodebin: Split implementation into a base class (149.30 KB, patch)
2018-08-16 08:55 UTC, Andrew Branson
none Details | Review
splitencodebin: Add new element (19.88 KB, patch)
2018-08-16 08:56 UTC, Andrew Branson
none Details | Review

Description Jan Schmidt 2016-11-03 13:44:41 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.
Comment 1 Jan Schmidt 2016-11-03 13:45:13 UTC
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.
Comment 2 Jan Schmidt 2016-11-03 13:45:19 UTC
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.
Comment 3 Andrew Branson 2018-08-16 08:54:52 UTC
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.
Comment 4 Andrew Branson 2018-08-16 08:55:53 UTC
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.
Comment 5 Andrew Branson 2018-08-16 08:56:38 UTC
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.
Comment 6 Jan Schmidt 2018-08-16 11:42:13 UTC
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.
Comment 7 Thibault Saunier 2018-08-16 20:15:15 UTC
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.
Comment 8 Andrew Branson 2018-08-27 14:08:06 UTC
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.
Comment 9 GStreamer system administrator 2018-11-03 11:51:13 UTC
-- 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.