The input to the FFT was distorted by the usage of fabs on the samples.
It led to a big DC offset and a distorted spectrum. Simply removing fabs
improves the quality of the spectrum a lot.
The FFT input should be windowed to reduce spectral leakage. This also
improves the visual quality of the spectrum.
Also, no need to do a FFT of the whole buffer if we only mean to render
64 bars. A 8192 point FFT may smooth out fast local changes but at 44100
hz samplerate that's 200 ms worth of sound which significantly reduces
FPS.
A better approach for a fluent visualization is to do small FFTs at the
current playing position inside the current buffer.
There may be a better way to get the current playing position, but for
now it's implemented as an estimation depending on how many frames where
already rendered with the current buffer.
Also I picked y-axis log scale as a default because there's usually a
big difference in energy between low and high frequency bands. log scale
looks nicer.
Visualization widgets should only have to tell how many samples they
need per frame and have a render method which receives all data relevant
to draw the next frame.
Although it's nice to have this as an option, it should be the default
to adjust higher frequencies as they intrinsically have less energy than
lower energies.
Several related improvements to our Fast Fourier Transform
implementation:
- FFT now operates on spans, allowing it to use many more container
types other than Vector. It's intended anyways that FFT transmutes the
input data.
- FFT is now constexpr, moving the implementation to the header and
removing the cpp file. This means that if we have static collections
of samples, we can transform them at compile time.
- sample_data.data() weirdness is now gone.
Display the album cover for the current playing song in the
visualization area for the "None" Visualization.
For now only "cover.png" and "cover.jpg" are looked for in the same
directory for the album cover image.
When no cover image is found the serenity background is shown instead as
a fallback.
This adds a new start_new_file() function to VisualizationWidget which
is called when the player starts a new file, passing the filename of the
file. This allows VisualizationWidget subclasses to do any setup needed
when a new file is started.
When the bars visualization receives a new buffer, it checks if it needs
a new buffer, which is only the case after it has repainted. However,
after then setting m_is_using_last, which is the flag for this, it
checks the buffer size of the passed buffer and returns if that is too
small. This means that if the visualizer receives a buffer that is too
small, and because of external circumstances the update doesn't run
after the buffer modification routine, the m_is_using_last variable is
stuck at true, which means that the visualization incorrectly believes
that the passed buffer is old and we need not update. This simply fixes
that by resetting m_is_using_last if the buffer we're passed is too
small, because in that case, we're clearly not using the last buffer
anymore.
Note: This bug is not exposed by the current SoundPlayer behavior. It
will become an issue with future changes, so we should fix it
regardless.
Enable the warning project-wide. It catches when a non-virtual method
creates an overload set with a virtual method. This might cause
surprising overload resolution depending on how the method is invoked.
Previously, SoundPlayer would read and enqueue samples in the GUI loop
(through a Timer). Apart from general problems with doing audio on the
GUI thread, this is particularly bad as the audio would lag or drop out
when the GUI lags (e.g. window resizes and moves, changing the
visualizer). As Piano does, now SoundPlayer enqueues more audio once the
audio server signals that a buffer has finished playing. The GUI-
dependent decoding is still kept as a "backup" and to start the entire
cycle, but it's not solely depended on. A queue of buffer IDs is used to
keep track of playing buffers and how many there are. The buffer
overhead, i.e. how many buffers "too many" currently exist, is currently
set to its absolute minimum of 2.
This shortcut let us mute/unmute the player, but it still doesn't update
the volume slider because the actual volume widget can't display a muted
state.
This fix syncs up the AudioPlayer's internal state for showing
playlist information with the AudioPlayer's GUI. Before, if the
AudioPlayer was opened with a playlist file (.m3u or .m3u8) it would
automatically show the playlist information in the GUI and set the
loop mode to playlist, but the menu options would be unchecked. In
order to hide the playlist information, the menu option would then
have to be toggled twice -- once on and again off.
Previously, a libc-like out-of-line error information was used in the
loader and its plugins. Now, all functions that may fail to do their job
return some sort of Result. The universally-used error type ist the new
LoaderError, which can contain information about the general error
category (such as file format, I/O, unimplemented features), an error
description, and location information, such as file index or sample
index.
Additionally, the loader plugins try to do as little work as possible in
their constructors. Right after being constructed, a user should call
initialize() and check the errors returned from there. (This is done
transparently by Loader itself.) If a constructor caused an error, the
call to initialize should check and return it immediately.
This opportunity was used to rework a lot of the internal error
propagation in both loader classes, especially FlacLoader. Therefore, a
couple of other refactorings may have sneaked in as well.
The adoption of LibAudio users is minimal. Piano's adoption is not
important, as the code will receive major refactoring in the near future
anyways. SoundPlayer's adoption is also less important, as changes to
refactor it are in the works as well. aplay's adoption is the best and
may serve as an example for other users. It also includes new buffering
behavior.
Buffer also gets some attention, making it OOM-safe and thereby also
propagating its errors to the user.
LibDSP can greatly benefit from this nice FFT implementation, so let's
move it into the fitting library :^)
Note that this now requires linking SoundPlayer against LibDSP. That's
not an issue (LibDSP is rather small currently anyways), as we can
probably make great use of it in the future anyways.
The path returned by GUI:FilePicker is stored on the stack when the
callback is executed. The player only stored a StringView to the path
however it should take ownership of the path instead since the path is
accessed even after the file menu open action has returned.
This fix allows us to move the knob wherever we click inside the slider.
The 'jump_to_cursor()' mechanism wasn't working properly because the
player was overwriting the value we had just clicked.
Derivatives of Core::Object should be constructed through
ClassName::construct(), to avoid handling ref-counted objects with
refcount zero. Fixing the visibility means that misuses like this are
more difficult.
The shuffling algorithm uses a naïve bloom filter to provide random
uniformity, avoiding items that were recently played. With 32 bits,
double hashing, and an error rate of ~10%, this bloom filter should
be able to hold around ~16 keys, which should be sufficient to give the
illusion of fairness to the shuffling algorithm.
This avoids having to shuffle the playlist itself (user might have
spent quite a bit of time to sort them, so it's not a good idea to mess
with it), or having to create a proxy model that shuffles (that could
potentially use quite a bit of memory).
This is a first pass at refactoring SoundPlayer so that the View widget
is decoupled from the player itself.
In doing so, this fixed a couple of issues, including possibly
inconsistent states (e.g. player could be paused and stopped at the
same time).
With the change, Player actually controls the show, and calls methods
overriden by its subclasses to perform actions, such as update the Seek
bar; the hard work of massaging the raw data is done by the Player
class, so subclasses don't need to reimplement any of these things.
This also removes some copies of playlist management code that happened
to be copied+pasted inside callbacks of buttons -- it now lives inside
a neatly packaged Playlist class, and the Player only asks for the next
song to play.
In addition, the menu bar has been slightly rearranged.
Across the entire audio system, audio now works in 0-1 terms instead of
0-100 as before. Therefore, volume is now a double instead of an int.
The master volume of the AudioServer changes smoothly through a
FadingProperty, preventing clicks. Finally, volume computations are done
with logarithmic scaling, which is more natural for the human ear.
Note that this could be 4-5 different commits, but as they change each
other's code all the time, it makes no sense to split them up.
freq_bin was converted to double after it was calculated, so there was
a much higher probability it could be 0 instead of some comma number,
which meant that the bars always stayed on top.
The freq_bin in bins_per_group was multiplied only to be divided later,
which could even result in a crash if you set higher buffer size
(like 1000ms) in PlaybackManager, due to rounding errors I presume.
Prior this change, opening a playlist always spawned a new widget.
This could end up with having a few the same widgets, which you couldn't
even close (besides the last one).
Removed the old custom checkbox selection code in the Visualization
menu and replaced them with GUI::ActionGroup with set_exclusive
enabled instead :^)
1) The Sound Player visualizer couldn't deal with small sample buffers,
which occur on low sample rates. Now, it simply doesn't update its
buffer, meaning the display is broken on low sample rates. I'm not too
familiar with the visualizer to figure out a proper fix for now, but
this mitigates the issue (and "normal" sample rates still work).
2) Piano wouldn't buffer enough samples for small sample rates, so the
sample count per buffer is now increased to 2^12, introducing minor
amounts of (acceptable) lag.
All audio applications (aplay, Piano, Sound Player) respect the ability
of the system to have theoretically any sample rate. Therefore, they
resample their own audio into the system sample rate.
LibAudio previously had its loaders resample their own audio, even
though they expose their sample rate. This is now changed. The loaders
output audio data in their file's sample rate, which the user has to
query and resample appropriately. Resampling code from Buffer, WavLoader
and FlacLoader is removed.
Note that these applications only check the sample rate at startup,
which is reasonable (the user has to restart applications when changing
the sample rate). Fully dynamic adaptation could both lead to errors and
will require another IPC interface. This seems to be enough for now.
Most of the models were just calling did_update anyway, which is
pointless since it can be unified to the base Model class. Instead, code
calling update() will now call invalidate(), which functions identically
and is more obvious in what it does.
Additionally, a default implementation is provided, which removes the
need to add empty implementations of update() for each model subclass.
Co-Authored-By: Ali Mohammad Pur <ali.mpfard@gmail.com>
Applications previously had to create a GUI::Menubar object, add menus
to it, and then call GUI::Window::set_menubar().
This patch introduces GUI::Window::add_menu() which creates the menubar
automatically and adds items to it. Application code becomes slightly
simpler as a result. :^)
