feat(gui): run inference in a separate process and temporarily disable button

[Lordmau5]

This PR will change the "Infer" button to run in a thread utilizing PySimpleGui's window.start_thread method.

While this is going on, the "Infer" button will be disabled until inference is completed (or fails due to an exception)

This will prevent the GUI from freezing up and will allow the user to already prepare the settings for the next inference

---

Added another commit fixing the "Infer" button from disabling a bit too late, potentially making it possible to quickly run it multiple times in a row (e.g. spam clicking)

[34j]

Wouldn't it be better to throw the whole thing to the pool?

[Lordmau5]

Wouldn't it be better to throw the whole thing to the pool?

I've tried doing that just now but it seems I can't update the window variable properly this way...
I have a separate function (since lambdas don't work with it) and it isn't updating the UI.
Perhaps because it's on another thread?

Apparently the function is just... never called at all. I added a LOG.info call and it's just not being printed to the console...

def enable_infer_button(window: sg.Window):
    window["infer"].update(disabled=False)

Further down in the event handling:

                    LOG.info("Starting inference...")
                    window["infer"].update(disabled=True)
                    pool.schedule(
                        infer,
                        kwargs=dict(
                            # # window
                            # window=window,
                            # paths
                            model_path=Path(values["model_path"]),
                            output_path=output_path,
                            input_path=input_path,
                            config_path=Path(values["config_path"]),
                            # svc config
                            speaker=values["speaker"],
                            cluster_model_path=Path(values["cluster_model_path"])
                            if values["cluster_model_path"]
                            else None,
                            transpose=values["transpose"],
                            auto_predict_f0=values["auto_predict_f0"],
                            cluster_infer_ratio=values["cluster_infer_ratio"],
                            noise_scale=values["noise_scale"],
                            f0_method=values["f0_method"],
                            # slice config
                            db_thresh=values["silence_threshold"],
                            pad_seconds=values["pad_seconds"],
                            chunk_seconds=values["chunk_seconds"],
                            absolute_thresh=values["absolute_thresh"],
                            device="cpu" if not values["use_gpu"] else get_optimal_device(),
                        )
                    )
                    pool.schedule(
                        enable_infer_button,
                        args=[window]
                    )
                    if values["auto_play"]:
                        pool.schedule(play_audio, args=[output_path])

[34j]

I meant that there would be no particular need to disable the button... what do you think?

[Lordmau5]

Okay so I moved the function around, maybe it was because I was lacking 2 newlines inbetween, and now it's working properly with the pool.

I've also noticed ThreadPool is faster than ProcessPool.

I meant that there would be no particular need to disable the button... what do you think?

Well, in theory there isn't, no. However, I think it might be a good visual indicator for the user to know that something is still going on.

If we don't disable the button, and instead just schedule further inferences afterwards, the user has to wait until the first inference is done until it get's to the next one.
So they might accidentally infer the exact same file a 2nd time, resulting in it getting overwritten.

[codecov-commenter]

Codecov Report

Merging #279 (c6965c2) into main (7ed71d6) will decrease coverage by 0.12%.
The diff coverage is 7.14%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main     #279      +/-   ##
==========================================
- Coverage   20.51%   20.39%   -0.12%     
==========================================
  Files          38       38              
  Lines        3247     3280      +33     
  Branches      417      425       +8     
==========================================
+ Hits          666      669       +3     
- Misses       2564     2593      +29     
- Partials       17       18       +1     

Impacted Files	Coverage Δ
src/so_vits_svc_fork/gui.py	0.00% <0.00%> (ø)
src/so_vits_svc_fork/__init__.py	100.00% <100.00%> (ø)

... and 4 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[34j]

I think this change will cause 3 tasks to run at the same time and not work properly.

[Lordmau5]

I think this change will cause 3 tasks to run at the same time and not work properly.

Which tasks in particular?
In my testing it always did infer, enable button, and then play audio in that order. Not at the same time.
max_workers=1 ensures only 1 can run at a time in that pool

Unless you mean different tasks?

[Lordmau5]

Also added a proper log call for when it's finished with the inference which will look something lke this:

[Lordmau5]

I reverted back to ProcessPool as ThreadPool gave issues with the realtime inference (it didn't seem to want to end the VC instance)

However, apparently now it doesn't want to schedule the finish_inference method anymore... Not even the LOG.info inside it...
(Man I hate Python)

[Lordmau5]

However, apparently now it doesn't want to schedule the finish_inference method anymore... Not even the LOG.info inside it...
(Man I hate Python)

Alright, managed to fix that by utilizing the future's add_done_callback.
I have no idea why Pebble is deciding to not want to run that method but hey, this works.

---

I'll test it a bit further and once I feel it's ready I'll merge all commits into a single one so it's better for a potential merge.

[34j]

Is this now complete? Can it be merged?

[Lordmau5]

It should be complete, yes, but I can do one last test once I'm back home in around 2 hours if you'd like me to.