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 748068 - equalizer: not changing settings dynamically
equalizer: not changing settings dynamically
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other All
: Normal normal
: 1.5.1
Assigned To: Luis de Bethencourt
GStreamer Maintainers
Depends on:
Reported: 2015-04-17 16:37 UTC by Luis de Bethencourt
Modified: 2015-09-09 03:16 UTC
See Also:
GNOME target: ---
GNOME version: ---

we need update_coefficients () to run for the dynamic changes in the bands' properties (1.09 KB, patch)
2015-04-17 16:37 UTC, Luis de Bethencourt
none Details | Review
equalizer: fix dynamic changes on bands (1.84 KB, patch)
2015-04-21 15:46 UTC, Luis de Bethencourt
committed Details | Review

Description Luis de Bethencourt 2015-04-17 16:37:02 UTC
Created attachment 301863 [details] [review]
we need update_coefficients () to run for the dynamic changes in the bands' properties

When doing something like this the audio is clearly equalized.
gst-launch-1.0 filesrc location=sample.mp3 ! mad ! audioconvert ! equalizer-3bands band1=-12.0 band2=-24.0 ! audioconvert ! autoaudiosink

But when running the equalizer application in tests/examples/equalizer/, moving the sliders doesn't change the audio at all.
Comment 1 Luis de Bethencourt 2015-04-17 16:39:04 UTC
The equalizer application to test this is:
Comment 2 Sebastian Dröge (slomo) 2015-04-17 17:08:18 UTC
Comment on attachment 301863 [details] [review]
we need update_coefficients () to run for the dynamic changes in the bands' properties

Additionally you should ideally check in transform_ip() if it is passthrough or not with the current coefficients.

Also changing the coefficients should disable passthrough mode (unless changed back to doing nothing).
So in theory this commit shouldn't be necessary, unless a) bug in basetransform or b) we don't unset passthrough when properties change
Comment 3 Luis de Bethencourt 2015-04-17 19:15:15 UTC
OK. I will check if it is A or B tomorrow.
Comment 4 Luis de Bethencourt 2015-04-21 15:08:32 UTC
So it is B.

Passthrough isn't being unset when the properties change.

Writing a fix for that now.
Comment 5 Luis de Bethencourt 2015-04-21 15:46:30 UTC
Created attachment 302085 [details] [review]
equalizer: fix dynamic changes on bands

Move the passthrough check from the transform function to when the gain properties are changed.

Currently when we are in passthrough, even after the gains change the passthrough check never happens because it is inside the transform function which doesn't run. This fix moves this check to the set_property() function, so whenever the gain coefficient changes we check if they are all 0.0 and we can passthrough.

I also set the passthrough to TRUE at init because the gains default to 0, so we can passthrough until any gain property is changed.

Tested with both gst-launch-1.0 (properties set before rolling) and test/examples/equalizer/demo (everything default to 0.0 and changed dynamically while rolling).
Comment 6 Sebastian Dröge (slomo) 2015-04-22 09:22:30 UTC
Comment on attachment 302085 [details] [review]
equalizer: fix dynamic changes on bands

Makes sense but please explain in the commit message why checking in transform_ip() is not useful (as it will never be called in passthrough)
Comment 7 Luis de Bethencourt 2015-04-22 09:45:04 UTC
Review of attachment 302085 [details] [review]:

commit c884a3b3a5569aa8ada440e423b90618b74b01ab
Comment 8 Luis de Bethencourt 2015-04-22 09:45:23 UTC
Thanks Sebastian :)
Comment 9 Matias De lellis 2015-09-09 03:16:14 UTC
Excuse the intrusion into a closed bug, but blease, can backport it to Gstreamer 1.4?

I delayed a new version of Pragha at least two months thinking that my code had a problem.. and just now I found this bug.

I work with Debian and Fedora, and both use this branch, which has the bug, and surely all stable distributions have it.