GNOME Bugzilla – Bug 573370
camerabin api improvements
Last modified: 2011-08-18 14:05:14 UTC
Camerabin uses a signal callback with thsi signature for "img-done" signal gboolean img_done (GstElement * elem, GString * filename, gpointer user_data) The user is supposed to modify the passed GString * filename. This is bad for two reasons: #1: camerabin has a filename property and chaging that from within the callback won't show any effect #2: its not visible from the docs that the filename parameter is a GString* as there is no type for the signal marshallers for it. #3: not using g_object_set, break notify:filename Proposal is to change the filename parameter to to "char *" and get rid of the GString totally.
Yes, please. No GStrings in API. Random other comments on the API (only had a *very* quick glance): - don't use abbreviations, ie. "img-done" => "image-done", "res" => "resolution" (also "enc", "pp", "src", "mux" etc.) - separate words in property names with a dash, ie. "audioenc" => "audio-encoder", "inputcaps" => "input-caps" - nicer names for the action signals would be nice (no "user-" prefix please, use imperatives: do-this etc.) - mix of setting stuff via properties and other stuff via action signals is a bit weird (although I do understand why you do it). - further to the above: how about (just an idea) combining the "start" + "set-res[+fps]" action signals and the "mode" property. Then you'd get something like: - action "start-video-recording" (with resolution + fps args) - action "start-image-capture" (with resolution arg) which would set the mode automatically (or warn if a new start-foo was called without a corresponding stop-foo first). - maybe "img-done"/"image-done" should be a message on the bus?
Another TODO - we should add something like the "flags" property in playbin2: http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gst-plugins-base-plugins/html/gst-plugins-base-plugins-playbin2.html#GstPlayFlags GST_CAPTURE_FLAG_NATIVE_VIDEO: - no ffmpegcolorspace (but keep the videoscale) GST_CAPTURE_FLAG_NATIVE_AUDIO: - we don't plug any audioconvert ! audioresample right now (which is fine if you use pulsesink, but for some platforms that might be needed) GST_CAPTURE_FLAG_SOFT_VOLUME: - actually pulsesrc could easily get volume and mute properties too, that would spare us the volume property in such a case - then again, we might want to just do this optimisation unconditionaly if possible and have no flag for it?
GstPhotography should use interface properties. Then people can: - g_signal_connect(camerabin,"notify:<value>",callback,user_data); - query the value ranges for the UI
> GstPhotography should use interface properties. Then people can: > - g_signal_connect(camerabin,"notify:<value>",callback,user_data); From what thread would those notify signals be emitted? This sounds like a bad idea to me - we've been trying hard in gst to get rid of notify::* and other signals emitted from threads other than the main threads (to notify the application of things). I think something message-based akin to the mixer interface stuff would be better, even if it's slightly more convoluted.
(In reply to comment #3) > GstPhotography should use interface properties. Then people can: > - g_signal_connect(camerabin,"notify:<value>",callback,user_data); > - query the value ranges for the UI > Some way of being able to query value ranges of the interface properties in the GstPhotography would be very good. Right now it can be done for example by using the V4L2 interface (if using v4l2src) directly, but it is very ugly and makes the implementation depend on things below the gstreamer layer. Useful properties from the properties would also be the following: - step size - default value Also the notifications of value changes are useful. I do not know enough about the Gstreamer internals to say which would be a good way to implement these, though.
I have a list of suggestions for the GstPhotography API. Currently the GstPhotography interface has functions for querying the value and setting the value of the different properties. The suggestion is that it would be good to have interfaces for querying other things from the properties in GstPhotography. This is an example of the functions that would be needed, using the exposure value as the place holder. gboolean gst_photography_maximum_exposure(GstPhotography * photo, guint32 * max_exposure); gboolean gst_photography_minimum_exposure(GstPhotography * photo, guint32 * min_exposure); gboolean gst_photography_step_exposure(GstPhotography * photo, guint32 * step_exposure); gboolean gst_photography_default_exposure(GstPhotography * photo, guint32 * default_exposure); Also, the notifications from value changes would be very useful - I still haven't learned enough of the Gstreamer internals to provide suggestions for implementing them, though. If you can think of a better way of querying the value ranges, please consider it, too. This is just one suggestion.
Mikko, by using interface properties (comment #3) you get introspectable parameters. You can check it its is e.g. a range or an enum, what is min/max and default value and so on.
Is here still something that should be done? camerabin was replaced by camerabin2
camerabin2 has the improvements suggested for camerabin on this bug. Haven't checked the photography interface suggestions.
Se let's assume this issue to be OBSOLETE. Please reopen if this is not the case or set to VERIFIED if is indeed not an issue anymore.