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

Write audio tags back into files #728

Merged
merged 57 commits into from Dec 10, 2017

Conversation

Projects
None yet
@uklotzde
Contributor

uklotzde commented Oct 3, 2015

Re-enables writing of metadata into files:
https://bugs.launchpad.net/mixxx/+bug/728197

Related bugs:
https://bugs.launchpad.net/mixxx/+bug/1156868
https://bugs.launchpad.net/mixxx/+bug/687293
https://bugs.launchpad.net/mixxx/+bug/1181869

Contains lots of fixes around TrackInfoObject including caching of TIOs to safely manage all references of a file. I'm already using a version of Mixxx built from this branch for myself. It has proven to be very stable and I only discovered and fixed some rare edge cases (like hide + purge + rescan while a track is still cached) recently.

New preferences for "Library" in section "Audio File Tags"

  • "Write modified track meta-data from library into files as audio tags"
  • "Update track meta-data in library by reloading audio tags from files" 2016-06-24: Considered experimental and moved into another branch. Has some nasty side-effects when selecting and loading multiple tracks at once.

The 2nd option should only be enabled after all files are in sync with your Mixxx library! Otherwise the information stored in your Mixxx library gets updated/overwritten whenever Mixxx accesses a file. A warning message will appear if you enable this option to prevent users from unintentionally selecting it. 2016-06-24: Considered experimental and moved into another branch. Has some nasty side-effects when selecting and loading multiple tracks at once.

New context menu item (mainly for migration purposes):

  • "Save Metadata into File"

What's missing (optional):

  • Add a separate dirty flag for track metadata
  • Use QSaveFile from Qt5

I've finished to separate my modifications into commits that are hopefully reviewable now. The changes are numerous! Even if the refactoring is not completely finished I'm already very happy with the end result. Now I can use Mixxx to manage my music library except when I need to edit the properties of multiple tracks at once.

Note
I will rebase and edit this branch frequently until we agree that it is mergeable ;) That's the reason for tagging this PR with [PREVIEW]

@ferranpujolcamins

This comment has been minimized.

Show comment
Hide comment
@ferranpujolcamins

ferranpujolcamins Oct 3, 2015

Contributor

Nice nice nice! Thank you.
Any specific things to test?

2015-10-03 17:59 GMT+02:00 Uwe Klotz notifications@github.com:

Re-enables writing of metadata into files:
https://bugs.launchpad.net/mixxx/+bug/728197

Related bugs:
https://bugs.launchpad.net/mixxx/+bug/1156868
https://bugs.launchpad.net/mixxx/+bug/687293
https://bugs.launchpad.net/mixxx/+bug/1181869

This PR is intended as a preview and for testing purposes. Please note
that I will rebase the corresponding branch on master frequently!

Contains lots of fixes around TrackInfoObject including caching of TIOs to
safely manage all references of a file. I'm already using a version of
Mixxx built from this branch for myself. It has proven to be very stable
and I only discovered and fixed some rare edge cases (like hide + purge +
rescan while a track is still cached) recently.

What's missing (optional):

  • Add a separate dirty flag for track metadata
  • Use QSaveFile from Qt5

You can view, comment on, or merge this pull request online at:

#728
Commit Summary

  • Delete redundant constructor
  • Initialize members directly and reuse initialize()
  • Delete redundant QFileInfo instance
  • Make the track location immutable
  • Delete unnecessary destructor
  • Fix signature of operator<< for QDebug
  • Log full track location instead of only the file name
  • Rename file name/size functions in TrackInfoObject
  • Exclude certain columns from editing in table model
  • Delete outdated comment
  • Add more comments about thread-safety of QFileInfo
  • Get rid of LegacyLibraryImporter
  • Move code from initialize() into constructor
  • Delete obsolete file headers and redundant #include directives
  • Reorder member functions in class definition
  • Remove TrackPointer from cover art widgets
  • Merge branch 'master' of https://github.com/mixxxdj/mixxx.git into
    TrackLocationImmutable
  • Move code for obtaining the global key as text to KeyUtils
  • Merge branch 'master' into TrackLocationImmutable
  • Wrap times played and played flag into PlayCounter
  • Replace QScopedPointer with std::unique_ptr
  • Add URL field to track metadata
  • Add operator ==/!= for track metadata
  • Add functions for normalizing floating-point values to track metadata
  • Add function for formatting duration values to track metadata
  • Require a TIO for creating a SoundSourceProxy instance
  • Use QSharedPointer for cue points
  • Rename functions in SoundSourceProviderRegistry
  • Reorder functions in SoundSourceProxy (static before member)
  • Reorder fields in track metadata
  • Keep TIO alive while reading audio data
  • Simplify handling of duration in TIO
  • Reduce visibility of copy constructor
  • Rename BPM functions and apply some refactoring
  • Add factory methods for creating temporary TIOs
  • Reorder and functions in TIO
  • Rename functions for loading metadata and cover art
  • Remove obsolete security token from SoundSourceProxy
  • Parse track metadata and cover art through SoundSourceProxy
  • Fix handling of musical key in TIO
  • Change replay gain type from float to double
  • Change management of dirty flag
  • Encapsulate location and id of tracks in TrackRef
  • Drop obsolete class AudioTagger
  • Add overridable function for writing track metadata into file
  • Rename functions in TrackDAO
  • Aggregated commit ...will be broken down into multiple PRs

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#728.

Contributor

ferranpujolcamins commented Oct 3, 2015

Nice nice nice! Thank you.
Any specific things to test?

2015-10-03 17:59 GMT+02:00 Uwe Klotz notifications@github.com:

Re-enables writing of metadata into files:
https://bugs.launchpad.net/mixxx/+bug/728197

Related bugs:
https://bugs.launchpad.net/mixxx/+bug/1156868
https://bugs.launchpad.net/mixxx/+bug/687293
https://bugs.launchpad.net/mixxx/+bug/1181869

This PR is intended as a preview and for testing purposes. Please note
that I will rebase the corresponding branch on master frequently!

Contains lots of fixes around TrackInfoObject including caching of TIOs to
safely manage all references of a file. I'm already using a version of
Mixxx built from this branch for myself. It has proven to be very stable
and I only discovered and fixed some rare edge cases (like hide + purge +
rescan while a track is still cached) recently.

What's missing (optional):

  • Add a separate dirty flag for track metadata
  • Use QSaveFile from Qt5

You can view, comment on, or merge this pull request online at:

#728
Commit Summary

  • Delete redundant constructor
  • Initialize members directly and reuse initialize()
  • Delete redundant QFileInfo instance
  • Make the track location immutable
  • Delete unnecessary destructor
  • Fix signature of operator<< for QDebug
  • Log full track location instead of only the file name
  • Rename file name/size functions in TrackInfoObject
  • Exclude certain columns from editing in table model
  • Delete outdated comment
  • Add more comments about thread-safety of QFileInfo
  • Get rid of LegacyLibraryImporter
  • Move code from initialize() into constructor
  • Delete obsolete file headers and redundant #include directives
  • Reorder member functions in class definition
  • Remove TrackPointer from cover art widgets
  • Merge branch 'master' of https://github.com/mixxxdj/mixxx.git into
    TrackLocationImmutable
  • Move code for obtaining the global key as text to KeyUtils
  • Merge branch 'master' into TrackLocationImmutable
  • Wrap times played and played flag into PlayCounter
  • Replace QScopedPointer with std::unique_ptr
  • Add URL field to track metadata
  • Add operator ==/!= for track metadata
  • Add functions for normalizing floating-point values to track metadata
  • Add function for formatting duration values to track metadata
  • Require a TIO for creating a SoundSourceProxy instance
  • Use QSharedPointer for cue points
  • Rename functions in SoundSourceProviderRegistry
  • Reorder functions in SoundSourceProxy (static before member)
  • Reorder fields in track metadata
  • Keep TIO alive while reading audio data
  • Simplify handling of duration in TIO
  • Reduce visibility of copy constructor
  • Rename BPM functions and apply some refactoring
  • Add factory methods for creating temporary TIOs
  • Reorder and functions in TIO
  • Rename functions for loading metadata and cover art
  • Remove obsolete security token from SoundSourceProxy
  • Parse track metadata and cover art through SoundSourceProxy
  • Fix handling of musical key in TIO
  • Change replay gain type from float to double
  • Change management of dirty flag
  • Encapsulate location and id of tracks in TrackRef
  • Drop obsolete class AudioTagger
  • Add overridable function for writing track metadata into file
  • Rename functions in TrackDAO
  • Aggregated commit ...will be broken down into multiple PRs

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#728.

@uklotzde

This comment has been minimized.

Show comment
Hide comment
@uklotzde

uklotzde Oct 3, 2015

Contributor

Nothing particular to test.

The code contains a lot of DEBUG_ASSERTs that are disabled in release
builds. It would be helpful if you use start playing with a debug build by
using the following scons options:
qdebug=1
debug_assertions_fatal=1

With these options Mixxx should crash when an assert condition is violated.
Also good for collecting stack traces. I'm interested in both of them ;)

Please have a look at the new Library options in the section Audio File
Tags. Use the 2nd option ONLY if the metadata in your files is the master
and the Mixxx library just serves as a fast, searchable cache that gets
updated from your files. This is the way I work.

On 10/03/2015 06:09 PM, Ferran Pujol Camins wrote:

Nice nice nice! Thank you.
Any specific things to test?

2015-10-03 17:59 GMT+02:00 Uwe Klotz notifications@github.com:

Re-enables writing of metadata into files:
https://bugs.launchpad.net/mixxx/+bug/728197

Related bugs:
https://bugs.launchpad.net/mixxx/+bug/1156868
https://bugs.launchpad.net/mixxx/+bug/687293
https://bugs.launchpad.net/mixxx/+bug/1181869

This PR is intended as a preview and for testing purposes. Please note
that I will rebase the corresponding branch on master frequently!

Contains lots of fixes around TrackInfoObject including caching of TIOs to
safely manage all references of a file. I'm already using a version of
Mixxx built from this branch for myself. It has proven to be very stable
and I only discovered and fixed some rare edge cases (like hide + purge +
rescan while a track is still cached) recently.

What's missing (optional):

  • Add a separate dirty flag for track metadata
  • Use QSaveFile from Qt5

You can view, comment on, or merge this pull request online at:

#728
Commit Summary

  • Delete redundant constructor
  • Initialize members directly and reuse initialize()
  • Delete redundant QFileInfo instance
  • Make the track location immutable
  • Delete unnecessary destructor
  • Fix signature of operator<< for QDebug
  • Log full track location instead of only the file name
  • Rename file name/size functions in TrackInfoObject
  • Exclude certain columns from editing in table model
  • Delete outdated comment
  • Add more comments about thread-safety of QFileInfo
  • Get rid of LegacyLibraryImporter
  • Move code from initialize() into constructor
  • Delete obsolete file headers and redundant #include directives
  • Reorder member functions in class definition
  • Remove TrackPointer from cover art widgets
  • Merge branch 'master' of https://github.com/mixxxdj/mixxx.git into
    TrackLocationImmutable
  • Move code for obtaining the global key as text to KeyUtils
  • Merge branch 'master' into TrackLocationImmutable
  • Wrap times played and played flag into PlayCounter
  • Replace QScopedPointer with std::unique_ptr
  • Add URL field to track metadata
  • Add operator ==/!= for track metadata
  • Add functions for normalizing floating-point values to track metadata
  • Add function for formatting duration values to track metadata
  • Require a TIO for creating a SoundSourceProxy instance
  • Use QSharedPointer for cue points
  • Rename functions in SoundSourceProviderRegistry
  • Reorder functions in SoundSourceProxy (static before member)
  • Reorder fields in track metadata
  • Keep TIO alive while reading audio data
  • Simplify handling of duration in TIO
  • Reduce visibility of copy constructor
  • Rename BPM functions and apply some refactoring
  • Add factory methods for creating temporary TIOs
  • Reorder and functions in TIO
  • Rename functions for loading metadata and cover art
  • Remove obsolete security token from SoundSourceProxy
  • Parse track metadata and cover art through SoundSourceProxy
  • Fix handling of musical key in TIO
  • Change replay gain type from float to double
  • Change management of dirty flag
  • Encapsulate location and id of tracks in TrackRef
  • Drop obsolete class AudioTagger
  • Add overridable function for writing track metadata into file
  • Rename functions in TrackDAO
  • Aggregated commit ...will be broken down into multiple PRs

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#728.


Reply to this email directly or view it on GitHub
#728 (comment).

Contributor

uklotzde commented Oct 3, 2015

Nothing particular to test.

The code contains a lot of DEBUG_ASSERTs that are disabled in release
builds. It would be helpful if you use start playing with a debug build by
using the following scons options:
qdebug=1
debug_assertions_fatal=1

With these options Mixxx should crash when an assert condition is violated.
Also good for collecting stack traces. I'm interested in both of them ;)

Please have a look at the new Library options in the section Audio File
Tags. Use the 2nd option ONLY if the metadata in your files is the master
and the Mixxx library just serves as a fast, searchable cache that gets
updated from your files. This is the way I work.

On 10/03/2015 06:09 PM, Ferran Pujol Camins wrote:

Nice nice nice! Thank you.
Any specific things to test?

2015-10-03 17:59 GMT+02:00 Uwe Klotz notifications@github.com:

Re-enables writing of metadata into files:
https://bugs.launchpad.net/mixxx/+bug/728197

Related bugs:
https://bugs.launchpad.net/mixxx/+bug/1156868
https://bugs.launchpad.net/mixxx/+bug/687293
https://bugs.launchpad.net/mixxx/+bug/1181869

This PR is intended as a preview and for testing purposes. Please note
that I will rebase the corresponding branch on master frequently!

Contains lots of fixes around TrackInfoObject including caching of TIOs to
safely manage all references of a file. I'm already using a version of
Mixxx built from this branch for myself. It has proven to be very stable
and I only discovered and fixed some rare edge cases (like hide + purge +
rescan while a track is still cached) recently.

What's missing (optional):

  • Add a separate dirty flag for track metadata
  • Use QSaveFile from Qt5

You can view, comment on, or merge this pull request online at:

#728
Commit Summary

  • Delete redundant constructor
  • Initialize members directly and reuse initialize()
  • Delete redundant QFileInfo instance
  • Make the track location immutable
  • Delete unnecessary destructor
  • Fix signature of operator<< for QDebug
  • Log full track location instead of only the file name
  • Rename file name/size functions in TrackInfoObject
  • Exclude certain columns from editing in table model
  • Delete outdated comment
  • Add more comments about thread-safety of QFileInfo
  • Get rid of LegacyLibraryImporter
  • Move code from initialize() into constructor
  • Delete obsolete file headers and redundant #include directives
  • Reorder member functions in class definition
  • Remove TrackPointer from cover art widgets
  • Merge branch 'master' of https://github.com/mixxxdj/mixxx.git into
    TrackLocationImmutable
  • Move code for obtaining the global key as text to KeyUtils
  • Merge branch 'master' into TrackLocationImmutable
  • Wrap times played and played flag into PlayCounter
  • Replace QScopedPointer with std::unique_ptr
  • Add URL field to track metadata
  • Add operator ==/!= for track metadata
  • Add functions for normalizing floating-point values to track metadata
  • Add function for formatting duration values to track metadata
  • Require a TIO for creating a SoundSourceProxy instance
  • Use QSharedPointer for cue points
  • Rename functions in SoundSourceProviderRegistry
  • Reorder functions in SoundSourceProxy (static before member)
  • Reorder fields in track metadata
  • Keep TIO alive while reading audio data
  • Simplify handling of duration in TIO
  • Reduce visibility of copy constructor
  • Rename BPM functions and apply some refactoring
  • Add factory methods for creating temporary TIOs
  • Reorder and functions in TIO
  • Rename functions for loading metadata and cover art
  • Remove obsolete security token from SoundSourceProxy
  • Parse track metadata and cover art through SoundSourceProxy
  • Fix handling of musical key in TIO
  • Change replay gain type from float to double
  • Change management of dirty flag
  • Encapsulate location and id of tracks in TrackRef
  • Drop obsolete class AudioTagger
  • Add overridable function for writing track metadata into file
  • Rename functions in TrackDAO
  • Aggregated commit ...will be broken down into multiple PRs

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#728.


Reply to this email directly or view it on GitHub
#728 (comment).

@uklotzde

This comment has been minimized.

Show comment
Hide comment
@uklotzde

uklotzde Oct 3, 2015

Contributor

...and there is a new context menu option "Write Metadata into File" that
allows to write the track metadata from your Mixxx library into your files.
This is mainly intended for migration purposes after writing tags into files
has been disabled for a long time. For most users the information stored in
their Mixxx library will be newer than what is stored in their files.

On 10/03/2015 06:09 PM, Ferran Pujol Camins wrote:

Nice nice nice! Thank you.
Any specific things to test?

2015-10-03 17:59 GMT+02:00 Uwe Klotz notifications@github.com:

Re-enables writing of metadata into files:
https://bugs.launchpad.net/mixxx/+bug/728197

Related bugs:
https://bugs.launchpad.net/mixxx/+bug/1156868
https://bugs.launchpad.net/mixxx/+bug/687293
https://bugs.launchpad.net/mixxx/+bug/1181869

This PR is intended as a preview and for testing purposes. Please note
that I will rebase the corresponding branch on master frequently!

Contains lots of fixes around TrackInfoObject including caching of TIOs to
safely manage all references of a file. I'm already using a version of
Mixxx built from this branch for myself. It has proven to be very stable
and I only discovered and fixed some rare edge cases (like hide + purge +
rescan while a track is still cached) recently.

What's missing (optional):

  • Add a separate dirty flag for track metadata
  • Use QSaveFile from Qt5

You can view, comment on, or merge this pull request online at:

#728
Commit Summary

  • Delete redundant constructor
  • Initialize members directly and reuse initialize()
  • Delete redundant QFileInfo instance
  • Make the track location immutable
  • Delete unnecessary destructor
  • Fix signature of operator<< for QDebug
  • Log full track location instead of only the file name
  • Rename file name/size functions in TrackInfoObject
  • Exclude certain columns from editing in table model
  • Delete outdated comment
  • Add more comments about thread-safety of QFileInfo
  • Get rid of LegacyLibraryImporter
  • Move code from initialize() into constructor
  • Delete obsolete file headers and redundant #include directives
  • Reorder member functions in class definition
  • Remove TrackPointer from cover art widgets
  • Merge branch 'master' of https://github.com/mixxxdj/mixxx.git into
    TrackLocationImmutable
  • Move code for obtaining the global key as text to KeyUtils
  • Merge branch 'master' into TrackLocationImmutable
  • Wrap times played and played flag into PlayCounter
  • Replace QScopedPointer with std::unique_ptr
  • Add URL field to track metadata
  • Add operator ==/!= for track metadata
  • Add functions for normalizing floating-point values to track metadata
  • Add function for formatting duration values to track metadata
  • Require a TIO for creating a SoundSourceProxy instance
  • Use QSharedPointer for cue points
  • Rename functions in SoundSourceProviderRegistry
  • Reorder functions in SoundSourceProxy (static before member)
  • Reorder fields in track metadata
  • Keep TIO alive while reading audio data
  • Simplify handling of duration in TIO
  • Reduce visibility of copy constructor
  • Rename BPM functions and apply some refactoring
  • Add factory methods for creating temporary TIOs
  • Reorder and functions in TIO
  • Rename functions for loading metadata and cover art
  • Remove obsolete security token from SoundSourceProxy
  • Parse track metadata and cover art through SoundSourceProxy
  • Fix handling of musical key in TIO
  • Change replay gain type from float to double
  • Change management of dirty flag
  • Encapsulate location and id of tracks in TrackRef
  • Drop obsolete class AudioTagger
  • Add overridable function for writing track metadata into file
  • Rename functions in TrackDAO
  • Aggregated commit ...will be broken down into multiple PRs

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#728.


Reply to this email directly or view it on GitHub
#728 (comment).

Contributor

uklotzde commented Oct 3, 2015

...and there is a new context menu option "Write Metadata into File" that
allows to write the track metadata from your Mixxx library into your files.
This is mainly intended for migration purposes after writing tags into files
has been disabled for a long time. For most users the information stored in
their Mixxx library will be newer than what is stored in their files.

On 10/03/2015 06:09 PM, Ferran Pujol Camins wrote:

Nice nice nice! Thank you.
Any specific things to test?

2015-10-03 17:59 GMT+02:00 Uwe Klotz notifications@github.com:

Re-enables writing of metadata into files:
https://bugs.launchpad.net/mixxx/+bug/728197

Related bugs:
https://bugs.launchpad.net/mixxx/+bug/1156868
https://bugs.launchpad.net/mixxx/+bug/687293
https://bugs.launchpad.net/mixxx/+bug/1181869

This PR is intended as a preview and for testing purposes. Please note
that I will rebase the corresponding branch on master frequently!

Contains lots of fixes around TrackInfoObject including caching of TIOs to
safely manage all references of a file. I'm already using a version of
Mixxx built from this branch for myself. It has proven to be very stable
and I only discovered and fixed some rare edge cases (like hide + purge +
rescan while a track is still cached) recently.

What's missing (optional):

  • Add a separate dirty flag for track metadata
  • Use QSaveFile from Qt5

You can view, comment on, or merge this pull request online at:

#728
Commit Summary

  • Delete redundant constructor
  • Initialize members directly and reuse initialize()
  • Delete redundant QFileInfo instance
  • Make the track location immutable
  • Delete unnecessary destructor
  • Fix signature of operator<< for QDebug
  • Log full track location instead of only the file name
  • Rename file name/size functions in TrackInfoObject
  • Exclude certain columns from editing in table model
  • Delete outdated comment
  • Add more comments about thread-safety of QFileInfo
  • Get rid of LegacyLibraryImporter
  • Move code from initialize() into constructor
  • Delete obsolete file headers and redundant #include directives
  • Reorder member functions in class definition
  • Remove TrackPointer from cover art widgets
  • Merge branch 'master' of https://github.com/mixxxdj/mixxx.git into
    TrackLocationImmutable
  • Move code for obtaining the global key as text to KeyUtils
  • Merge branch 'master' into TrackLocationImmutable
  • Wrap times played and played flag into PlayCounter
  • Replace QScopedPointer with std::unique_ptr
  • Add URL field to track metadata
  • Add operator ==/!= for track metadata
  • Add functions for normalizing floating-point values to track metadata
  • Add function for formatting duration values to track metadata
  • Require a TIO for creating a SoundSourceProxy instance
  • Use QSharedPointer for cue points
  • Rename functions in SoundSourceProviderRegistry
  • Reorder functions in SoundSourceProxy (static before member)
  • Reorder fields in track metadata
  • Keep TIO alive while reading audio data
  • Simplify handling of duration in TIO
  • Reduce visibility of copy constructor
  • Rename BPM functions and apply some refactoring
  • Add factory methods for creating temporary TIOs
  • Reorder and functions in TIO
  • Rename functions for loading metadata and cover art
  • Remove obsolete security token from SoundSourceProxy
  • Parse track metadata and cover art through SoundSourceProxy
  • Fix handling of musical key in TIO
  • Change replay gain type from float to double
  • Change management of dirty flag
  • Encapsulate location and id of tracks in TrackRef
  • Drop obsolete class AudioTagger
  • Add overridable function for writing track metadata into file
  • Rename functions in TrackDAO
  • Aggregated commit ...will be broken down into multiple PRs

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#728.


Reply to this email directly or view it on GitHub
#728 (comment).

@uklotzde uklotzde changed the title from [PREVIEW] Write audio tags to Write audio tags Oct 13, 2015

Show outdated Hide outdated src/library/browse/browsetablemodel.h Outdated
Show outdated Hide outdated src/metadata/bpm.cpp Outdated
Show outdated Hide outdated src/metadata/bpm.cpp Outdated
Show outdated Hide outdated src/trackinfocache.cpp Outdated
@Alibaobao

This comment has been minimized.

Show comment
Hide comment
@Alibaobao

Alibaobao Feb 20, 2016

hey im a complete noob about all these branches, compiling and stuff
how can i compile a version of mixxx with your changes ?

Alibaobao commented Feb 20, 2016

hey im a complete noob about all these branches, compiling and stuff
how can i compile a version of mixxx with your changes ?

@uklotzde

This comment has been minimized.

Show comment
Hide comment
@uklotzde

uklotzde Feb 20, 2016

Contributor

You definitely need some knowledge about how to set up a development
environment for your OS to compile Mixxx. It is explained in the Wiki:

http://mixxx.org/wiki/doku.php/start#developer_documentation

If you don't want to do any development yourself you can simply clone my
Mixxx repository from GitHub:

git clone https://github.com/uklotzde/mixxx.git
git checkout WriteAudioTags

...and then start the build as explained in the Wiki.

On 02/20/2016 01:25 PM, Alibaobao wrote:

hey im a complete noob about all these branches, compiling and stuff
how can i compile a version of mixxx with your changes ?


Reply to this email directly or view it on GitHub
#728 (comment).

Contributor

uklotzde commented Feb 20, 2016

You definitely need some knowledge about how to set up a development
environment for your OS to compile Mixxx. It is explained in the Wiki:

http://mixxx.org/wiki/doku.php/start#developer_documentation

If you don't want to do any development yourself you can simply clone my
Mixxx repository from GitHub:

git clone https://github.com/uklotzde/mixxx.git
git checkout WriteAudioTags

...and then start the build as explained in the Wiki.

On 02/20/2016 01:25 PM, Alibaobao wrote:

hey im a complete noob about all these branches, compiling and stuff
how can i compile a version of mixxx with your changes ?


Reply to this email directly or view it on GitHub
#728 (comment).

@Alibaobao

This comment has been minimized.

Show comment
Hide comment
@Alibaobao

Alibaobao Feb 20, 2016

thanks for your work and help
it works very nice

Alibaobao commented Feb 20, 2016

thanks for your work and help
it works very nice

@uklotzde

This comment has been minimized.

Show comment
Hide comment
@uklotzde

uklotzde Feb 20, 2016

Contributor

Glad to hear you managed to compile it :) Please note, that I rebase this
branch on master very often to keep it aligned. You might need to checkout
master, pull all changes from my repository, checkout the remote
WriteAudioTags branch again and rebuild.

I'm using this version for keeping my tags in sync with the library. Editing
tags of multiple files still requires puddletag (or something similar).

If you have some feedback, e.g. on how to name or describe the two new
options in the library settings that could be helpful. Did you understand
from the descriptions how these two options work, especially the 2nd one?

On 02/20/2016 03:56 PM, Alibaobao wrote:

thanks for your work and help
it works very nice


Reply to this email directly or view it on GitHub
#728 (comment).

Contributor

uklotzde commented Feb 20, 2016

Glad to hear you managed to compile it :) Please note, that I rebase this
branch on master very often to keep it aligned. You might need to checkout
master, pull all changes from my repository, checkout the remote
WriteAudioTags branch again and rebuild.

I'm using this version for keeping my tags in sync with the library. Editing
tags of multiple files still requires puddletag (or something similar).

If you have some feedback, e.g. on how to name or describe the two new
options in the library settings that could be helpful. Did you understand
from the descriptions how these two options work, especially the 2nd one?

On 02/20/2016 03:56 PM, Alibaobao wrote:

thanks for your work and help
it works very nice


Reply to this email directly or view it on GitHub
#728 (comment).

@Alibaobao

This comment has been minimized.

Show comment
Hide comment
@Alibaobao

Alibaobao Feb 21, 2016

I use your build because i don't want an external beat-analyzer
but want to sort my files by bpm and key.
I still use the normal Version to Scan new files though
because it seems that no beat and key analyzer is configured.
i don't know if it is possible, but can mixxx write the key libary-data into the file ?
the second option means that it rewrites your libary-data using the filetags ?
if that is right its well explained how it works

Alibaobao commented Feb 21, 2016

I use your build because i don't want an external beat-analyzer
but want to sort my files by bpm and key.
I still use the normal Version to Scan new files though
because it seems that no beat and key analyzer is configured.
i don't know if it is possible, but can mixxx write the key libary-data into the file ?
the second option means that it rewrites your libary-data using the filetags ?
if that is right its well explained how it works

@uklotzde

This comment has been minimized.

Show comment
Hide comment
@uklotzde

uklotzde Feb 23, 2016

Contributor

What do you mean with "normal Version"? Both bpm and key are written into tags.

You can explicitly overwrite the file tags with the metadata from your
library by selecting some tracks and invoking ""Save Metadata into File"
from the context menu. That's more or less the opposite of 2nd configuration
option and enables you to synchronize your file tags with your library.

On 02/21/2016 07:05 PM, Alibaobao wrote:

I use your build because i don't want an external beat-analyzer
but want to sort my files by bpm and key.
I still use the normal Version to Scan new files though
because it seems that no beat and key analyzer is configured.
i don't know if it is possible, but can mixxx write the key libary-data
into the file ?
the second option means that it rewrites your libary-data using the filetags ?
if that is right its well explained how it works


Reply to this email directly or view it on GitHub
#728 (comment).

Contributor

uklotzde commented Feb 23, 2016

What do you mean with "normal Version"? Both bpm and key are written into tags.

You can explicitly overwrite the file tags with the metadata from your
library by selecting some tracks and invoking ""Save Metadata into File"
from the context menu. That's more or less the opposite of 2nd configuration
option and enables you to synchronize your file tags with your library.

On 02/21/2016 07:05 PM, Alibaobao wrote:

I use your build because i don't want an external beat-analyzer
but want to sort my files by bpm and key.
I still use the normal Version to Scan new files though
because it seems that no beat and key analyzer is configured.
i don't know if it is possible, but can mixxx write the key libary-data
into the file ?
the second option means that it rewrites your libary-data using the filetags ?
if that is right its well explained how it works


Reply to this email directly or view it on GitHub
#728 (comment).

@uklotzde

This comment has been minimized.

Show comment
Hide comment
@uklotzde

uklotzde Mar 6, 2016

Contributor

Further discussions about the topic can be found in this closed PR:

#901

This solution works fine for me and is safe. But we need to figure out how to handle existing libraries with inconsistencies or duplicates.

Contributor

uklotzde commented Mar 6, 2016

Further discussions about the topic can be found in this closed PR:

#901

This solution works fine for me and is safe. But we need to figure out how to handle existing libraries with inconsistencies or duplicates.

@skaldarnar

This comment has been minimized.

Show comment
Hide comment
@skaldarnar

skaldarnar Mar 21, 2016

Just wanted to let you know I've been using this branch (local build) for a few days now and haven't encountered any serious issues. Here are two notes/questions I'd like to share with you:

  • saving the metadata of several files at once freezes the UI (active song still playing)
  • which tags are actually written? is the popularimeter in use for saving the rating?

skaldarnar commented Mar 21, 2016

Just wanted to let you know I've been using this branch (local build) for a few days now and haven't encountered any serious issues. Here are two notes/questions I'd like to share with you:

  • saving the metadata of several files at once freezes the UI (active song still playing)
  • which tags are actually written? is the popularimeter in use for saving the rating?
@uklotzde

This comment has been minimized.

Show comment
Hide comment
@uklotzde

uklotzde Mar 21, 2016

Contributor

UI freezes are a general issue in Mixxx, because all the library management
code runs in the same UI thread. You will notice this whenever you try to
perform batch operations on a large number of tracks. Even searching is
painfully slow.

Unfortunately there is no common standard for saving ratings in tags, so I
decided don't save them. And before saving them you need to read them ;) I
have my own scheme to encode "rating", "energy", "mood", and "decade" in the
comments field that is easy to filter and portable across various applications.

On 03/21/2016 10:43 AM, Tobias Nett wrote:

Just wanted to let you know I've been using this branch (local build) for
a few days now and haven't encountered any serious issues. Here are two
notes/questions I'd like to share with you:

  • saving the metadata of several files at once freezes the UI (active
    song still playing)
  • which tags are actually written? is the |popularimeter| in use for
    saving the rating?


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#728 (comment)

Contributor

uklotzde commented Mar 21, 2016

UI freezes are a general issue in Mixxx, because all the library management
code runs in the same UI thread. You will notice this whenever you try to
perform batch operations on a large number of tracks. Even searching is
painfully slow.

Unfortunately there is no common standard for saving ratings in tags, so I
decided don't save them. And before saving them you need to read them ;) I
have my own scheme to encode "rating", "energy", "mood", and "decade" in the
comments field that is easy to filter and portable across various applications.

On 03/21/2016 10:43 AM, Tobias Nett wrote:

Just wanted to let you know I've been using this branch (local build) for
a few days now and haven't encountered any serious issues. Here are two
notes/questions I'd like to share with you:

  • saving the metadata of several files at once freezes the UI (active
    song still playing)
  • which tags are actually written? is the |popularimeter| in use for
    saving the rating?


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#728 (comment)

@ywwg

This comment has been minimized.

Show comment
Hide comment
@ywwg

ywwg Mar 21, 2016

Member

take a look at the new track exporter code for an idea of how to do non-blocking library operations in a separate worker thread. That code still uses a modal window, but the important part is it's not too hard to make sure the write-to-disk operations don't happen in the UI thread.

Member

ywwg commented Mar 21, 2016

take a look at the new track exporter code for an idea of how to do non-blocking library operations in a separate worker thread. That code still uses a modal window, but the important part is it's not too hard to make sure the write-to-disk operations don't happen in the UI thread.

@uklotzde

This comment has been minimized.

Show comment
Hide comment
@uklotzde

uklotzde Mar 21, 2016

Contributor

But write-to-disk operations occur individually for each track and cannot be
queued safely. If they happen sometime later in a different thread the
corresponding file might already be in use again. Modifications must be
written to disk immediately when the corresponding TrackInfoObject is
evicted from the cache before it is deleted.

On 03/21/2016 03:24 PM, Owen Williams wrote:

take a look at the new track exporter code for an idea of how to do
non-blocking library operations in a separate worker thread. That code
still uses a modal window, but the important part is it's not too hard to
make sure the write-to-disk operations don't happen in the UI thread.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#728 (comment)

Contributor

uklotzde commented Mar 21, 2016

But write-to-disk operations occur individually for each track and cannot be
queued safely. If they happen sometime later in a different thread the
corresponding file might already be in use again. Modifications must be
written to disk immediately when the corresponding TrackInfoObject is
evicted from the cache before it is deleted.

On 03/21/2016 03:24 PM, Owen Williams wrote:

take a look at the new track exporter code for an idea of how to do
non-blocking library operations in a separate worker thread. That code
still uses a modal window, but the important part is it's not too hard to
make sure the write-to-disk operations don't happen in the UI thread.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#728 (comment)

@ywwg

This comment has been minimized.

Show comment
Hide comment
@ywwg

ywwg Mar 21, 2016

Member

how long do these writes usually take? Would it be possible to flip a flag in the TIO that prevents the track from being loaded while the write is in progress? A boolean indicating a lock, basically. We could gray the track out in the library while it's being written. For bulk operations, a modal window with progress bar seems reasonable.

Member

ywwg commented Mar 21, 2016

how long do these writes usually take? Would it be possible to flip a flag in the TIO that prevents the track from being loaded while the write is in progress? A boolean indicating a lock, basically. We could gray the track out in the library while it's being written. For bulk operations, a modal window with progress bar seems reasonable.

@uklotzde

This comment has been minimized.

Show comment
Hide comment
@uklotzde

uklotzde Mar 21, 2016

Contributor

The library gets updated in the same moment which also involves a write
operation on disk. This is the current behaviour. Consequently all those
operations need to be revised.

But the write operations have never been an issue for me. Doing bulk updates
in a live situation is a very rare use case. What's becoming more and more
frustrating to me are the UI freezes during each database query that block
the UI for seconds. This really hurts!

On 03/21/2016 03:43 PM, Owen Williams wrote:

how long do these writes usually take? Would it be possible to flip a flag
in the TIO that prevents the track from being loaded while the write is in
progress? A boolean indicating a lock, basically. We could gray the track
out in the library while it's being written. For bulk operations, a modal
window with progress bar seems reasonable.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#728 (comment)

Contributor

uklotzde commented Mar 21, 2016

The library gets updated in the same moment which also involves a write
operation on disk. This is the current behaviour. Consequently all those
operations need to be revised.

But the write operations have never been an issue for me. Doing bulk updates
in a live situation is a very rare use case. What's becoming more and more
frustrating to me are the UI freezes during each database query that block
the UI for seconds. This really hurts!

On 03/21/2016 03:43 PM, Owen Williams wrote:

how long do these writes usually take? Would it be possible to flip a flag
in the TIO that prevents the track from being loaded while the write is in
progress? A boolean indicating a lock, basically. We could gray the track
out in the library while it's being written. For bulk operations, a modal
window with progress bar seems reasonable.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#728 (comment)

@ywwg

This comment has been minimized.

Show comment
Hide comment
@ywwg

ywwg Mar 21, 2016

Member

So there are two writes going on -- writing to the library and writing the track metadata. The library is an sqlite database, and sqlite does its own caching and disk access so it's going to be fairly efficient. Writing the audio tags involves us mutating track files on disk ourselves. My interpretation is that the new UI freezes are caused by the writes to the track files, not the library database. Therefore I'm talking about putting a "lock" on the track entries so the tracks can't be loaded while the metadata is being synced, and then moving that syncing to a separate thread.

If the UI freezes we are talking about are totally unrelated to this PR, then that's a separate bug that should be discussed in its own report. But @skaldarnar seemed to say that something in this PR specifically is causing more UI freezes, and that's what I'm trying to figure out. If metadata syncing is being done in the UI thread, I think that's a bad design choice and we should use techniques like I mentioned to avoid it.

Bulk updates will happen while exiting the program and the whole cache gets cleared at once.

Member

ywwg commented Mar 21, 2016

So there are two writes going on -- writing to the library and writing the track metadata. The library is an sqlite database, and sqlite does its own caching and disk access so it's going to be fairly efficient. Writing the audio tags involves us mutating track files on disk ourselves. My interpretation is that the new UI freezes are caused by the writes to the track files, not the library database. Therefore I'm talking about putting a "lock" on the track entries so the tracks can't be loaded while the metadata is being synced, and then moving that syncing to a separate thread.

If the UI freezes we are talking about are totally unrelated to this PR, then that's a separate bug that should be discussed in its own report. But @skaldarnar seemed to say that something in this PR specifically is causing more UI freezes, and that's what I'm trying to figure out. If metadata syncing is being done in the UI thread, I think that's a bad design choice and we should use techniques like I mentioned to avoid it.

Bulk updates will happen while exiting the program and the whole cache gets cleared at once.

@uklotzde

This comment has been minimized.

Show comment
Hide comment
@uklotzde

uklotzde Mar 21, 2016

Contributor

Most bulk operations on tracks are currently causing UI freezes. That's an
issue that needs to be solved by a general library re-design that takes both
read and write operations into account.

There still is an unfinished "Library concurrency refactoring" PR from a
previous GSOC that has not been integrated?

On 03/21/2016 04:01 PM, Owen Williams wrote:

So there are two writes going on -- writing to the library and writing the
track metadata. The library is an sqlite database, and sqlite does its own
caching and disk access so it's going to be fairly efficient. Writing the
audio tags involves us mutating track files on disk ourselves. My
interpretation is that the new UI freezes are caused by the writes to the
track files, not the library database. Therefore I'm talking about putting
a "lock" on the track entries so the tracks can't be loaded while the
metadata is being synced, and then moving that syncing to a separate thread.

If the UI freezes we are talking about are totally unrelated to this PR,
then that's a separate bug that should be discussed in its own report. But
@skaldarnar https://github.com/skaldarnar seemed to say that something
in this PR specifically is causing more UI freezes, and that's what I'm
trying to figure out. If metadata syncing is being done in the UI thread,
I think that's a bad design choice and we should use techniques like I
mentioned to avoid it.

Bulk updates will happen while exiting the program and the whole cache
gets cleared at once.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#728 (comment)

Contributor

uklotzde commented Mar 21, 2016

Most bulk operations on tracks are currently causing UI freezes. That's an
issue that needs to be solved by a general library re-design that takes both
read and write operations into account.

There still is an unfinished "Library concurrency refactoring" PR from a
previous GSOC that has not been integrated?

On 03/21/2016 04:01 PM, Owen Williams wrote:

So there are two writes going on -- writing to the library and writing the
track metadata. The library is an sqlite database, and sqlite does its own
caching and disk access so it's going to be fairly efficient. Writing the
audio tags involves us mutating track files on disk ourselves. My
interpretation is that the new UI freezes are caused by the writes to the
track files, not the library database. Therefore I'm talking about putting
a "lock" on the track entries so the tracks can't be loaded while the
metadata is being synced, and then moving that syncing to a separate thread.

If the UI freezes we are talking about are totally unrelated to this PR,
then that's a separate bug that should be discussed in its own report. But
@skaldarnar https://github.com/skaldarnar seemed to say that something
in this PR specifically is causing more UI freezes, and that's what I'm
trying to figure out. If metadata syncing is being done in the UI thread,
I think that's a bad design choice and we should use techniques like I
mentioned to avoid it.

Bulk updates will happen while exiting the program and the whole cache
gets cleared at once.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#728 (comment)

@daschuer

This comment has been minimized.

Show comment
Hide comment
@daschuer

daschuer Mar 21, 2016

Member

That is on my list. (After all my other projects are merged)
I still like the used concept, but it requires some extra refactoring, before it will work as desired.

Member

daschuer commented Mar 21, 2016

That is on my list. (After all my other projects are merged)
I still like the used concept, but it requires some extra refactoring, before it will work as desired.

@skaldarnar

This comment has been minimized.

Show comment
Hide comment
@skaldarnar

skaldarnar Mar 21, 2016

Thanks for clarification on the UI freezes. Good to know it is a general issue and that you are aware of the problem.

@uklotzde Thank you for the hint to write additional information into the comment field. Is it possible to sort your library by the rating in the comment field somehow? What do you think about setting (kind of arbitrary) numbers for the rating if the user wants to (opt-in in the preferences) as mentioned on the wikipedia page?

skaldarnar commented Mar 21, 2016

Thanks for clarification on the UI freezes. Good to know it is a general issue and that you are aware of the problem.

@uklotzde Thank you for the hint to write additional information into the comment field. Is it possible to sort your library by the rating in the comment field somehow? What do you think about setting (kind of arbitrary) numbers for the rating if the user wants to (opt-in in the preferences) as mentioned on the wikipedia page?

@ywwg

This comment has been minimized.

Show comment
Hide comment
@ywwg

ywwg Mar 21, 2016

Member

ok, I'll give this PR a try and see if I notice any additional freezes while writing track metadata.

Member

ywwg commented Mar 21, 2016

ok, I'll give this PR a try and see if I notice any additional freezes while writing track metadata.

@LindyBalboa

This comment has been minimized.

Show comment
Hide comment
@LindyBalboa

LindyBalboa Nov 7, 2016

Contributor

What is the status of this?

Contributor

LindyBalboa commented Nov 7, 2016

What is the status of this?

@uklotzde

This comment has been minimized.

Show comment
Hide comment
@uklotzde

uklotzde Nov 7, 2016

Contributor

I'm using this version to keep my files in sync with the library. Please
note, that I frequently rebase this branch on master!

On 07.11.2016 11:07, Conner Phillips wrote:

What is the status of this?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#728 (comment), or
mute the thread
https://github.com/notifications/unsubscribe-auth/ABZ-akWOfc6enu3-h3kCAPkiLtGrJFBlks5q7vh7gaJpZM4GIcOu.

Contributor

uklotzde commented Nov 7, 2016

I'm using this version to keep my files in sync with the library. Please
note, that I frequently rebase this branch on master!

On 07.11.2016 11:07, Conner Phillips wrote:

What is the status of this?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#728 (comment), or
mute the thread
https://github.com/notifications/unsubscribe-auth/ABZ-akWOfc6enu3-h3kCAPkiLtGrJFBlks5q7vh7gaJpZM4GIcOu.

@LindyBalboa

This comment has been minimized.

Show comment
Hide comment
@LindyBalboa

LindyBalboa Nov 9, 2016

Contributor

I meant in regards to being merged. Any timeline prediction on that?

Contributor

LindyBalboa commented Nov 9, 2016

I meant in regards to being merged. Any timeline prediction on that?

@uklotzde

This comment has been minimized.

Show comment
Hide comment
@uklotzde

uklotzde Dec 5, 2017

Contributor

@Be-ing Sorry, forgot to remove the debug output. I didn't mean to confuse you ;) The different durations are expected, because TagLib only provides millisecond precision. The duration and other audio properties are excluded from the actual modification check.

Really, really tricky to get the synchronization right in both directions, minimize write operations and to preserve as much information as possible.

Contributor

uklotzde commented Dec 5, 2017

@Be-ing Sorry, forgot to remove the debug output. I didn't mean to confuse you ;) The different durations are expected, because TagLib only provides millisecond precision. The duration and other audio properties are excluded from the actual modification check.

Really, really tricky to get the synchronization right in both directions, minimize write operations and to preserve as much information as possible.

@uklotzde

This comment has been minimized.

Show comment
Hide comment
@uklotzde

uklotzde Dec 5, 2017

Contributor

Hm, I tested with FLACs, seemed to work. I've seen "Skipping..." messags after loading, editing and undoing modifications.

Contributor

uklotzde commented Dec 5, 2017

Hm, I tested with FLACs, seemed to work. I've seen "Skipping..." messags after loading, editing and undoing modifications.

@uklotzde

This comment has been minimized.

Show comment
Hide comment
@uklotzde

uklotzde Dec 5, 2017

Contributor

Confirmed. I found a FLAC that is exported again. Next round.

Contributor

uklotzde commented Dec 5, 2017

Confirmed. I found a FLAC that is exported again. Next round.

uklotzde added some commits Dec 6, 2017

Re-enable new metadata properties
...with much less TODOs and excluded code lines than before.
@uklotzde

This comment has been minimized.

Show comment
Hide comment
@uklotzde

uklotzde Dec 6, 2017

Contributor

I've changed the strategy how to handle floating point values and currently unsupported fields. The special case handling is co-located and only invoked when exporting metadata instead of trying to keep it consistent at any time. Should be much more maintainable now! I added lots of comments, because all those issues are NOT obvious and you can't expect anyone to have that background knowledge.

Exporting of tags happens before writing into the database. So we have eventual consistency with the database. The differences are just rounding errors of floating point values that don't really matter.

Hopefully everything now works as expected, both minimizing write operations and preserving as much information as possible.

Contributor

uklotzde commented Dec 6, 2017

I've changed the strategy how to handle floating point values and currently unsupported fields. The special case handling is co-located and only invoked when exporting metadata instead of trying to keep it consistent at any time. Should be much more maintainable now! I added lots of comments, because all those issues are NOT obvious and you can't expect anyone to have that background knowledge.

Exporting of tags happens before writing into the database. So we have eventual consistency with the database. The differences are just rounding errors of floating point values that don't really matter.

Hopefully everything now works as expected, both minimizing write operations and preserving as much information as possible.

@Be-ing

It works! Deleting Library, TrackDAO, TrackCache, and closing database connections all together takes about 150 ms when no loaded tracks have had metadata modified. Modifying metadata in the track table without loading a track commits the metadata changes immediately. Modifying metadata for loaded tracks commits the changes when the track is no longer loaded in any deck. LGTM aside from making the comments a little more verbose.

Show outdated Hide outdated src/track/track.cpp Outdated
tr("Export Modified Track Metadata"),
tr("Mixxx may wait to modify files until they are not loaded to any decks or samplers. "
"If you do not see changed metadata in other programs immediately, "
"eject the track from all decks and samplers or shutdown Mixxx."));

This comment has been minimized.

@Be-ing

Be-ing Dec 6, 2017

Contributor

👍

@Be-ing

Be-ing Dec 6, 2017

Contributor

👍

@uklotzde

This comment has been minimized.

Show comment
Hide comment
@uklotzde

uklotzde Dec 8, 2017

Contributor

Oops. I accidentally pushed some changes while testing the loop escape PR. Fixed.

Contributor

uklotzde commented Dec 8, 2017

Oops. I accidentally pushed some changes while testing the loop escape PR. Fixed.

@Be-ing

This comment has been minimized.

Show comment
Hide comment
@Be-ing

Be-ing Dec 10, 2017

Contributor

LGTM thank you! @daschuer, ready for merge?

Contributor

Be-ing commented Dec 10, 2017

LGTM thank you! @daschuer, ready for merge?

@daschuer

This comment has been minimized.

Show comment
Hide comment
@daschuer

daschuer Dec 10, 2017

Member

Yes, we can merge. Juchhu! Thank you so much! Now I can consider to ditch Clementine for metadata.

Member

daschuer commented Dec 10, 2017

Yes, we can merge. Juchhu! Thank you so much! Now I can consider to ditch Clementine for metadata.

@daschuer daschuer merged commit 43e4462 into mixxxdj:master Dec 10, 2017

0 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
@uklotzde

This comment has been minimized.

Show comment
Hide comment
@uklotzde

uklotzde Dec 10, 2017

Contributor

Thank you both so much for reviewing this beast! I'm curious to get some real-world user feedback. It's always sensitive to modify a user's own private data.

...now let's get the post-fader effects ready ;)

Contributor

uklotzde commented Dec 10, 2017

Thank you both so much for reviewing this beast! I'm curious to get some real-world user feedback. It's always sensitive to modify a user's own private data.

...now let's get the post-fader effects ready ;)

@daschuer

This comment has been minimized.

Show comment
Hide comment
@daschuer

daschuer Dec 10, 2017

Member

Yes!

Member

daschuer commented Dec 10, 2017

Yes!

@foss-

This comment has been minimized.

Show comment
Hide comment
@foss-

foss- Dec 11, 2017

This is awesome. Thanks so much for your work on this Uwe! I am really happy to see this in mixxx

The next thing missing regarding metadata in / out and improving mixxx + other app compatibility and what would be highly helpful to me personally is star ratings, which has a PR but it doesn't look as if active development is happening: #1089

foss- commented Dec 11, 2017

This is awesome. Thanks so much for your work on this Uwe! I am really happy to see this in mixxx

The next thing missing regarding metadata in / out and improving mixxx + other app compatibility and what would be highly helpful to me personally is star ratings, which has a PR but it doesn't look as if active development is happening: #1089

@christophski

This comment has been minimized.

Show comment
Hide comment
@christophski

christophski Dec 12, 2017

Very exciting! Definitely going to test this asap (dw, I have backups! haha)

christophski commented Dec 12, 2017

Very exciting! Definitely going to test this asap (dw, I have backups! haha)

@uklotzde

This comment has been minimized.

Show comment
Hide comment
@uklotzde

uklotzde Dec 12, 2017

Contributor

Vorbis -> MusicBrainz style comments: If anyone needs help with converting DESCRIPTION into COMMENT fields in FLAC files safely, just ask me. I'm preparing a shell script using metaflac to analyze and cleanup my collection.

I don't have any OggVorbis files, but the proceedings should be similar.

Contributor

uklotzde commented Dec 12, 2017

Vorbis -> MusicBrainz style comments: If anyone needs help with converting DESCRIPTION into COMMENT fields in FLAC files safely, just ask me. I'm preparing a shell script using metaflac to analyze and cleanup my collection.

I don't have any OggVorbis files, but the proceedings should be similar.

@MK-42

This comment has been minimized.

Show comment
Hide comment
@MK-42

MK-42 Dec 18, 2017

Contributor

Awesome work @uklotzde, @daschuer and and @Be-ing we finally have it in master 👍
One thing I noticed while playing with it:
when inline-editing a track in the TrackTable, the export is triggered only when the focus switches to another track in the table. Is that expected behaviour? It's nothing serious but caught my attention.

By the way: It's really nice to see that mixxx gains some pace in development - looking forward for a new beta release to update some friends 👍 I'm already using current master for noncritical daily work

Contributor

MK-42 commented Dec 18, 2017

Awesome work @uklotzde, @daschuer and and @Be-ing we finally have it in master 👍
One thing I noticed while playing with it:
when inline-editing a track in the TrackTable, the export is triggered only when the focus switches to another track in the table. Is that expected behaviour? It's nothing serious but caught my attention.

By the way: It's really nice to see that mixxx gains some pace in development - looking forward for a new beta release to update some friends 👍 I'm already using current master for noncritical daily work

@uklotzde

This comment has been minimized.

Show comment
Hide comment
@uklotzde

uklotzde Dec 18, 2017

Contributor

@MK-42 It would be overkill to save tags (including a backup file copy) after every field edit, impossible. Tags are only saved if the corresponding cached track object is destroyed, i.e. when it is evicted from the in-memory cache. You may have noticed the message box that popped up when enabling this feature 😉

Exporting tags is a very costly file operation that might introduce some latency within the main tread due to lock contention on the cache. We already have a follow-up PR trying to minimize this impact. There is also a serious issue with the initialization order on startup that needs to be fixed.

Contributor

uklotzde commented Dec 18, 2017

@MK-42 It would be overkill to save tags (including a backup file copy) after every field edit, impossible. Tags are only saved if the corresponding cached track object is destroyed, i.e. when it is evicted from the in-memory cache. You may have noticed the message box that popped up when enabling this feature 😉

Exporting tags is a very costly file operation that might introduce some latency within the main tread due to lock contention on the cache. We already have a follow-up PR trying to minimize this impact. There is also a serious issue with the initialization order on startup that needs to be fixed.

@MK-42

This comment has been minimized.

Show comment
Hide comment
@MK-42

MK-42 Dec 19, 2017

Contributor

It would be overkill to save tags (including a backup file copy) after every field edit, impossible. Tags are only saved if the corresponding cached track object is destroyed

I followed your discussion here and were aware of that fact. But I was unsure if there is some cached track object for that track if it is not loaded to any deck/sampler and expected an immediate write operation in that case. That changing the focus in the table triggers the export was unexpected for me and I was unsure if that could be a bug. If thats expected behavior, everything is alright 👍

Contributor

MK-42 commented Dec 19, 2017

It would be overkill to save tags (including a backup file copy) after every field edit, impossible. Tags are only saved if the corresponding cached track object is destroyed

I followed your discussion here and were aware of that fact. But I was unsure if there is some cached track object for that track if it is not loaded to any deck/sampler and expected an immediate write operation in that case. That changing the focus in the table triggers the export was unexpected for me and I was unsure if that could be a bug. If thats expected behavior, everything is alright 👍

@esbrandt esbrandt referenced this pull request Apr 14, 2018

Merged

WIP Manual 2.1.x updates #59

20 of 37 tasks complete

esbrandt added a commit to esbrandt/manual that referenced this pull request Apr 14, 2018

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