Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CoverArtUtils: Fix Reload From File/Folder, Updates Wrong Cover Art #4909

Merged
merged 2 commits into from
Aug 23, 2022

Conversation

fatihemreyildiz
Copy link
Contributor

This PR aims to solve this bug mentioned on Launchpad.

This simply fixes to wrong cover art update if the names has more than one "." dot. I
track.1.mp3
track.2.mp3
track.3.mp3
track.4.jpg
track.5.jpg

If track.1-track.2-track.3's cover arts updated via "Reload from file/folder" it updates it with track.4.jpg.

With this change, it looks for the complete full name, so track.1.mp3 won't be updated with track.4.jpg.

But with that change, this can change the old behavior, if the files named as;
track.1.mp3.jpg
track.1.mp3
updating wouldn't not update the cover art, to not change the behavior of old updating, there is an another block added. If complete full name is not working, this will look for file name of the track "track.1.mp3" and complete name of the cover art "track.1.mp3".jpg

@fatihemreyildiz fatihemreyildiz changed the title cautils&fileinfo: compBaseName add for cart update CoverArtUtils: Fix Reload From File/Folder, Updates Wrong Cover Art Aug 22, 2022
@daschuer
Copy link
Member

Thank you. This looks like a bug fix for 2.3.

@fatihemreyildiz
Copy link
Contributor Author

Yes, that is happening in 2.3 👍

Copy link
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

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

One missing line.

Can you rebase this on 2.3 than. Thank you.

src/library/coverartutils.cpp Show resolved Hide resolved
@daschuer daschuer changed the base branch from main to 2.3 August 22, 2022 14:39
Copy link
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

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

LGTM

@daschuer
Copy link
Member

Does anyone else want to have a look?

@daschuer
Copy link
Member

Thank you!

@daschuer daschuer merged commit f269f81 into mixxxdj:2.3 Aug 23, 2022
@daschuer
Copy link
Member

Ups, this is now broken in the 2.3 branch: https://github.com/mixxxdj/mixxx/runs/7979410003
I did not realize that the CI was still testing the main merge after changing the merge target.
I will push a fix directly to 2.3

@daschuer
Copy link
Member

517d7a6

@fatihemreyildiz
Copy link
Contributor Author

Thank you for taking care.

I guess the file trackfile.h in 2.3 branch, changed to fileinfo.h in main branch right? I was trying to fix it and added the completeBaseName in file.h in main branch and while re-basing to 2.3 there was no fileinfo.h so I deleted completeBaseName, and compiled successfully.

I just didn't get it why it failed. I hope I didn't do something missing-wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants