APIs - 4.1.0a1 #499

CarlGao4 · 2023-05-24T11:52:09Z

No description provided.

adefossez

Thanks a lot for all the hard work! Sorry for not commenting before. See my comments, basically aim for a simpler, more compact API, you don't have to support every possible use case!! aim for simplicity and covering 90% of the use case with much simpler code!

demucs/api.py

CarlGao4 · 2023-06-02T15:17:10Z

I'll update it next week as I have no time this week

CarlGao4 · 2023-06-02T15:25:14Z

To sum up, the module would look like this:

class Separator
    method __init__
    method separate_audio_file
    method separate_audio_tensor
    method _load_model
    method _load_audio
    method _separate_audio
function save_audio

….0a1

adefossez

Looks super nice ! we are getting really close :)

adefossez · 2023-06-20T14:55:44Z

README.md

@@ -257,6 +257,8 @@ import shlex
 demucs.separate.main(shlex.split('--mp3 --two-stems vocals -n mdx_extra "track with space.mp3"'))
 ```

+To use more complicated APIs, see [API docs](docs/api.md)


The API should be the preferred way :) The shlex thing is a huge hack. If the API is well done we should totally replace those instructions.

Maybe this API can be kept so users can easier separate an audio without learning how the API works inside another program

demucs/api.py

demucs/apply.py

adefossez · 2023-06-21T15:45:35Z

demucs/api.py

+        callback: A function will be called when the separation of a chunk starts or finished. \
+            The argument passed to the function will be a dict. For more information, please see \
+            the Callback section.
+        callback_arg: A dict containing private parameters to be passed to callback function. For \


so if we would follow the minimalistic approach, we could get rid of that, that would simplify the _replace_dict logic. Basically if someone wants extra info, they can use partial from functools, e.g.

from functools import partial def callback(args_passed_by_demucs_api, ..., my_arg): ... api.update_parameters(callback=partial(callback, my_arg=42))

But what if the dictionary key can't be a keyword argument? e.g. begins with numbers like 1st

but that wouldn't be a valid argument to the callback then no ?

The argument of callback is a dict, not keyword arguments.

OK I'll remove callback_arg

I've just realized that my implementation of callback relies on callback_args, so I don't know how to make it better without callback_args

demucs/api.py

demucs/apply.py

adefossez · 2023-06-21T15:48:18Z

docs/api.md

+
+##### Notes for callback
+
+The function will be called with only one positional parameter whose type is `dict`. The `callback_arg` will be combined with information of current separation progress. The progress information will override the values in `callback_arg` if same key has been used.


I'm a bit worried about the pool processing and callback logic, from the look of it it seems like the callback function might get called from different threads which could be a bit dangerous ?

I'll add a thread lock
By the way, how can I abort the separation process using callback? Maybe once the callback returns "break", apply_model will return None? But whether this is capable of pool processing?

You can just raise an exception inside the callback, like raise RuntimeError("Interrupted computation")

I just want to make sure that the thread pool will be stopped altogether

Use _NotProvided to allow passing `None` Add returing "break" to stop separation Make callback thread-safe

adefossez · 2023-08-02T21:51:36Z

Two last points: don't drop the callback if not callable, and unsafe usage of lock (use a with statement).

Sorry for being a pain, this is such a sensitive part of the code I don't want to end up with some hard to chase bug down the line :)

CarlGao4 · 2023-08-04T14:49:51Z

Sorry but I can't understand what you mean. If callback is not callable, then an exception will be raised like this: TypeError: 'NoneType' object is not callable
How can you solve this? Besides, what does "unsafe usage of lock" mean? (I've never used lock in Python before)

Directly calls audio.save_audio Changing parameter `sr` (samplerate) to optional argument Fixes linter

hoonlight · 2023-08-29T10:17:18Z

@adefossez Hi, I was wondering if you could provide any additional comments or help with this PR? I'm looking forward to this PR being merged.

CarlGao4 · 2023-09-21T00:26:48Z

@adefossez Could you please provide your latest suggestions to this PR?

adefossez

Looking great thanks so much for the final changes @CarlGao4, and thanks for keeping pushing this, I know this was a bit long :)
I will wait a bit before pushing to PyPI to see if any one comes up with some issues.

demucs/api.py

demucs/apply.py

APIs - 4.1.0a1 (facebookresearch#499)

CarlGao4 added 24 commits May 10, 2023 09:58

Allow manually run workflow

0517243

Add HTDemucs to AnyModel

0b55014

Set minimum Python version to 3.8

241ce88

Add type hints for save_audio

7434ce7

Update segment machenism

2e44697

Update help message that htdemucs is the default

eced443

Add segment test

e292512

Fix linter

df5fb96

API

8e42a11

Update separate.py to use api

1b844c3

Version 4.1.0a1

166ed8e

Update api.md

2c47857

Make parameter repo clearer

6c8523a

Add doc for loading existing audio in quick start

cc9fdd6

Fix typo in docs

29d6bfd

Add flac output

a3bc05d

Fix linter

bfdb0f2

Merge branch 'main' into 4.0.1a1

2801a90

Fix conflicts

a02c866

Fix wrong indent

6dbc3f8

import htdemucs

b0dc401

import htdemucs

626729d

rename variable

f6d61c1

Fix variable name

b082f52

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label May 24, 2023

CarlGao4 added 5 commits May 24, 2023 20:04

Fix mypy linting

436ba3d

Fix mypy linting again

94c4bcb

flac output & max_allowed_segment only for HTDemucs

e19f270

Check max_allowed_segment inside API

b1abd68

Fix codes forgot to change

ace733d

fix linter

b816810

adefossez suggested changes Jun 2, 2023

View reviewed changes

CarlGao4 added 3 commits June 15, 2023 17:52

Make API simpler

475eb90

Merge branch '4.1.0a1' of https://github.com/CarlGao4/demucs into 4.1…

543d141

….0a1

Merge branch 'main' into 4.1.0a1

26ca122

CarlGao4 requested a review from adefossez June 15, 2023 10:03

Optimise code and fix separate api returns different wave

085df79

adefossez suggested changes Jun 21, 2023

View reviewed changes

Fixes according to review

b432ede

Use _NotProvided to allow passing `None` Add returing "break" to stop separation Make callback thread-safe

CarlGao4 requested a review from adefossez June 25, 2023 14:47

CarlGao4 added 4 commits July 25, 2023 15:26

Allow changing remote_root when listing models

839089b

revert changes

0f1f45c

return file path for list_models

7779789

fix Typing

21855b5

CarlGao4 mentioned this pull request Jul 26, 2023

Option to return numpy or torch data directly without serialization to disk? #533

Open

CarlGao4 added 2 commits August 4, 2023 23:09

Merge branch 'main' into 4.1.0a1

9bcf7d0

Minor fixes with linter

9957e2c

Directly calls audio.save_audio Changing parameter `sr` (samplerate) to optional argument Fixes linter

CarlGao4 added 4 commits September 21, 2023 02:08

Fix lock; use KeyboardInterrupt to abort

576a5a5

Fix Linter

2be8e11

Update doc

f789ca3

List model argument

733cb25

adefossez approved these changes Sep 22, 2023

View reviewed changes

demucs/api.py Show resolved Hide resolved

demucs/apply.py Show resolved Hide resolved

demucs/apply.py Outdated Show resolved Hide resolved

adefossez merged commit 0fa7d45 into facebookresearch:main Sep 22, 2023
3 checks passed

taozhang77 added a commit to music-being/demucs that referenced this pull request Nov 7, 2023

Merge pull request #1 from facebookresearch/main

e4ce69f

APIs - 4.1.0a1 (facebookresearch#499)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

APIs - 4.1.0a1 #499

APIs - 4.1.0a1 #499

CarlGao4 commented May 24, 2023

adefossez left a comment

CarlGao4 commented Jun 2, 2023

CarlGao4 commented Jun 2, 2023 •

edited

Loading

adefossez left a comment

adefossez Jun 20, 2023

CarlGao4 Jul 31, 2023

adefossez Jun 21, 2023

CarlGao4 Jun 23, 2023

adefossez Jul 11, 2023

CarlGao4 Jul 11, 2023

CarlGao4 Sep 21, 2023

CarlGao4 Sep 21, 2023

adefossez Jun 21, 2023

CarlGao4 Jun 23, 2023

adefossez Jul 11, 2023

CarlGao4 Jul 11, 2023

adefossez commented Aug 2, 2023

CarlGao4 commented Aug 4, 2023

hoonlight commented Aug 29, 2023

CarlGao4 commented Sep 21, 2023

adefossez left a comment


		##### Notes for callback

		The function will be called with only one positional parameter whose type is `dict`. The `callback_arg` will be combined with information of current separation progress. The progress information will override the values in `callback_arg` if same key has been used.

APIs - 4.1.0a1 #499

APIs - 4.1.0a1 #499

Conversation

CarlGao4 commented May 24, 2023

adefossez left a comment

Choose a reason for hiding this comment

CarlGao4 commented Jun 2, 2023

CarlGao4 commented Jun 2, 2023 • edited Loading

adefossez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adefossez commented Aug 2, 2023

CarlGao4 commented Aug 4, 2023

hoonlight commented Aug 29, 2023

CarlGao4 commented Sep 21, 2023

adefossez left a comment

Choose a reason for hiding this comment

CarlGao4 commented Jun 2, 2023 •

edited

Loading