fumiama / Retrieval-based-Voice-Conversion-WebUI

Easily train a good VC model with voice data <= 10 mins!
GNU Affero General Public License v3.0
74 stars 10 forks source link

Suggestion: Get rid of running python scripts in shell #49

Open alexlnkp opened 1 month ago

alexlnkp commented 1 month ago

The title might be a bit confusing, but what i'm referring to is how some python scripts are ran in a subshell, even though they could be used as regular python modules.

cmd = (
    '"%s" infer/modules/train/extract/extract_f0_print.py "%s/logs/%s" %s %s'
    % (
        config.python_cmd,
        now_dir,
        exp_dir,
        n_p,
        f0method,
    )
)
logger.info("Execute: " + cmd)
p = Popen(
    cmd, shell=True, cwd=now_dir
)  # , stdin=PIPE, stdout=PIPE,stderr=PIPE

Is something that is present A LOT in the web.py file, but i fail to understand why even do that if you can create a job in the python that executes code, instead of opening a shell as a subprocess.

I assume this will be fixed when the RVC becomes a module, however I'll prefer fixing this sooner than later.

Opening a subshell and executing a python script adds a huge amount of overhead and introduces extreme amounts of security vulnerabilities, shell=True is one of the culprits for that in particular.

Also, the code will look just more appealing without all of the subprocess creation within shells and all the yields of the stdout as a generator for gradio interface to update properly

fumiama commented 1 month ago

but i fail to understand why even do that if you can create a job in the python that executes code

The initial author do not know how to do that. He just want a subprocess to run some code asynchronously.

fumiama commented 1 month ago

Also, the code will look just more appealing without all of the subprocess creation

Yes. And it will make it possible to add a button break in gradio.

alexlnkp commented 1 month ago

Also, the code will look just more appealing without all of the subprocess creation

Yes. And it will make it possible to add a button break in gradio.

That reminds me of my Mangio-RVC-Fork and how painful it was to add the stop training button :joy:

fumiama commented 1 month ago

how painful it was to add the stop training button

Yes 😂. So very important to change the code structure. It is the base to the further improvement.