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 655257 - [camerabin2][design] Removing redundant options/elements
[camerabin2][design] Removing redundant options/elements
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other All
: Normal enhancement
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2011-07-25 12:48 UTC by Thiago Sousa Santos
Modified: 2012-01-16 12:19 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Thiago Sousa Santos 2011-07-25 12:48:00 UTC
Currently, camerabin2 is using encodebin for video and image encoding and it already contains converters and caps restriction API.

camerabin2 itself also has capsfilters and converters on the image and video branches of its pipeline. I'm considering removing those from camerabin2 to avoid redundant elements and API. The removal of the converters wouldn't require any changes to the applications, but removing the capsfilters would also require removing some properties (image-capture-caps/video-capture-caps/audio-capture-caps), this would be replaced by using the 'restriction' option on encodebin profiles.

What do you think?
Comment 1 Luciana Fujii 2011-07-25 13:13:10 UTC
I think that's a great idea. I think the change for applications is simple enough and it seems a good idea to start using the restriction option from now on.
Comment 2 Raluca-Elena Podiuc 2011-07-25 14:25:50 UTC
As far as I see it the encodebins are internal to camerabin2 and not exposed.

If somebody from outside wants to change the profile's restriction we need to either:
- expose acces to the encodebins (new camerabin2 properties?) or
- use the existing properties (image-capture-caps/video-capture-caps/audio-capture-caps), but  have them work on the encodebins directly not on camerabin2 capsfilters.

Did I understand this correctly?
Comment 3 Thiago Sousa Santos 2011-07-25 14:39:51 UTC
(In reply to comment #2)
> As far as I see it the encodebins are internal to camerabin2 and not exposed.
> 
> If somebody from outside wants to change the profile's restriction we need to
> either:
> - expose acces to the encodebins (new camerabin2 properties?) or
> - use the existing properties
> (image-capture-caps/video-capture-caps/audio-capture-caps), but  have them work
> on the encodebins directly not on camerabin2 capsfilters.
> 
> Did I understand this correctly?

To access the restriction you can simply get the profile property from camerabin2 and set the restriction on that profile using http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gst-plugins-base-libs/html/gst-plugins-base-libs-encoding-profile.html#gst-encoding-profile-set-restriction The restriction caps is what encodebin sets on its capsfilter

The second suggestion is also valid, but I guess using the profiles directly is better as we have only 1 way of doing it.
Comment 4 Thiago Sousa Santos 2011-07-25 15:04:01 UTC
Let me try to summarize the problem with a simplification of the scenario. Here's what we currently have in camerabin2*:

src ! converters ! capsfilter ! ( converters ! capsfilter ! encoder ! muxer ) ! filesink

As you can see, we have duplicates for converters and capsfilters.

Step 1 - Remove the converters

The duplicate converters really should be removed as they slow down caps negotiation and increase camerabin2 state changes and capturing negotiation time. I've already got a patch for this (credits to robswain for it). I really see no problem here other than the caps set to the first capsfilter through *-capture-caps properties now needing to be a subset of what the source can produce, otherwise you would get not-negotiated errors.

If we do this, applications should really rely on setting the restriction setting on profiles for encodebin when selecting resolutions/formats for captures instead of using the *-capture-caps properties of camerabin2. Using the restriction setting is safer as it acts after converters, that helps when the source can't produce the format directly and might avoid not-negotiated errors.

The resulting pipeline would be:

src ! capsfilter ! ( converters ! capsfilter ! encoder ! muxer ) ! filesink


Step 2 - Remove the capsfilter

We can remove the first capsfilter along with the *-capture-caps if we agree that there is no scenario where we would want to force the source to capture in a certain format, but the encoder would have its input in a different one by forcing some conversion.

Do we have such a scenario? Any kind of sensor requirement/restriction?

Can we simply set the format we want the encoder to get its input and leave the sensors+converters to do their work to provide this input? I'm no specialist in camera sensors to tell :)

Resulting pipeline:

src ! (converters ! capsfilter ! encoder ! muxer ) ! filesink


--
* The part inside ( ) is what's inside encodebin
Comment 5 Teemu Katajisto 2011-07-26 06:58:20 UTC
I agree that these are necessary changes but I am not sure if setting the capture resolution using the restriction in encodebin is clear enough for a user who does not bother to read the camerabin2 documentation :)

I propose keeping the *-capture-caps properties and setting the restriction internally in camerabin2.

If the capsfilter is needed for sources, then there could be *-source-filter-caps (or something like that) for setting the capsfilter caps.

This way the API for setting capture resolution would be obvious from user point of view.
Comment 6 Thiago Sousa Santos 2011-07-28 11:11:02 UTC
(In reply to comment #5)
> I agree that these are necessary changes but I am not sure if setting the
> capture resolution using the restriction in encodebin is clear enough for a
> user who does not bother to read the camerabin2 documentation :)
> 
> I propose keeping the *-capture-caps properties and setting the restriction
> internally in camerabin2.

The problem with this is that setting the profile again will change the values of the properties implicitly and users might not understand this if they didn't read the docs.

> 
> If the capsfilter is needed for sources, then there could be
> *-source-filter-caps (or something like that) for setting the capsfilter caps.

Agreed.

> 
> This way the API for setting capture resolution would be obvious from user
> point of view.
Comment 7 Luciana Fujii 2011-08-02 03:27:44 UTC
Ok, there is one big difference between using image-capture-caps and restriction on GstEncodingProfile and I just ran into it: restrictions only apply when the encodebin has to encode the stream. When the stream is already encoded, they don't go through the capsfilter.

I'm assuming here camerabin2 works with cameras that provide jpeg images, for instance. I think that is true at least if you don't use wrappercamerabinsrc. Correct me if I'm wrong.

This is easier to happen for image capture. So the pipeline is like this:

src ! capsfilter ! ( split name=s ! converters ! capsfilter ! encoder ! combiner name=c ) ! filesink
s. ! c.

If you want to restrict, for instance, resolution, it will only work for raw streams if you set it on restriction. It would be tricky to set it using image-capture-caps and not force to have only raw streams though. I think in that case you can actually add the restriction directly to the GstEncodingProfile to restrict the encoded stream (meaning, using caps like "image/jpeg,width=x,height=y" to create the profile),

Anyway, I still think it is a good idea, but it would be good to make this explicit in documentation, for those like me that don't realize this difference at first. =]
Comment 8 Robert Swain 2012-01-16 12:19:26 UTC
Completely unnecessary elements have since been removed and properties have been added to allow removal of others in the case where their removal is known to be safe (native-video or so in encodebin, as I recall).