Add BPM tags to built-in beat loops (#5439) #6747

mirk0dex · 2023-06-26T19:57:03Z

Added floating-point vorbis BPM tags to files in lmms/data/samples/beats
Added rounded BPM to filenames, surrounded by square brackets and separated from the rest of the filename by an underscore, as requested by @ttepatti.

To achieve this, I used the bpm-tools utils and verified that the values were correct using LMMS itself. When the tempos were not correct, I fired up the DAW and started looking, manually, for one fitting the loop. When I found one, I once again used bpm-tools (but this time with a range that suited the approximate tempo) and detected the precise value.

I then proceed to add the vorbis tags, and rename the files.

All tempos should be accurate. Only loops I'd double-check are the breaks: finding the right BPM wasn't easy there.

mirk0dex · 2023-06-26T20:04:29Z

I found out that LMMS has an "assets" repo. Gonna use that.

mirk0dex · 2023-06-26T20:10:20Z

Not sure whether it's OK for me to leave this here or not. I can't find the beat loops in https://github.com/LMMS/assets, so I'm keeping the PR open.

zonkmachine · 2023-09-16T19:07:35Z

Hi, thanks for looking into this!

Now that the names are changed we need some form of upgrade routine so old projects that use the samples will find them. How is your coding? :)

mirk0dex · 2023-09-16T20:24:10Z

Hey, I'd say it's fine. I'd be glad to help out, if you tell me what to do I'll try my best to wrap my head around it.

…

On September 16, 2023 9:07:46 PM GMT+02:00, Oskar Wallgren ***@***.***> wrote: Hi, thanks for looking into this! Now that the names are changed we need some form of upgrade routine so old projects that use the samples will find them. How is your coding? :) -- Reply to this email directly or view it on GitHub: #6747 (comment) You are receiving this because you modified the open/close state. Message ID: ***@***.***>

zonkmachine · 2023-09-16T20:58:12Z

OK. When we need to upgrade a project file to work with a new feature, name changes quite commonly, the job is done here: https://github.com/LMMS/lmms/blob/master/src/core/DataFile.cpp

Here is a recent PR that addressed an issue with elements that had been renamed but the upgrade didn't work:
#6851

Here is a minimal demo project with just a loop that needs to be upgraded to the new name:
beatupgrade.mmp.txt
<sampleclip muted="0" off="0" src="factorysample:beats/electro_beat01.ogg" pos="0" sample_rate="44100" len="384"/>

Have fun!

mirk0dex · 2023-09-16T21:18:31Z

Thanks! I'll look into this tomorrow.

…

On September 16, 2023 10:58:22 PM GMT+02:00, Oskar Wallgren ***@***.***> wrote: OK. When we need to upgrade a project file to work with a new feature, name changes quite commonly, the job is done here: https://github.com/LMMS/lmms/blob/master/src/core/DataFile.cpp Here is a recent PR that addressed an issue with elements that had been renamed but the upgrade didn't work: #6851 Here is a minimal demo project with just a loop that needs to be upgraded to the new name [beatupgrade.mmp.txt](https://github.com/LMMS/lmms/files/12641568/beatupgrade.mmp.txt) `<sampleclip muted="0" off="0" src="factorysample:beats/electro_beat01.ogg" pos="0" sample_rate="44100" len="384"/>` Have fun! -- Reply to this email directly or view it on GitHub: #6747 (comment) You are receiving this because you modified the open/close state. Message ID: ***@***.***>

mirk0dex · 2023-09-17T12:05:39Z

I created an upgrade procedure. It simply loops through all elements and if they contain one of the changed strings, it appends " - X BPM" to the sample names.

Unfortunately, it looks like Emacs drastically changed the file's indentation and code arrangement, for some reason. If you want me to, I can restore the original version and use vim or something to copy and paste my function in it.

Also, I haven't tested any of the code yet, because I'm currently using Void GNU/Linux, and building LMMS here is a pain in the neck.

Edit: BTW, here's the updated file: https://github.com/mirk0dex/lmms/blob/master/src/core/DataFile.cpp. The function's name is upgrade_loopsRename().

zonkmachine · 2023-09-17T13:19:57Z

If you want me to, I can restore the original version and use vim or something to copy and paste my function in it.

Want. The problem is that it's one commit and it's not easy to find your intended changes among all the format changes. The format changes, at a glance, do look like they follow the coding standard.

mirk0dex · 2023-09-17T13:49:41Z

If you want me to, I can restore the original version and use vim or something to copy and paste my function in it.

Done.

I pasted it at the end of the file. I also committed it to the correct branch this time.

zonkmachine · 2023-09-17T14:11:09Z

If you want me to, I can restore the original version and use vim or something to copy and paste my function in it.

Done.

I pasted it at the end of the file. I also committed it to the correct branch this time.

Did this actually compile? You can't just add a function to DataFile.cpp without declaring it in DataFile.h
Search src/core/Datafile.cpp and include/DataFile.h for occurences of upgrade_sampleAndHold(). Try and *mirror what's done and stick your stuff after.

src/core/DataFile.cpp

zonkmachine · 2023-09-17T14:25:54Z

@mirk0dex Does this actually compile for you?

/builds/lmms/src/core/DataFile.cpp:1966:39: error: template argument 1 is invalid
 1966 |         std::vector<std::pair<string, string>> loopBPMs{

mirk0dex · 2023-09-17T14:28:34Z

@mirk0dex Does this actually compile for you?

/builds/lmms/src/core/DataFile.cpp:1966:39: error: template argument 1 is invalid
 1966 |         std::vector<std::pair<string, string>> loopBPMs{

I haven't tried compiling anything, yet. Is there any way I can just compile that file, or check for errors without having to build everything?

mirk0dex · 2023-09-17T14:30:17Z

Maybe using std::string instead of just string will do the trick?

zonkmachine · 2023-09-17T14:30:22Z

@mirk0dex Does this actually compile for you?
/builds/lmms/src/core/DataFile.cpp:1966:39: error: template argument 1 is invalid
 1966 |         std::vector<std::pair<string, string>> loopBPMs{
I haven't tried compiling anything, yet. Is there any way I can just compile that file, or check for errors without having to build everything?

No. You should make sure that things build before committing it to a project or be sure to mention that you haven't tested it yet.

mirk0dex · 2023-09-17T14:31:58Z

@mirk0dex Does this actually compile for you?
/builds/lmms/src/core/DataFile.cpp:1966:39: error: template argument 1 is invalid
 1966 |         std::vector<std::pair<string, string>> loopBPMs{
I haven't tried compiling anything, yet. Is there any way I can just compile that file, or check for errors without having to build everything?
No. You should make sure that things build before committing it to a project or be sure to mention that you haven't tested it yet.

I did say it in a previous message. Apologies for the inconvenience. Anyways, tomorrow I will have my Arch system with me, so I will be able to compile everything with ease.

zonkmachine · 2023-09-17T14:33:39Z

Maybe using std::string instead of just string will do the trick?

QString is what the rest of the file is using. Stick to that. And then compile it. You actually need to do that. If you have problem compiling it you need to bring it up at our Discord site. This is not the right place.

zonkmachine · 2023-09-19T16:05:35Z

I tested the function. It now works as it should.

<sampleclip off="0" sample_rate="44100" len="384" muted="0" src="factorysample:beats/electro_beat01 - 120 BPM.ogg" pos="0"/>

Right. This now upgrades. Loops can be imported into our builtin sampler, the audio file processor. This doesn't seem to upgrade.

          <instrument name="audiofileprocessor">
            <audiofileprocessor eframe="1" lframe="0" reversed="0" amp="100" stutter="0" interp="1" src="factorysample:beats/909beat01.ogg" looped="0" sframe="0">
              <key/>
            </audiofileprocessor>
          </instrument>

New project to test. I forgot to mention that we add .txt to the end of project files instead of zipping as github doesn't recognize .mmp. Just drop the suffix and go...
beatupgrade2.mmp.txt

zonkmachine

I left some remarks on coding style but this looks good and tests out fine. The Vorbis tags look fine although I don't know the usefulness of bpm measured to 3 decimals precision. The upgrade works. I've run out of things to test now. Approved.

zonkmachine · 2023-09-21T15:17:15Z

src/core/DataFile.cpp

+						{
+							// Add " - X BPM" to filename
+							srcVal.replace(toReplace,
+									toReplace + BPMPREF + currentPair.second + BPMSUFF);


The lines above can be written together. We go to 120 characters for a line.

include/DataFile.h

src/core/DataFile.cpp

mirk0dex · 2023-09-22T05:24:39Z

Thanks for all the suggestions. I will work on these issues ASAP.

Co-authored-by: Dominic Clark <mrdomclark@gmail.com>

zonkmachine

Hopefully the last review.

include/DataFile.h

src/core/DataFile.cpp

zonkmachine · 2023-11-17T15:52:35Z

No. You should make sure that things build before committing it to a project or be sure to mention that you haven't tested it yet.

I did say it in a previous message. Apologies for the inconvenience. Anyways, tomorrow I will have my Arch system with me, so I will be able to compile everything with ease.

For the record, yes you did. I came through as a bit more salty than intended. Sorry!

Thanks for contributing to lmms and for sticking with us through the review process.

mirk0dex · 2023-11-17T15:54:09Z

No worries, I understand!

It was fun to contribute to the project, thanks to you all for this amazing piece of software.

Fix the upgrade routine that was introduced with pull request LMMS#6747 which added the BPM value to some file names. This also simplifies the implementation by using a map.

Fix the upgrade routine that was introduced with pull request #6747 which added the BPM value to some file names. This also simplifies the implementation by using a map. Note: this also removes the code about the prefix `factorysample:`. If it is used in some files these entries will also have to be added to the map.

Added BPM tags to built-in beat loops (#5439)

75f90b2

mirk0dex mentioned this pull request Jun 26, 2023

Add BPM tags to built-in .ogg samples #5439

Closed

mirk0dex closed this Jun 26, 2023

Updated file naming

ce50c97

mirk0dex reopened this Jun 26, 2023

mirk0dex added 3 commits June 26, 2023 22:41

Added BPM tags to bassloops

0a5a64a

Moved BPM to its own tag

4787ab5

Added BPM tags to latin loops

e93d8b1

mirk0dex and others added 3 commits September 17, 2023 13:33

Merge branch 'LMMS:master' into bpm-tags

e89aa16

Created upgrade routine for loop new filenames with BPM

d739f6f

Restored original DataFile.cpp, added loop filename update function

465c46d

Declared update_loopsRename() in DataFile.h; added it to UPGRADE_METHODS

de33282

zonkmachine reviewed Sep 17, 2023

View reviewed changes

src/core/DataFile.cpp Outdated Show resolved Hide resolved

Moved upgrade_loopsRename() to after upgrade_sampleAndHold()

92e7b6d

Added support for audiofileprocessor in upgrade_loopsRename()

c2d64b4

zonkmachine approved these changes Sep 21, 2023

View reviewed changes

DomClark reviewed Sep 21, 2023

View reviewed changes

src/core/DataFile.cpp Outdated Show resolved Hide resolved

src/core/DataFile.cpp Outdated Show resolved Hide resolved

src/core/DataFile.cpp Outdated Show resolved Hide resolved

src/core/DataFile.cpp Outdated Show resolved Hide resolved

src/core/DataFile.cpp Outdated Show resolved Hide resolved

mirk0dex and others added 10 commits September 22, 2023 09:12

Moved upgrade_loopsRename() to below upgrade_sampleAndHold()

051b1de

Explicitly made elem and srcAttrs constant

b3aab05

Co-authored-by: Dominic Clark <mrdomclark@gmail.com>

Removed useless variables

72ff308

Merge branch 'master' into bpm-tags

e9b77db

Replaced std::vector with std::array

b6eb8a2

Made check less loose

670dfea

Replaced normal loop with range-based

983cafc

Merge branch 'LMMS:master' into bpm-tags

86d6f95

Made loopBPMs static

846f072

Moved upgrade_loopsRename() before upgrade_midiCCIndexing()

a3cad7c

zonkmachine requested changes Nov 11, 2023

View reviewed changes

include/DataFile.h Show resolved Hide resolved

Moved include_loopsRename() down

48f0b88

zonkmachine approved these changes Nov 13, 2023

View reviewed changes

zonkmachine requested a review from DomClark November 13, 2023 19:42

mirk0dex changed the title ~~Added BPM tags to built-in beat loops (#5439)~~ Add BPM tags to built-in beat loops (#5439) Nov 14, 2023

sakertooth reviewed Nov 17, 2023

View reviewed changes

src/core/DataFile.cpp Outdated Show resolved Hide resolved

src/core/DataFile.cpp Outdated Show resolved Hide resolved

Reduced nesting; removed pointless casting

19cb17b

sakertooth approved these changes Nov 17, 2023

View reviewed changes

zonkmachine merged commit a64bbc7 into LMMS:master Nov 17, 2023
9 checks passed

Monospace-V mentioned this pull request Apr 1, 2024

Account for changed native folder/file names when loading files #7186

Closed

1 task

michaelgregorius mentioned this pull request May 1, 2024

Fix upgrade routine for BPM renamings #7235

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add BPM tags to built-in beat loops (#5439) #6747

Add BPM tags to built-in beat loops (#5439) #6747

mirk0dex commented Jun 26, 2023 •

edited

Loading

mirk0dex commented Jun 26, 2023

mirk0dex commented Jun 26, 2023 •

edited

Loading

zonkmachine commented Sep 16, 2023

mirk0dex commented Sep 16, 2023 via email

zonkmachine commented Sep 16, 2023 •

edited

Loading

mirk0dex commented Sep 16, 2023 via email

mirk0dex commented Sep 17, 2023 •

edited

Loading

zonkmachine commented Sep 17, 2023

mirk0dex commented Sep 17, 2023

zonkmachine commented Sep 17, 2023

zonkmachine commented Sep 17, 2023

mirk0dex commented Sep 17, 2023

mirk0dex commented Sep 17, 2023

zonkmachine commented Sep 17, 2023

mirk0dex commented Sep 17, 2023

zonkmachine commented Sep 17, 2023

zonkmachine commented Sep 19, 2023

zonkmachine left a comment

zonkmachine Sep 21, 2023

mirk0dex commented Sep 22, 2023 via email •

edited

Loading

zonkmachine left a comment

zonkmachine commented Nov 17, 2023

mirk0dex commented Nov 17, 2023

Add BPM tags to built-in beat loops (#5439) #6747

Add BPM tags to built-in beat loops (#5439) #6747

Conversation

mirk0dex commented Jun 26, 2023 • edited Loading

mirk0dex commented Jun 26, 2023

mirk0dex commented Jun 26, 2023 • edited Loading

zonkmachine commented Sep 16, 2023

mirk0dex commented Sep 16, 2023 via email

zonkmachine commented Sep 16, 2023 • edited Loading

mirk0dex commented Sep 16, 2023 via email

mirk0dex commented Sep 17, 2023 • edited Loading

zonkmachine commented Sep 17, 2023

mirk0dex commented Sep 17, 2023

zonkmachine commented Sep 17, 2023

zonkmachine commented Sep 17, 2023

mirk0dex commented Sep 17, 2023

mirk0dex commented Sep 17, 2023

zonkmachine commented Sep 17, 2023

mirk0dex commented Sep 17, 2023

zonkmachine commented Sep 17, 2023

zonkmachine commented Sep 19, 2023

zonkmachine left a comment

Choose a reason for hiding this comment

zonkmachine Sep 21, 2023

Choose a reason for hiding this comment

mirk0dex commented Sep 22, 2023 via email • edited Loading

zonkmachine left a comment

Choose a reason for hiding this comment

zonkmachine commented Nov 17, 2023

mirk0dex commented Nov 17, 2023

mirk0dex commented Jun 26, 2023 •

edited

Loading

mirk0dex commented Jun 26, 2023 •

edited

Loading

zonkmachine commented Sep 16, 2023 •

edited

Loading

mirk0dex commented Sep 17, 2023 •

edited

Loading

mirk0dex commented Sep 22, 2023 via email •

edited

Loading