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

lp1955314: Preserve file creation time when exporting track metadata (Windows) #4572

Closed
wants to merge 2 commits into from
Closed

lp1955314: Preserve file creation time when exporting track metadata (Windows) #4572

wants to merge 2 commits into from

Conversation

uklotzde
Copy link
Contributor

@uklotzde uklotzde commented Dec 18, 2021

@daschuer
Copy link
Member

There seems to be a file locking issue on windows that prevents to write the metadata at all testing wit plain 2.3.1:

Debug [Main]: MetadataSourceTagLib - Exporting track metadata into file "C:/Users/dasch/Music/Bata Illic - Ich möcht der Knopf an Deiner Bluse sein.mp3" with type 3
Critical [Main]: MetadataSourceTagLib - "Die Quelldatei kann nicht entfernt werden" - Failed to rename the original file for backup before writing: "C:/Users/dasch/Music/Bata Illic - Ich möcht der Knopf an Deiner Bluse sein.mp3" -> "C:/Users/dasch/Music/Bata Illic - Ich möcht der Knopf an Deiner Bluse sein.mp3_orig"
Warning [Main]: MetadataSourceTagLib - Failed to save tags of file "C:/Users/dasch/Music/Bata Illic - Ich möcht der Knopf an Deiner Bluse sein.mp3"
Warning [Main]: Track - Failed to export track metadata: "C:/Users/dasch/Music/Bata Illic - Ich möcht der Knopf an Deiner Bluse sein.mp3"
Debug [Main]: TrackCollectionManager - Saving track "C:/Users/dasch/Music/Bata Illic - Ich möcht der Knopf an Deiner Bluse sein.mp3" in internal collection
Debug [Main]: TrackDAO: Saving track 3 QFileInfo(C:\Users\dasch\Music\Bata Illic - Ich möcht der Knopf an Deiner Bluse sein.mp3)
Debug [Main]: TrackDAO: Updating track in database 3 QFileInfo(C:\Users\dasch\Music\Bata Illic - Ich möcht der Knopf an Deiner Bluse sein.mp3)
Debug [Main]: BaseTrackCache(0x1e30431cf30) updateIndexWithQuery took 0 ms

My first Idea was that this this is because of a looking up the file properties in windows explorer? But this also happens when I do not touch the file at all with other applications than Mixxx. It also happens after a fresh restart of Windows.
It is a Windows 10 Runnning in a virtual Box.

Interestingly the renaming works after I have loaded the track into a deck and edit the metadata while paying.

I cant confirm the original issue though. The file creation time is preserved reliable when the export works:

This is the test result without this PR (I have not even downloaded it yet):
grafik

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.

I cannot reproduce the issue.

@uklotzde
Copy link
Contributor Author

https://doc.qt.io/qt-5/qfile.html#copy

"With the exception of permissions, which are copied, no other file metadata is copied."

@uklotzde
Copy link
Contributor Author

Force pushed with a reference to the Qt docs. If you still think this is not necessary I will close the PR.

@daschuer
Copy link
Member

If you still think this is not necessary

It seems to be necessary on Paolo's machine, but not on mine.

Under the hood Qt uses ::CopyFile2
https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-copyfile2
Without a clear info what happens with the file times.

The the code of this PR sounds reasonable.

Let's wait for the confirmation from Paolo that the issue is fixed.

@uklotzde
Copy link
Contributor Author

Behavior might differ depending on OS platform and version and the file system. We have to follow the Qt specs.

@daschuer
Copy link
Member

I am sorry, but unfortunately this code is not able to preserve the creation time on Paolo's system.
https://mixxx.discourse.group/t/updating-id3-creates-new-file-instead-of-modyfing/23814/29

@daschuer
Copy link
Member

This error message:
Critical [Main]: MetadataSourceTagLib - "Die Quelldatei kann nicht entfernt werden" - Failed to rename the original file for backup before writing: "C:/Users/dasch/Music/Bata Illic - Ich möcht der Knopf an Deiner Bluse sein.mp3" -> "C:/Users/dasch/Music/Bata Illic - Ich möcht der Knopf an Deiner Bluse sein.mp3_orig"

Originates from here:
https://github.com/qt/qtbase/blob/40143c189b7c1bf3c2058b77d00ea5c4e3be8b28/src/corelib/io/qfile.cpp#L738

So we are in the alternative path where Qt replaces a move rename by a copy rename. Described here: https://doc.qt.io/qt-5/qfile.html#rename If this kicks in, the file creation time is also lost. So you may try to store the file creation time in the beginning and restore it ant the very end. Hopefully that will fix Paolo issue.

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.

None yet

2 participants