Differential D20879

do not forget the album cover when using embedded cover images
ClosedPublic
Actions

Authored by mgallien on Apr 28 2019, 2:53 PM.

Details

Reviewers
ngraham
Commits
R255:9fd1e7bcafa1: do not forget the album cover when using embedded cover images
Summary

Use embedded cover images when inserting an album in database allowing tracks and albums to use the right cover image

Currently require a reimport of database. Will fix that in a later revision.

CCBUG: 393486
BUG: 406482

Test Plan

Auto tests are OK. Attached test files show an embedded cover image.

Diff Detail

Repository
R255 Elisa
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
mgallien requested review of this revision.Apr 28 2019, 2:53 PM
mgallien created this revision.
mgallien added a reviewer: ngraham.Apr 28 2019, 2:55 PM
mgallien added a project: Elisa.
ngraham accepted this revision.Apr 28 2019, 3:01 PM

Yep, confirmed that this fixes the issue for all of my albums that lacked art, after re-importing the library. Thanks!

This revision is now accepted and ready to land.Apr 28 2019, 3:01 PM
mgallien added a comment.Apr 28 2019, 3:04 PM
In D20879#457489, @ngraham wrote:

Yep, confirmed that this fixes the issue for all of my albums that lacked art, after re-importing the library. Thanks!

Yes but I think that it also needs to be fixed without re-importing. What do you think ?

mgallien added a comment.Apr 28 2019, 3:05 PM
In D20879#457489, @ngraham wrote:

Yep, confirmed that this fixes the issue for all of my albums that lacked art, after re-importing the library. Thanks!

Thanks for the test ;)

ngraham added a comment.Apr 29 2019, 4:33 PM

It would be nice to fix it without re-importing, yes. If that's necessary, it should happen automatically. Does that already happen for people who upgrade from one stable release to another?

mgallien updated this revision to Diff 57273.Apr 30 2019, 3:16 PM
  • allow proper load of embedded cover even when database has wrong data

allow proper display of embedded cover images even without a fixed database

mgallien retitled this revision from [WIP] do not forget the album cover when using embedded cover images to do not forget the album cover when using embedded cover images.Apr 30 2019, 3:17 PM
mgallien added a comment.Apr 30 2019, 9:50 PM
In D20879#458165, @ngraham wrote:

It would be nice to fix it without re-importing, yes. If that's necessary, it should happen automatically. Does that already happen for people who upgrade from one stable release to another?

I have pushed another fix that allows to display the embedded cover without database changes. I am testing upgrades from last stable version (currently the upgrade is broken (logic given I have not added automated tests)).

ngraham added a comment.May 1 2019, 2:19 AM

Out of curiosity where on disk is the database located, and is it safe to blow away for testing purposes?

mgallien added a comment.May 1 2019, 2:27 PM
In D20879#458857, @ngraham wrote:

Out of curiosity where on disk is the database located, and is it safe to blow away for testing purposes?

It is located in $HOME/.local/share/elisa/elisaDatabase.db .
You can wipe it out but you will lose your statistics (play count, first and last play date).
I definitely need to look again at a way to store those data in the music files (and push support in KFileMetaData).

ngraham accepted this revision.May 2 2019, 9:56 PM
Closed by commit R255:9fd1e7bcafa1: do not forget the album cover when using embedded cover images (authored by mgallien). · Explain WhyMay 3 2019, 5:06 AM
This revision was automatically updated to reflect the committed changes.
ngraham added a comment.May 3 2019, 1:43 PM

So much better now!

Revision Contents
Changeset List

PathPackages
Msrc/databaseinterface.cpp (201 lines)

Diff 57435

View Options

src/databaseinterface.cpp

Loading...