GNOME Bugzilla – Bug 415627
[PLUGIN-MOVE] move equalizer to good
Last modified: 2008-02-08 02:49:30 UTC
These things should be done before equalizer plugin can be moved from bad to good (besides the items mentioned in the moving-plugins list): * add elements with fixed bands (equalizer3, equalizer10) equalizer3: simple low,mid,high tone control equalizer10: advanced eq for media players contrary to the current eq, this way frequency bands can have explicit labels (for the center frequency). * change the equalizer element to use GstChildProxy for the bands. this is the pattern we use for dynamic number of parameters. Using a value_array is plainful for the application side, the UI generation and additional components (like presets).
I'd be glad to use the 3 band EQ in BMP (http://beep-media-player.org), as i found that 10 bands is way too much for normal users in a media _player_: they just tend to fiddle around with the bands until it "sounds good" and don't really know what they've actually changed, giving 3 bands only for choice is not too few, but enough to adjust the sound to your needs (Thx Stefan :)
(In reply to comment #0) > These things should be done before equalizer plugin can be moved from bad to > good (besides the items mentioned in the moving-plugins list): > > * add elements with fixed bands (equalizer3, equalizer10) > equalizer3: simple low,mid,high tone control > equalizer10: advanced eq for media players > contrary to the current eq, this way frequency bands can have explicit labels > (for the center frequency). As said on bug #76522 the equalizer for rhythmbox is done in a way that it won't take any advantage of having those elements. If they had existed when I started hacking on it, I would have considered restricting the plugin to using only the particular equalizer10 element and nothing else, but now the plugin can parameterize the general equalizer element itself, so you can have any number of bands you want. Actually, I miss 2 things: * being able to retrieve the band frequencies. Would the GstChildProxy mecanism allow this for the general version of the equalizer ? * having a mapping to dB units for the gain range values (instead of [-1,1]). That's only for consistency with the ladspa equalizers. > > * change the equalizer element to use GstChildProxy for the bands. this is the > pattern we use for dynamic number of parameters. Using a value_array is > plainful for the application side, the UI generation and additional components > (like presets). > That would surely be cleaner.
Exactly, with the GstChildProxy support each band ist a child and thus has a name. Obviously frequencies for the 10band. For the 3 band I am not sure wheter if should be frequencies still or just 'low'/'mid'/'high'. Frequencies don't need translation, text does. The unit mapping is a different story. I have similar problem with generated UI and I am collecting ideas about adding ui specific hints to paramspecs.
(In reply to comment #3) > Exactly, with the GstChildProxy support each band ist a child and thus has a > name. That's ok, but if it also contains the frequency as a numeric value, it will avoid client code to parse the name. (even greater pain if the names are actually things like '10kHz' for instance) Well I realize that I only need this value because I'm interpolating presets defined with a different support...
Christophe: In the nband version the band-objects will have names like 10kHz. I can also make the frequency readable (double). For the 3/10 band version I currently have the problem that the propertyname can't start with a number. I will name them like 'Band-10kHz' and put '10kHz' into the nick.
(In reply to comment #5) > Christophe: In the nband version the band-objects will have names like 10kHz. I > can also make the frequency readable (double). Yes, if it doesn't involve too much contortion from the element side. I think that an application using the nband version in its plain extent will like to have the numeric frequency available. > For the 3/10 band version I > currently have the problem that the propertyname can't start with a number. I > will name them like 'Band-10kHz' and put '10kHz' into the nick. > That's less of a problem here, because the client can easily reproduce a 3/10 double array containing the actual frequency values (if they are documented somewhere). It just doesn't follow the 'introspection' principle.
First changed in cvs: Refactor plugin into a base class and a first subclass (nband eq). The nband eq uses GstChildProxy and is controlable. More subclasses will follow.
-bad/tests/icles/equalizer-test needs porting to the new API too.
I have added the 3 and 10 band versions. They need a bugfix in gstreamer-core.
Elements have docs and the test in -bad/tests/icles/equalizer-test has been ported. From my point of view apps can start to use the elements.
Some questions: - it's not clear what the example actually does - maybe it should print something that explains to whoever's testing it what is going on. This allows someone to verify that what you are testing indeed works. - why can the gain be negative ? does it just invert the signal then ? why should this be done in an equalizer ? - the 3band equalizer has weird frequency centers imo - can you hear changes in the 20 KHz one ? - band-width's explanation is confusing, can you give an example ? Also, why band-width and not bandwidth ? - are you sure the docs are there ? I don't see them in my built copy of the docs. I wanted to check how to use the nbands equalizer.
Example: I added a comment at the top of the source. please note that its not *my* example. Printing it would be useless as it would scroll ot of the terminal in a second. Parameter ranges: Good question. Company one designed the filter. 3band eq.: yes, the band layout is not useful. the code that calculates the center-frequencies for the bands should take the number of bands into account, when chooding the lowest frequency. band-width: it was band-width before ad I saw no reason to change it. I am open for better property descriptions. docs: the docs are in the code, but not added to plugin docs as iho that makes mre sense after the move. as the element might need to mature a bit more I can add them to plugins-bad docs, if you agree (mean we gonna recheck the moving status for next release cycle).
more info about the scaling of the gain: -1 mean lower the gain 0 mean keep the gain 1 mean raise the gain to visualize the mapping via gnuplot: set xrange [-1:1] plot 0.25 * exp (log (5) * x) - 0.25
Is the result of the mapping in dB ? So we have a [-0.2;1] dB range (according to gnuplot graph) ?
No its a gain used in the filter design. Thus I am wondering if thomas' concern regarding the negative values is right. Shouldn't that [-1:1] range map to e.g. [-20db:+6db] or as factor[0.01:4.0]. Or maybe we should even directly offer the db values.
(In reply to comment #15) > Or maybe we should even directly offer the db values. I would favor that. dB means nothing for most people, but many hifi systems use dB for displaying volume values, so we are used to that.
docs are there now.
I worked on the equalizer a bit more. Now the gain maps to -12 ... +6db. I also spread the bands a bit better over the frequency range (help the 3 band eq). The problem we still have is, that the band-width control is not working. Filters are quite narrow and thus, lowering a band is barely recognisable. We also better use shelf filter foe low and high band.
Ok, so what's missing now... docs and unit test should be all from what I see ;)
Ok, marking it as a blocker for the coming releases. This should be ready. docs are good now, there exists a test in tests/icles and a demo application in gst/equalizer. Also there is some example code in the equalizer-nbands docs.
+1 from my side to move.
And now there's also some minimal tests in tests/check/elements/equalizer.c
+1 for move - docs, demo, code look good.
The demo should probably be moved to tests/examples too when the plugin is moved.
demo moved to tests/examples, everything else in the same place in -good. Someone who knows how needs to fix up the docs to point to/include the example now.