urinieto / msaf

Music Structure Analysis Framework
MIT License
478 stars 79 forks source link

Adding the CBM algorithm. #153

Closed ax-le closed 6 months ago

ax-le commented 6 months ago

Hi! This pull request aims at adding the CBM algorithm in the MSAF toolbox (see issue https://github.com/urinieto/msaf/issues/146). In the end, all the important files are added in the "algorithm/cbm" folder, in order not to depend on the external toolbox (and the associated installation problems...). Let me know if I can improve the code in any way!

urinieto commented 6 months ago

This looks good, thanks! I am running the tests now.

A couple of comments:

ax-le commented 6 months ago

Hi Oriol! Thanks for the review and the comments! :)

Please note that two Pull Requests should follow: one about mel spectrograms, and one about barwise-aligned features (Barwise TF matrix). I think we should discuss about barwise-aligned features, as they require major modifications! (But this is for later, just saying it here.)

urinieto commented 6 months ago

Thanks @ax-le ! I know that the other algorithms don't have tests (yet!?) but it might be nice to set a precedence. There's tests for the main MSAF system, you can take them as reference (see: https://github.com/urinieto/msaf/tree/main/tests). Of course, if that's something you don't want to commit to at the moment, I'm happy to just merge this right away.

ax-le commented 6 months ago

Hi @urinieto and happy festive season! I think that it would be better to merge it right away, if that's ok with you!

urinieto commented 6 months ago

Sounds good. Happy new year, @ax-le , and thanks for your contribution!