mixxxdj / mixxx

Mixxx is Free DJ software that gives you everything you need to perform live mixes.
http://mixxx.org
Other
4.52k stars 1.28k forks source link

BPM value not exported after editing #10420

Closed mixxxbot closed 2 years ago

mixxxbot commented 2 years ago

Reported by: atskler Date: 2021-05-22T22:54:28Z Status: Fix Released Importance: High Launchpad Issue: lp1929311 Attachments: mp3-bpm-tags.pdf


In mixxx I can set the BPM to 129.8333 but it will write 130 in the mp3 ID3 tag. Flac works correctly.

Mixxx 2.3-beta-4046-g1d1294746f

mixxxbot commented 2 years ago

Commented by: atskler Date: 2021-05-22T23:00:07Z


Please change the milestone of this bug to 2.3.0. - Thank you.

mixxxbot commented 2 years ago

Commented by: uklotzde Date: 2021-05-22T23:05:54Z


ID3 tags only support integer BPM values.

mixxxbot commented 2 years ago

Commented by: atskler Date: 2021-05-23T00:24:52Z


imho, in this case there should be on option to do

1) Re-Import Metadata from file 2) Import from file tags

WITHOUT re-reading and re-importing to mixxx database the unknowingly rounded BPM value from the ID3 tag.

(I usually manage and edit my library with foobar2000 and Mp3tag and then re-read the changes in mixxx.)

mixxxbot commented 2 years ago

Commented by: atskler Date: 2021-05-23T00:44:48Z


ok, the 2) option currently does not re-read and re-import the BPM, plus I can use the "lock BPM" feature

mixxxbot commented 2 years ago

Commented by: uklotzde Date: 2021-05-23T09:03:35Z


Please provide an example for your use case, I don't get it.

We already ignore the rounded BPM value on re-import. It is only imported initially or if the BPM in Mixxx is unknown.

mixxxbot commented 2 years ago

Commented by: uklotzde Date: 2021-05-23T09:22:11Z


All BPM values in tags are imprecise and we already deal with this. Modifying BPM values and re-importing them cannot be permitted, as it would cause inconsistencies between the beat grid and this nominal BPM.

mixxxbot commented 2 years ago

Commented by: atskler Date: 2021-05-23T11:15:27Z


1) Set the BPM in mix to 129,83333333 for a mp3 (in Right Click » Properties ...) 2) Right Click » Metadata » Export To File Tags 3) Exit mixxx 4) Start mixxx 5) Find that mp3 » Right Click » Properties » Re-Import Metadata from File » Apply/OK 6) Now the rounded integer BPM imported to mixxx library.

(My workflow: I get my music » tag it with foobar2000 and mp3tag » load and analyze in mixxx » do an Export To File Tags in mixxx. This writes the BPM in file tags and will show up in foobar2000 – I play and pre-select music for my sets using foobar2000.)

[Can’t there be a workaround for this problem using APE tag or a custom field? If I have an mp3 with APEv2 tag only, mixxx will write BPM with thousandth precision (and will add an ID3v2.4 tag part) and it will show up this way in the Mp3tag software (thousandth precision), but mixxx will see/import integer rounded value.]

mixxxbot commented 2 years ago

Commented by: uklotzde Date: 2021-05-23T11:26:30Z


If you use APEv2 tags only Mixxx will use these. But we won't merge conflicting information from different tags, out of scope. The priority is ID3v2 > APEv2.

mixxxbot commented 2 years ago

Commented by: uklotzde Date: 2021-05-23T11:55:20Z


You might have discovered an edge case. I'll check that.

mixxxbot commented 2 years ago

Commented by: uklotzde Date: 2021-05-23T12:49:09Z


https://github.com/mixxxdj/mixxx/pull/3896

mixxxbot commented 2 years ago

Commented by: atskler Date: 2021-05-23T14:16:57Z Attachments: mp3-bpm-tags.pdf


I created screenshots, please check it.

If there is ONLY APEv2 tag then mixxx will create ID3v2.4 besides APEv2 at Export To File Tags, AND will write the BPM value with thousandths. But it will re-import it as integer. BUT if I convert the tags to APEv2 ONLY, then mixxx will import properly the written BPM with thousandths.

mixxxbot commented 2 years ago

Commented by: uklotzde Date: 2021-05-23T14:39:32Z


Confirmed, TagLib is invonvenient.

ID3v2::Tag* TagLib::MPEG::File::ID3v2Tag(bool create = false) "Note: This may return a valid pointer regardless of whether or not the file on disk has an ID3v2 tag. Use hasID3v2Tag() to check if the file on disk actually has an ID3v2 tag."

mixxxbot commented 2 years ago

Commented by: atskler Date: 2021-05-23T15:26:18Z


To sum thing up I can see 5 possible bugs/problems depending on the intended behavior:

1) The problem of exporting precise BPM values to mp3 file tag

2) The “Right Click » Metadata » Import From File Tags” function does not import the BPM value from the file tag to the library.

3) The “Right Click » Properties » Re-Import Metadata from File” function does import the BPM value from the file tag to the library

4) BPM value not exported after editing

5) MP3 files with only APEv2 tags will get ID3v2.4 tags when “Right Click » Metadata » Export to File Tags” instead just APEv2 update.

.

Regarding problem 1) and 5) maybe there could be an option in mixxx for choosing tag writing methods for mp3 files. Only keep and write ID3v2.4 or only keep and write APEv2 or keep and write both. And the use of APEv2 tags could be a good solution to problem 1). Or?

mixxxbot commented 2 years ago

Commented by: uklotzde Date: 2021-05-23T16:27:05Z


https://github.com/mixxxdj/mixxx/pull/3898

mixxxbot commented 2 years ago

Commented by: uklotzde Date: 2021-05-23T16:30:18Z


1) ID3v2 standard, cannot/won't fix

2) + 3) Has to be retested after the bug fixes have been merged

4) Only affects main/2.4? (Closed PR: https://github.com/mixxxdj/mixxx/pull/3896)

5) Fixed: https://github.com/mixxxdj/mixxx/pull/3898

mixxxbot commented 2 years ago

Commented by: uklotzde Date: 2021-05-23T19:35:12Z


The slight inconsistency between 2) and 3) cannot be fixed easily for 2.3. When importing through the properties dialog an existing beat grid and bpm will be overridden unconditionally.

A major refactoring of the properties dialog is almost ready to be published and will prevent this.

mixxxbot commented 2 years ago

Commented by: uklotzde Date: 2021-05-25T07:43:51Z


4) I cannot reproduce this bug in 2.3. After editing the BPM value it is exported as a file tag. There was a regression on main that has been fixed in https://github.com/mixxxdj/mixxx/pull/3870

mixxxbot commented 2 years ago

Commented by: uklotzde Date: 2021-05-25T10:40:20Z


2) + 3) The inconsistent re-import behavior will only be fixed in 2.4: https://github.com/mixxxdj/mixxx/pull/3906

Valid bpm values should never be overwritten when re-importing from file tags.

mixxxbot commented 2 years ago

Commented by: atskler Date: 2021-05-26T17:11:27Z


Uwe Klotz thank you for the fix for 5).

mixxxbot commented 2 years ago

Issue closed with status Fix Released.