[Bug 611689] [NEW PLUGIN] crossfeed plugin
GStreamer (bugzilla.gnome.org)
bugzilla at gnome.org
Fri Dec 12 01:23:09 PST 2014
https://bugzilla.gnome.org/show_bug.cgi?id=611689
GStreamer | gst-plugins-bad | git
--- Comment #12 from Christoph Reiter (lazka) <reiter.christoph at gmail.com> 2014-12-12 09:23:03 UTC ---
(In reply to comment #11)
> (In reply to comment #10)
> > (In reply to comment #8)
> > > Review of attachment 284336 [details] [details] [details]:
> > > @@ +343,3 @@
> > > + bs2b_clear (element->bs2bdp);
> > > + GST_BS2B_DP_UNLOCK (element);
> > > + g_object_notify_by_pspec (object, properties[PROP_PRESET]);
> > >
> > > you don't need to do this.
> >
> > bs2b_set_level_fcut() will change the internal value of preset, changing the
> > result of get_property(PRESET), so shouldn't this notify that change as well?
> >
> > The same happens for the other way around where setting the preset will change
> > the filter properties (FCUT, FEED).
> >
> You are right, I misread the code.
OK.
> > > @@ +356,3 @@
> > > + case PRESET_DEFAULT:
> > > + GST_BS2B_DP_LOCK (element);
> > > + bs2b_set_level (element->bs2bdp, BS2B_DEFAULT_CLEVEL);
> > >
> > > Could you use the values in the enum to avoid the switch case here? Like drop
> > > PRESET_{DEFAULT, CMOY, ...} and use BS2B_{DEFAULT,CMOY,..}_CLEVEL in the enum
> > > above.
> >
> > BS2B_{DEFAULT,CMOY,..}_CLEVEL is a int containing the configuration state (e.g.
> > default=2949820). While its format is defined in a header comment I'd prefer if
> > this value wouldn't be exposed in the element interface. Also using gst-launch
> > syntax: "bs2b preset=0" seems nicer than "bs2b preset=2949820".
>
> You can use the enum names on gst-launch. But what do you think about actually
> implementing the preset interface. See
> https://bugzilla.gnome.org/show_bug.cgi?id=741427 for making presets usable
> from the command-line.
* Would that mean I'd lose the default presets? Or should I override the
default implementation of get_preset_names(), load_preset() and get_meta() to
provide them in any case (I can't seem to find a plugin in git which implements
the interface besides using the default impl)? Should they be deletable?
* Besides gst-launch support, would gst-inspect show the existing presets and
meta data (comments/descriptions) as is currently done for enums?
* Just a minor issue.. Afaics I would lose the property notification for the
preset if I change the other properties. I currently have the following UI:
https://i.imgur.com/skrL8cU.png where the combo box changes depending on the
sliders and vice versa.
Maybe use the default GstPreset impl (+ adjusted get_property_names ()) in
addition to the current preset property?
> >
> > Thanks for the review!
--
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.
More information about the gstreamer-bugs
mailing list