Skip to content

Conversation

@Yoz0
Copy link

@Yoz0 Yoz0 commented Nov 15, 2024

This PR add a new feature representation with a fixed number of frames per beats. This is an advancement of issue #154 , but on beats instead of bars.

For the user this adds :

  • A configurable parameter frames_per_beat (int, default 3). The number of frames sampled for each beat.
  • An argument of Feature : multibeat (bool, default False). Whether to use this representation or not.
  • An in line parameter of the run_MSAF.py : -mb. To use this representation.
  • A little bit of documentaion.

In the Feature class :

  • self._est_multibeat_features and self._ann_mutlibeat_features where the representation will be for resp. estimated beats and annotated beats.
  • self._est_multibeat_frames and self._ann_multibeat_frames with the frame index of the beats. (We remove the last beat from self._est_beatsync_frames, because we compute frames between beats)
  • self._est_multibeat_times and self._ann_multibeat_times with the times in secondes of the beats.
  • _compute_multibeat(self, beat_frames) to compute the index of the frames used for the representation
  • _shape_beatwise(self, multibeat_features) to stack the feature representation as a beatwise TF matrix.
  • It also does some refactoring of compute_beat_sync_features in order to use the same function to compute multibeat features. The padding of beat_times is moved to a new function called _pad_beats_times
  • Two new FeatureTypes : est_multibeat and ann_multibeat and the appropriate code in select_features(...)
  • A new Exception FramesPerBeatTooHigh raised by '_compute_multibeat()if the number of frames between two beats is lower thanframes_per_beat`.
  • Tests of the functions in test_multibeat.py

Thanks for reading, and reviewing code 😃

Copy link
Owner

@urinieto urinieto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What a great PR! This is a lot, but I think this is the cleanest way without having to do a massive refactoring on metafeatures. I just left a couple of minor comments regarding documentation. I am running the tests now with Github Actions. If they pass, I will merge away after the docstring typos/requests are addressed. Thanks!

@urinieto
Copy link
Owner

urinieto commented Apr 5, 2025

Apologies, I realized I never get back on this! Some of the tests didn't pass. @Yoz0 , could you please make sure all tests pass? Once they do, I'll merge this PR. Thanks!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants