Skip to content
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

Resumable processing #430

Closed
wants to merge 6 commits into from
Closed

Resumable processing #430

wants to merge 6 commits into from

Conversation

pozitronik
Copy link
Contributor

@pozitronik pozitronik commented Jun 11, 2023

#377
Much simpler approach than before (#408).
It is possible to determine the processing status from the state of the target temporary directory. The logic is simple: we mark already processed files by changing their names, so it is always known whether the processing has started and what remains.

This approach is also compatible with any types of processors. Also, you can change processing parameters on the fly without any consequences.

@pozitronik pozitronik marked this pull request as draft June 11, 2023 08:25
@pozitronik pozitronik marked this pull request as ready for review June 11, 2023 09:24
roop/core.py Outdated
update_status('Extracting frames...')
extract_frames(roop.globals.target_path)
# temp directory with frames is existed and all frames are extracted (or some already processed)
if state.is_exists(roop.globals.target_path) or state.unprocessed_frames_count(roop.globals.target_path) == detect_fc(roop.globals.target_path):
Copy link
Contributor

Choose a reason for hiding this comment

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

That needs to be one function like state.is_resumable()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

roop/core.py Outdated
extract_frames(roop.globals.target_path)
# temp directory with frames is existed and all frames are extracted (or some already processed)
if state.is_exists(roop.globals.target_path) or state.unprocessed_frames_count(roop.globals.target_path) == detect_fc(roop.globals.target_path):
update_status(f'Temp resources for this target already exists with {state.processed_frames_cnt} frames processed, continue processing...')
Copy link
Contributor

Choose a reason for hiding this comment

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

Please do not use shortcuts in our code... there is no idx, cnt or err allowed...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

roop/core.py Outdated
@@ -237,7 +244,7 @@ def start() -> None:


def destroy() -> None:
if roop.globals.target_path:
if roop.globals.target_path and state.is_done(roop.globals.target_path) and state.unprocessed_frames_count(roop.globals.target_path) != detect_fc(roop.globals.target_path):
Copy link
Contributor

Choose a reason for hiding this comment

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

That needs to be a single function as awell like state.is_unfinished()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

roop/enhancer.py Outdated
@@ -180,8 +183,8 @@ def multi_process_frame(source_img, frame_paths, progress) -> None:

def process_video(source_path: str, frame_paths: list[str], mode: str) -> None:
progress_bar_format = '{l_bar}{bar}| {n_fmt}/{total_fmt} [{elapsed}<{remaining}, {rate_fmt}{postfix}]'
total = len(frame_paths)
with tqdm(total=total, desc='Processing', unit='frame', dynamic_ncols=True, bar_format=progress_bar_format) as progress:
total = len(frame_paths) + state.processed_frames_cnt
Copy link
Contributor

Choose a reason for hiding this comment

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

the state has to provide a method like state.count_unfinished_frames()

Copy link
Contributor Author

@pozitronik pozitronik Jun 11, 2023

Choose a reason for hiding this comment

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

I tried to avoid the usage of global variables here. But it does not really matters.

roop/state.py Outdated


def is_exists(target_path: str) -> bool:
"""
Copy link
Contributor

@henryruhs henryruhs Jun 11, 2023

Choose a reason for hiding this comment

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

Sorry but all the comments have to go... I reject to read them - if a method needs a paragraph to be explained, the method name is just bad.

The name is-exist does not tell me what exists... so I need to read your comment for sure. But this can actual be resolved by having a better name like has_unfinished_frames() or has_resumable_temp.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

roop/swapper.py Outdated
@@ -52,7 +53,8 @@ def process_frames(source_path: str, temp_frame_paths: List[str], progress=None)
temp_frame = cv2.imread(temp_frame_path)
try:
result = process_faces(source_face, temp_frame)
cv2.imwrite(temp_frame_path, result)
cv2.imwrite(state.get_frame_processed_name(temp_frame_path), result)
Copy link
Contributor

Choose a reason for hiding this comment

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

It would make send to add a variable that points to state.get_frame_processed_name(temp_frame_path) so the reader can easier understand what it contains

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

roop/enhancer.py Outdated
@@ -149,7 +151,8 @@ def process_frames(source_path: str, frame_paths: list[str], progress=None) -> N
try:
frame = cv2.imread(frame_path)
result = process_faces(source_face, frame)
cv2.imwrite(frame_path, result)
cv2.imwrite(state.get_frame_processed_name(frame_path), result)
Copy link
Contributor

Choose a reason for hiding this comment

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

It would make send to add a variable that points to state.get_frame_processed_name(temp_frame_path) so the reader can easier understand what it contains

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

roop/swapper.py Outdated
@@ -90,8 +92,8 @@ def process_image(source_path: str, target_path: str, output_path: str) -> None:

def process_video(source_path: str, temp_frame_paths: List[str], mode: str) -> None:
progress_bar_format = '{l_bar}{bar}| {n_fmt}/{total_fmt} [{elapsed}<{remaining}, {rate_fmt}{postfix}]'
total = len(temp_frame_paths)
with tqdm(total=total, desc='Processing', unit='frame', dynamic_ncols=True, bar_format=progress_bar_format) as progress:
total = len(temp_frame_paths) + state.processed_frames_cnt
Copy link
Contributor

Choose a reason for hiding this comment

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

Same like in enhancer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -34,6 +35,23 @@ def detect_fps(target_path: str) -> int:
return 30


def detect_fc(target_path: str) -> int:
Copy link
Contributor

@henryruhs henryruhs Jun 11, 2023

Choose a reason for hiding this comment

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

Rename to detect_frame_count or get_total_frames

I think this belongs better to capturer.py and should not be done via ffmpeg but cv2.VideoCapture(video_path)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

roop/state.py Outdated
from roop import utilities

PROCESSED_PREFIX = '_'
processed_frames_cnt = 0
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a fan of this global counter... it should be calculated via a method everytime

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, I would prefer to make this counter as a class attribute (probably accessed through a getter). But since classes are not used here, I used a global variable. Anyway, it doesn't matter.

@@ -54,7 +72,8 @@ def restore_audio(target_path: str, output_path: str) -> None:

def get_temp_frame_paths(target_path: str) -> List[str]:
temp_directory_path = get_temp_directory_path(target_path)
return glob.glob(os.path.join(temp_directory_path, '*.png'))
return [os.path.join(temp_directory_path, file) for file in os.listdir(temp_directory_path) if
Copy link
Contributor

Choose a reason for hiding this comment

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

can we simplify this condition using any? I can barely understand this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Comment on lines 75 to 76
return [os.path.join(temp_directory_path, file) for file in os.listdir(temp_directory_path) if
not file.startswith(state.PROCESSED_PREFIX) and file.endswith(".png")]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
return [os.path.join(temp_directory_path, file) for file in os.listdir(temp_directory_path) if
not file.startswith(state.PROCESSED_PREFIX) and file.endswith(".png")]
return [file for file in glob.glob(os.path.join(temp_directory_path, '*.png')) if not os.path.basename(file).startswith(state.PROCESSED_PREFIX)]

Is this better?

@henryruhs henryruhs changed the title Issue 377 rethink Resumable processing Jun 11, 2023
@henryruhs
Copy link
Contributor

@pozitronik Please resolve the conflicts

@pozitronik
Copy link
Contributor Author

@pozitronik Please resolve the conflicts

Done.
I've looked at the new code while merging and want to say it's excellent work! Keep it up!

@henryruhs henryruhs deleted the branch s0md3v:next June 20, 2023 20:07
@henryruhs henryruhs closed this Jun 20, 2023
@pozitronik pozitronik deleted the issue_377_rethink branch June 29, 2023 09:36
kurtarma34 pushed a commit to kurtarma34/roop that referenced this pull request Aug 25, 2024
* Rename landmark 5 variables

* Mark as NEXT

* Render tabs for multiple ui layout usage

* Allow many face detectors at once, Add face detector tweaks

* Remove face detector tweaks for now (kinda placebo)

* Fix lint issues

* Allow rendering the landmark-5 and landmark-5/68 via debugger

* Fix naming

* Convert face landmark based on confidence score

* Convert face landmark based on confidence score

* Add scrfd face detector model (#397)

* Add scrfd face detector model

* Switch to scrfd_2.5g.onnx model

* Just some renaming

* Downgrade OpenCV, Add SYSTEM_VERSION_COMPAT=0 for MacOS

* Improve naming

* prepare detect frame outside of semaphore

* Feat/process manager (#399)

* Minor naming

* Introduce process manager to start and stop

* Introduce process manager to start and stop

* Introduce process manager to start and stop

* Introduce process manager to start and stop

* Introduce process manager to start and stop

* Remove useless test for now

* Avoid useless variables

* Show stop once is_processing is True

* Allow to stop ffmpeg processing too

* Implement output image resolution (#403)

* Implement output image resolution

* Reorder code

* Simplify output logic and therefore fix bug

* Frame-enhancer-onnx (s0md3v#404)

* changes

* changes

* changes

* changes

* add models

* update workflow

* Some cleanup

* Some cleanup

* Feat/frame enhancer polishing (#410)

* Some cleanup

* Polish the frame enhancer

* Frame Enhancer: Add more models, optimize processing

* Minor changes

* Improve readability of create_tile_frames and merge_tile_frames

* We don't have enough models yet

* Feat/face landmarker score (#413)

* Introduce face landmarker score

* Fix testing

* Fix testing

* Use release for score related sliders

* Reduce face landmark fallbacks

* Scores and landmarks in Face dict, Change color-theme in face debugger

* Scores and landmarks in Face dict, Change color-theme in face debugger

* Fix some naming

* Add 8K support (for whatever reasons)

* Fix testing

* Using get() for face.landmarks

* Introduce statistics

* More statistics

* Limit the histogram equalization

* Enable queue() for default layout

* Improve copy_image()

* Fix error when switching detector model

* Always set UI values with globals if possible

* Use different logic for output image and output video resolutions

* Enforce re-download if file size is off

* Remove unused method

* Remove unused method

* Remove unused warning filter

* Improved output path normalization (s0md3v#419)

* Handle some exceptions

* Handle some exceptions

* Cleanup

* Prevent countless thread locks

* Listen to user feedback

* Fix webp edge case

* Feat/cuda device detection (s0md3v#424)

* Introduce cuda device detection

* Introduce cuda device detection

* it's gtx

* Move logic to run_nvidia_smi()

* Finalize execution device naming

* Finalize execution device naming

* Merge execution_helper.py to execution.py

* Undo lowercase of values

* Undo lowercase of values

* Finalize naming

* Add missing entry to ini

* fix lip_syncer preview (#426)

* fix lip_syncer preview

* change

* Refresh preview on trim changes

* Cleanup frame enhancers and remove useless scale in merge_video() (s0md3v#428)

* Keep lips over the whole video once lip syncer is enabled (s0md3v#430)

* Keep lips over the whole video once lip syncer is enabled

* changes

* changes

* Fix spacing

* Use empty audio frame on silence

* Use empty audio frame on silence

* Fix ConfigParser encoding (#431)

facefusion.ini is UTF8 encoded but config.py doesn't specify encoding which results in corrupted entries when non english characters are used. 

Affected entries:
source_paths
target_path
output_path

* Adjust spacing

* Improve the GTX 16 series detection

* Use general exception to catch ParseError

* Use general exception to catch ParseError

* Host frame enhancer models4

* Use latest onnxruntime

* Minor changes in benchmark UI

* Different approach to cancel ffmpeg process

* Add support for amd amf encoders (#433)

* Add amd_amf encoders

* remove -rc cqp from amf encoder parameters

* Improve terminal output, move success messages to debug mode

* Improve terminal output, move success messages to debug mode

* Minor update

* Minor update

* onnxruntime 1.17.1 matches cuda 12.2

* Feat/improved scaling (#435)

* Prevent useless temp upscaling, Show resolution and fps in terminal output

* Remove temp frame quality

* Remove temp frame quality

* Tiny cleanup

* Default back to png for temp frames, Remove pix_fmt from frame extraction due mjpeg error

* Fix inswapper fallback by onnxruntime

* Fix inswapper fallback by major onnxruntime

* Fix inswapper fallback by major onnxruntime

* Add testing for vision restrict methods

* Fix left / right face mask regions, add left-ear and right-ear

* Flip right and left again

* Undo ears - does not work with box mask

* Prepare next release

* Fix spacing

* 100% quality when using jpg for temp frames

* Use span_kendata_x4 as default as of speed

* benchmark optimal tile and pad

* Undo commented out code

* Add real_esrgan_x4_fp16 model

* Be strict when using many face detectors

---------

Co-authored-by: Harisreedhar <46858047+harisreedhar@users.noreply.github.com>
Co-authored-by: aldemoth <159712934+aldemoth@users.noreply.github.com>
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