[Bug 611689] [NEW PLUGIN] crossfeed plugin
GStreamer (bugzilla.gnome.org)
bugzilla at gnome.org
Thu Dec 11 07:14:39 PST 2014
https://bugzilla.gnome.org/show_bug.cgi?id=611689
GStreamer | gst-plugins-bad | git
--- Comment #10 from Christoph Reiter (lazka) <reiter.christoph at gmail.com> 2014-12-11 15:05:00 UTC ---
(In reply to comment #8)
> Review of attachment 284336 [details]:
>
> ::: ext/bs2b/gstbs2b.c
> @@ +22,3 @@
> + * SECTION:element-bs2b
> + *
> + * Improve headphone listening of stereo audio records using the bs2b library.
>
> Could you expand this a little and tell how it improves the audio?
Good idea, thanks.
> @@ +68,3 @@
> +{
> + PROP_0,
> + PROP_FCUT,
>
> PROP_FCUT = 1,
> and remove the PROP_0
OK.
> @@ +267,3 @@
> + case GST_EVENT_SEGMENT:
> + GST_BS2B_DP_LOCK (element);
> + bs2b_clear (element->bs2bdp);
>
> what is this doing? Maybe do this when the input-buffer has the DISCONT flag
> set?
I wanted to reset the filter on seek events. DISCONT looks like what I wanted,
thanks.
> @@ +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).
> @@ +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".
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