Skip to content

SoundPlayer: Show album cover in the Album Cover visualization#12841

Merged
bgianfo merged 5 commits intoSerenityOS:masterfrom
nfraprado:soundplayer-cover
Mar 3, 2022
Merged

SoundPlayer: Show album cover in the Album Cover visualization#12841
bgianfo merged 5 commits intoSerenityOS:masterfrom
nfraprado:soundplayer-cover

Conversation

@nfraprado
Copy link
Copy Markdown
Contributor

This PR renames the "None" visualization to "Album Cover" and shows the album cover for the current loaded sound file there.

This is just the first small step, and simply looks for a file named "cover.png" or "cover.jpg" in the same directory as the sound file, since this is common practice. In the future we'd want to also try loading the image embedded in the sound file and only use the "cover.png" file 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.
Comment thread Userland/Applications/SoundPlayer/CMakeLists.txt Outdated
Comment thread Userland/Applications/SoundPlayer/AlbumCoverVisualizationWidget.cpp Outdated
Comment thread Userland/Applications/SoundPlayer/main.cpp Outdated
Copy link
Copy Markdown
Member

@bgianfo bgianfo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just a few minor nit picks.

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.
Since the NoVisualization widget now shows the album cover, it should be
called AlbumCoverVisualization instead.
Instead of drawing the album cover scaled to cover the whole
visualization area, draw it resized to fit the area without altering the
aspect ratio.
@nfraprado nfraprado force-pushed the soundplayer-cover branch from b2b6238 to 4fc9fc4 Compare March 3, 2022 03:23
@nfraprado
Copy link
Copy Markdown
Contributor Author

@bgianfo All feedback addressed, thanks for the review!

@bgianfo bgianfo merged commit cb2e187 into SerenityOS:master Mar 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants