nightingale-media-player / nightingale-hacking

Working tree for the community fork of Songbird, Nightingale. If building, use the sb-trunk-oldxul (development) branch, with the tag 1.12.1 tag for stable, for now. The master-xul-9.0.1 branch is the current progress in building Nightingale with XULRunner 9 and builds, but is broken. All help in terms of patches and pull requests is welcome.
https://getnightingale.com
GNU General Public License v2.0
185 stars 41 forks source link

Equaliser settings don't match displayed settings #271

Open Peterpion opened 10 years ago

Peterpion commented 10 years ago

Hi, hope this is the right place to submit a bug (just joined to do so). Absolutely great project you have here, i've just moved to using it 100% and noticed a bug in the process.

This is on OSX (a mac) so its possible this bug might be restricted to OSX.

Briefly on the build I have (1.12.1, Build 2454 (20140112193356)) when you set the equaliser settings the 'set' values (for the sliders) are not being used to actually set the filter 'cut' or 'boost' values (ie what you hear is not what you set).

I used to be a sound engineer and I've written a multichannel parametric eq for linux in C so I know what filters sound like and have debugged a similar program before, and I can see that whats happening is that the slider positions are not being properly updated to the filters. When you set a slider position and then close the dialogue and reopen it, the settings are either as they were before you just changed it, or they are different (but still not what you set them to). Not by a small amount but miles out. The problem seems to be just the GUI probably as I can hear the filters doing their work (but you can't set a filter pattern that you actually want since you cant see what random positions the sliders have set themselves to).

So as above if I open the graphic eq panel and set some sliders, the sound does change (just not as I want it). It looks correct on the display though. But whats being shown is not whats being applied to the music. If I close the graphic eq and reopen it, I see the ACTUAL slider positions (the positions correspond to the sound I am heading). But still if I try to adjust it, its not possible to make it work properly as the actual filter values just dont match the slider positions at all.

I did install the equaliser pane extension and then when I realised the problem existed I uninstalled it, so its possible this problem is associated with that plugin.

I'm not familiar with the languages used in nightindale (theres a modern html and browser extension files etc) but if theres any C involved I could probably help find this bug. Also I might be able to donate some of the parametric filters I created if its possible to plug them in (they are in C). Not really sure how nightindale is written yet as I haven't looked at the source but I thought I would report this problem and see if its just my installation or its a real problem.

Pete

--- Want to back this issue? **[Post a bounty on it!](https://www.bountysource.com/issues/1429093-equaliser-settings-don-t-match-displayed-settings?utm_campaign=plugin&utm_content=tracker%2F230233&utm_medium=issues&utm_source=github)** We accept bounties via [Bountysource](https://www.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F230233&utm_medium=issues&utm_source=github).
freaktechnik commented 10 years ago

As you seem to have some background on the matter, I guess you waited long enough for the filter in theory to be applied? We do have a bit a delay as our gstreamer pipe has quite a big buffer. This results in a noticeable delay until EQ changes are applied. the EQ backend is in components/mediacore/gstreamer/src in a file named something like sbIMultichannelEqualizer.cpp (So it's C++). As mentioned it is based on gstramer.

Peterpion commented 10 years ago

Hi, yes I noticed the delay but thats not confusing the problem (as an aside I think its not a problem to have a few second delay for this type of app). I suspect the problem is not in the actual filter code as I say (in fact the more I think about it the more sure I get) because when I reopen the eq window the settings do seem to reflect the audible filter settings (ie its pretty easy to hear a large boost at 1kHz so whether its showing in the gui or not you can tell its being applied). Im pretty sure its just the gui doing something odd, I suspect its a very small error. I haven't compiled it locally yet so first thing would be to grab the latest version (I downloaded the latest OSX binary).

If I wanted to log the settings being sent to gstreamer without delving too far into the guts, where would be a good place to stick a printf (or js equivalent)? I guess the values are set once per onchange rather than continually, but off the top of your head can you think of a good place to look (to save me spending hours learning the project)? As I say my javascript and such are not greatly strong!

Perhaps since a lot of the code on my local machine looks to be 'uncompiled' and hence editable I can add a printf to a file to log these values without compiling afresh?

Thanks for a great project btw.

Peterpion commented 10 years ago

Sorry to spam but I thought I would add I am using the pink martini feather set, customise filter pane, now playing art, track art fetcher, and have grid view installed but not being used, in case anyone wants to try to replicate the problem I am seeing. I've tested the default feathers but the problem still exists for me. I've also modified layout.css.devPixelsPerPx from -1 to 1.05 in about:config, and am using british english. I shouldn't think any of this matters but since it looks a gui thing to me I thought I better mention in case it rings a bell to someone who knows the code better.

thebecwar commented 10 years ago

The media core is kinda complicated. The EQ band sliders control the EQ band object which is used by the mediacore EQ component, which the media core uses to make settings changes on the GStreamer pipeline, of course in all actuality it's a lot more complicated than that. All of the mediacore components are written in C++.

That all said, I can't reproduce the bug with the sliders moving without being set, and the EQ works as expected on my system with changes coming immediately (almost). Isolating each band seems to work. The localization and skin settings shouldn't have an effect on this. This looks like an OS specific problem.

@freaktechnik - Does the OSX build use a bundled GStreamer implementation, or is it using system?

Anyone who's on a mac who can confirm this bug still exists in the current sb-trunk-oldxul?

freaktechnik commented 10 years ago

@thebecwar on Mac we use a bundked gstreamer. So it's possible it has bugs. However it's also possible that there is a rounding error somewhere in the chain...

Peterpion commented 10 years ago

Sounds pretty complex. I would love to really get to grips with the whole project but you know how it is, time is always limited so a shortcut to a fix would be attractive :-) Therefore let me try to explain an exact method to reproduce.

Kill and reopen Nightindale. Click 'music' in the service panel. Select an artist, album and double click a song and it starts playing. Click Controls > Equaliser. EQ window opens up. Currently its set to the last eq before closing the program. 1kHz slider is at the top as are the 32Hz and 64Hz sliders. Drag 1kHz slider to middle position (it stays there). Close EQ window. Reopen EQ window, 1kHz slider is back at the top. Drag it right to the bottom. Close EQ window. Reopen EQ window, 1kHz slider is now where I left it (at the bottom). Move it to the mid position (it stays). Close EQ window, open EQ window, its back to the top. Drag it to mid position (I can hear there is still a significant boost to 1kHz - can't tell how many db of course by ear!)

It seems that when I reopen the EQ window, any slider which has been touched has its position boosted by about 2.5 times. IE if I set all the sliders to about 18%, close and reopen the EQ window, they are all now about 50% (IE middle position) (the error in setting them to 18%, maybe plus minus 2%, is now boosted to about plus minus 5% also and the sliders are noticeably more uneven than when I set them to around 18%). So now all sliders are around the middle. If I dont touch them and close the EQ window and reopen it, they are all still in the same place. However if I touch a particular slider but still leave it at the same place (in this case, the middle, so all sliders are in the middle still) then when I close and reopen the EQ window, the slider I touched is now up to maximum.

Seems to me that when a slider is touched, the number sent to the backend (wherever the settings are stored) is not scaled right (maybe the backend stores 0 to 1, and the number being sent back by the gui is 0 to 3 for instance). So when the backend is updated to (say) 1.5, the backend considers this 100%. When the EQ window reopens and reads the values from the backend, its using the 0 to 1 range, but when it updates, its sending incorrectly ranged numbers back.

Where is the code which draws the EQ window? And is it html / javascript / something I can understand and open locally?

Thanks for your help, Pete

Peterpion commented 10 years ago

Yea rounding or maybe the code which maps slider position to number is wrongly scaled? Its mapping the stored values correctly and displaying them right but when you adjust it, its multiplying the slider position by about 2.5 to 3 I think.

freaktechnik commented 10 years ago

I tried your STR on linux. However the only thing that I noticed was the inconsistency in the slider's positions. 0 isn't always in the same spot, depending where you come from. However since I use a development build I do have percentage indicators where I can see where 0 is. We are speaking about a visual inaccuracy of about the knob's size, when pulling the slider from the extreme position to the middle. When reopening the EQ the slider's position is correct again. So there probably is a calculation error in the EQ slider binding (or a binding it derives from, which would also explain some issues with the volume slider, don't remember the issue # atm). I should be able to try this on windows over the weekend.

Peterpion commented 10 years ago

For the purpose of description I will define the knob being at the bottom as 0% and at the top as 100%.

I also notice FWIW that when I click on a slider knob, the knob jumps to 100% (the very top) until I move the mouse (while holding the mouse button down) at which point the knob moves to the mouse pointer. If I just click and release without moving the mouse during the button hold the knob stays at 100%.

Another experiment:. If I click and release once at (roughly) position 18%, the knob moves to the mouse pointer (at 18%) but if I close and reopen the window at this time the knob is at about 50%. However if I make a second click without moving the mouse between clicks, the knob moves to about 50%. This 'twice clicking' results in the knob ending up at the 'true' position (IE the one shown when I close and reopen the EQ window). IE (STR): Click and release at point n% (not necessarily where the knob is but on the 'track') and do the same again, the knob jumps to about (n*2)% (if thats greater than 100% then the knob sticks at 100). If I use this technique to set all knobs to 50% not only do the positions stay the same if I close and reopen the EQ window, but also I can hear that the sound is as it should be (flat EQ in this case).

freaktechnik commented 10 years ago

I blame XULRunner. At least to some degree. However I do not notice this on Linux, but we have a more recent version of XULR on Linux.

Peterpion commented 10 years ago

Interesting (just read up on XULRunner). Is there a place I could check at the edge of code under our control? Where is the code for the EQ window creation and sliders etc?

freaktechnik commented 10 years ago

The GUI part starts at app/content/xul/mediacore and goes down to app/content/bindings which then reaches to components/mediacore/base which then goes to components/mediacore/gstreamer.This is without the chain of the slider binding itself, just the path of the data. To log from js you might want to use something along the lines of Components.utils.reportError() or just dump(), which will show up in the system console when you start ngale with it.

Peterpion commented 10 years ago

OK thanks. Been trying to get it to build today but its resisting (versioning of xcode etc) so until I backup my system I don't want to go too far with changing it round in case changing things breaks my other projects (work ones etc). I'm usually a Linux person but just recently got this machine to learn to write iphone apps so I'm not very familiar with OSX and scared of breaking it. I'm using the mac as a daily machine ATM because its quite nice and smooth / pretty with its interface. Re this 'bug' (if its genuine as its still not reproduced I believe) - how much could I do by just editing the javascript and other source etc thats in a binary install? I've done a quick bit of grepping through the installation directories and not found anything but its worth asking. Is all the code I would be interested in compiled code? Thanks for your help btw. I can live without the graphic TBH as the player is really good. And theres other options to install system wide graphics if mac users want.

freaktechnik commented 10 years ago

I guess you could easily experiment with the slider's rounding and stuff, as only the gstreamer backend of the possibly relevant code is in native code. However you'll have to take the files out of the songbird.jar, edit them, reinsert them into the jar and then start Nightingale. Sadly we don't have a Mac OS X nightly build bot yet, else you could use a nightly, as it'd have the slider % indicators.

rsjtdrjgfuzkfg commented 10 years ago

Cannot reproduce on windows.

freaktechnik commented 10 years ago

Hm, the missmatch between settings and what you (@peterpion) hear might be caused by the advertised Gain Frequencies not being true. At least in the documentation of the Equalizer we use in the backend for gstreamer 1.0 the bands are different: http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gst-plugins-good-plugins/html/gst-plugins-good-plugins-equalizer-10bands.html I don't know if gst0.10 had different bands. EDIT: we seem to adjust the bands to the frequencies we want, if I understand this correctly: https://github.com/nightingale-media-player/nightingale-hacking/blob/sb-trunk-oldxul/components/mediacore/gstreamer/src/sbGStreamerMediacore.cpp#L1745

Peterpion commented 10 years ago

How could I check that? Due to lack of backup on this mac, I still havent been brave enough to install willynilly what I need to compile the project and am just doing without the graphic until I can compile - from the description you posted the control data path through various code modules seems complex enough that I wont be able to find the problem without the ability to compile. But re what you just posted, if I understand right, I don't think its that. IE the 1kHz slider does sound to my pretty experienced ear to be working on 1kHz. The odd thing is as per my last post (or last but one) - pick a specific spot on the slider track, lets say pick a spot at position 20% (if the slider range is described as at 0% at the bottom, and 100% at the top) - click that spot once, the knob will move to the spot you clicked (but the sound will not reflect this position correctly - the filter is NOT at 20%) - dont move the mouse, but click a second time on this spot (so youre clicking on the knob now) - the slider will now pop up to about 50 to 60% and now it WILL correctly reflect the sound. So the filter itself has not changed when you click the second time (I can hear it hasent changed), but the knob is now at the correct position.

I understand this now so well that I can almost perfectly imagine whats happening in the code, and maybe given a bit of time I could find it without being able to compile, but without being able to test it, there seems little point. On the other hand if someone could compile with a patch I supply and send me the binary maybe its worth me having a go.

Also has anyone compiled a more recent binary for the mac that I could test? This might be gone in the current build.

Cheers, Pete

On Sunday, 16 March 2014, 15:48, Martin Giger notifications@github.com wrote:

Hm, the missmatch between settings and what you (@peterpion) hear might be caused by the advertised Gain Frequencies not being true. At least in the documentation of the Equalizer we use in the backend for gstreamer 1.0 the bands are different: http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gst-plugins-good-plugins/html/gst-plugins-good-plugins-equalizer-10bands.html I don't know if gst0.10 had different bands. — Reply to this email directly or view it on GitHub.

johnmurrayvi commented 10 years ago

Hi Pete (@Peterpion), There are actually a couple of builds I did somewhat recently for #186. Here are the links:

Nightingale_1.12.2a-2455_macosx-i686-debug.dmg Nightingale_1.12.2a-2455_macosx-i686-debug.dmg.md5

Nightingale_1.12.2a-2455_macosx-i686-release.dmg Nightingale_1.12.2a-2455_macosx-i686-release.dmg.md5

johnmurrayvi commented 10 years ago

Also, if you are interested in poking around at the code (C++), this would certainly seem like the place to start: sbBaseMediacoreMultibandEqualizer.cpp

If you would like to post any patches, I'd be happy to get some builds for you.

freaktechnik commented 10 years ago

I am pretty sure these two functions are the devils: https://github.com/nightingale-media-player/nightingale-hacking/blob/sb-trunk-oldxul/app/content/bindings/eqBandSlider.xml#L147-L163

While implementing the EQ presets I noticed this bug in a small extent too. It's very apparent when saving a preset and then immediately applying it.

EDIT: short explanation of what these functions do: they convert a floating point number between -1 and 1 into a number between 0 and 20000, which will be rounded to the next integer later on. EDIT2: I guess the question is, if 10000 in each direction is a sensible scale for equalizer bands on the UI side.