-
Notifications
You must be signed in to change notification settings - Fork 202
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
Online beat tracking #238
Online beat tracking #238
Conversation
4e9b7dc
to
9acce81
Compare
1e8cd39
to
421fa8f
Compare
396a526
to
9a12ea8
Compare
ab254af
to
6828ee7
Compare
6828ee7
to
5537d5f
Compare
5537d5f
to
65c1eb3
Compare
ef0b559
to
b677a0f
Compare
65c1eb3
to
819f1c9
Compare
b677a0f
to
47499ba
Compare
819f1c9
to
c319108
Compare
47499ba
to
eefb714
Compare
c319108
to
e7b07c2
Compare
eefb714
to
afc418d
Compare
e7b07c2
to
b0fb477
Compare
109d05b
to
6a525d1
Compare
30c1710
to
576a884
Compare
5ba02a5
to
3afaab0
Compare
3afaab0
to
e14ad8b
Compare
576a884
to
38aed8f
Compare
e14ad8b
to
6fa56fd
Compare
6fa56fd
to
bceab16
Compare
3b72ece
to
735d438
Compare
38aed8f
to
b95bd7d
Compare
b95bd7d
to
18dbbd3
Compare
18dbbd3
to
b6d5051
Compare
Looking forward to using this! I'm testing it out but here's what happens:
|
Did you update the models? Please run |
I did try that (but it didn't work):
|
Ah, I forgot to include the models in |
Awesome, thanks! It's working now. |
Thanks for testing :) Will fix it and (hopefully) release v0.15 very soon. |
197a78a
to
42f341b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool beans! 👍
Just some minor comments....
docs/usage.rst
Outdated
|
||
If no output directory is given, the program writes the output files to same | ||
location as the audio files. | ||
location as the the audio files. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo :)
@@ -48,38 +49,47 @@ class RNNBeatProcessor(SequentialProcessor): | |||
|
|||
""" | |||
|
|||
def __init__(self, post_processor=average_predictions, **kwargs): | |||
def __init__(self, post_processor=average_predictions, online=False, | |||
nn_files=None, **kwargs): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Is there any use case for the
nn_files
parameter? - Does it make sense to have the
nn_files
parameter if there is noframe_sizes
parameter? As I see it, only neural networks trained on the given frame sizes (either for online or offline mode) would work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Although you're right about which model can be used (only trained specifically for/with this settings), it can be quite useful to select only one or two of the trained models if processing power is limited.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand. But then this use case should be explained in the docs, I think it's not obvious from the beginning. Now that I've checked, the online
parameter is also missing from the docstring of the class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added docstrings and doctests.
@@ -92,6 +92,9 @@ def process(self, data, reset=True, **kwargs): | |||
Network predictions for this data. | |||
|
|||
""" | |||
# make data at least 2d (required by NN-layers) | |||
if data.ndim < 2: | |||
data = np.array(data, subok=True, copy=False, ndmin=2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just out of curiosity, is this better than np.atleast_2d
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it is way faster on all machines I tested. I only spotted this when profiling the whole online beat tracking stuff, and it had a quite remarkable impact.
42f341b
to
38a0dd9
Compare
Rebased everything on top of #185.Before this PR can be merged, a couple of things need to be addressed:
process()
method