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

Implement an automatic bmp computation #3856

Closed
ribeaud opened this issue Feb 12, 2021 · 8 comments · Fixed by #4923
Closed

Implement an automatic bmp computation #3856

ribeaud opened this issue Feb 12, 2021 · 8 comments · Fixed by #4923
Labels
feature features we would like to implement

Comments

@ribeaud
Copy link

ribeaud commented Feb 12, 2021

Use case

beets already comes with a plugin for computing bpm, based on manual tapping (!).
However alternatives exist for automatically computing the bpm:

Solution

I quickly test both. Both projects are quite active. audio was a bit simpler to install and to test. It could be installed as a standalone command as well, not only as Python dependency.

Other criteria to consider if I want to write a plugin around it? What about the name? Or are they already plugins in beets which might compute the bmp?

@wisp3rwind
Copy link
Member

I quickly test both. Both projects are quite active. audio was a bit simpler to install and to test. It could be installed as a standalone command as well, not only as Python dependency.

Other criteria to consider if I want to write a plugin around it? What about the name? Or are they already plugins in beets which might compute the bmp?

In fact, I would say it should become part of the bpm plugin: You could add a method configuration option. It would need to take a value like manual corresponding to the current functionality. In addition, you could pick your preferred backend out of librosa and aubio and implement that (or both, or leave the other to whoever is interested). This would be similar to other plugins (lyrics, fetchart) that provide some functionality from a number of user-selectable backends.

@ribeaud
Copy link
Author

ribeaud commented Feb 12, 2021

Thanks a lot. I will have a look.

@sampsyo sampsyo added the feature features we would like to implement label Feb 12, 2021
@sampsyo
Copy link
Member

sampsyo commented Feb 12, 2021

Sounds reasonable. FWIW, you can also try AcousticBrainz, which gets not only bpm but also many other acoustic measurements:
https://beets.readthedocs.io/en/stable/plugins/acousticbrainz.html

@ribeaud
Copy link
Author

ribeaud commented Feb 12, 2021

Sounds reasonable. FWIW, you can also try AcousticBrainz, which gets not only bpm but also many other acoustic measurements:
https://beets.readthedocs.io/en/stable/plugins/acousticbrainz.html

True. But then this information will be triggered from the AcousticBrainz API. If the track you are trying to import is not part of the project, then you have nothing or, better said, you will need an offline solution.

This is where the current bpm plugin makes senses.

Does it make sense?

@sampsyo
Copy link
Member

sampsyo commented Feb 12, 2021

Yes, but there is also absubmit, which actually does the analysis:
https://beets.readthedocs.io/en/stable/plugins/absubmit.html

@ribeaud
Copy link
Author

ribeaud commented Feb 12, 2021

Having a deeper look at it... If we go the way @wisp3rwind is suggesting, then we end up with an unclean interface IMHO: max_strokes makes only sense in manual mode. One possibility to overcome this flaw would be to have a deeper YAML structure:

method:
  manual:
    max_strokes: ...

What do you think?

@wisp3rwind
Copy link
Member

Sorry @ribeaud for not replying for such a long time here!

Having a deeper look at it... If we go the way @wisp3rwind is suggesting, then we end up with an unclean interface IMHO: max_strokes makes only sense in manual mode. One possibility to overcome this flaw would be to have a deeper YAML structure:

method:
  manual:
    max_strokes: ...

What do you think?

Sounds good! I guess beets doesn't really have a very clean config structure in that sense overall, and we don't validate the configuration apart from checking types for the values that are actually being accessed anyway.

Currently, the plugin can be configured as

bpm:
    max_strokes: 3,
    overwrite: yes,

So the full config section for the plugin might look like this I guess?

bpm:
    max_strokes: 3,  # Still support this for the time being, but issue a deprecation warning
    overwrite: yes,
    method: "manual"|"aubio",   # Default to "manual", which doesn't have any external dependency and matches the old behaviour
    manual:
        max_strokes: 3,
    aubio:
        some_setting: "value",

One could also consider adding an auto option to enable analysis on import, and it could be worthwhile to add a --method CLI option to the bpm command which overrides the config value (neither of these features would of course be required for an initial implementation of these new backends, just collecting some ideas here).

@sampsyo
Copy link
Member

sampsyo commented Mar 22, 2021

All this sounds totally reasonable, including the separate subsections in the config! I just also wanted to mention that it could even make sense to establish a second plugin, called something like autobpm or whatever, to support methods like aubio. In particular, the manual space-bar-pressing method is so different from any kind of audio analysis that they might have very little in common in terms of implementation, configuration, or interested audience.

Anyway, no strong preference in favor of separation, but just thought I'd mention it as a possibility!

@tandy-1000 tandy-1000 mentioned this issue Sep 24, 2023
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature features we would like to implement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants