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

Improving TrainingDataGenerator, sort tool and fixes #307

Closed
wants to merge 9 commits into from
Closed

Improving TrainingDataGenerator, sort tool and fixes #307

wants to merge 9 commits into from

Conversation

iperov
Copy link
Contributor

@iperov iperov commented Mar 22, 2018

Sort tool:

fixed 'No faces were detected'
speed of face sorting much increased, because 256x256 face already "detected", so no need to detect it via dlib again

removed 'face' as face_recognition , and changed 'face-cnn' to 'face', because face-cnn better and faster and works with 3GB vram.

added --sort-by face-yaw
result:
fsviewer_2018-03-22_14-16-17
fsviewer_2018-03-22_14-17-13

Improving TrainingDataGenerator:

now we can properly train model with tens of thousands of unevenly distributed by rotation faces

idea:

photoshop_2018-03-22_14-20-48

training_data.py automatically generates alignments_yaw.json in aligned\..\ folder
by launching yaw_sorter.py as fully separated python process but with same stdout, so no additional VRAM of main process consumed

@iperov
Copy link
Contributor Author

iperov commented Mar 22, 2018

training with 23400 source faces,

rare side angles training fine:
python_2018-03-22_16-19-51

@Clorr
Copy link
Contributor

Clorr commented Mar 22, 2018

Seems fine ;-)

@h1vem1nd85
Copy link

Would it be less redundant to gather this information at the extraction step? Or possibly reading the existing alignments.json landmarks and processing from there? Running the aligned images back through the face detector to get landmarks a second time seems wasteful from an overall workflow perspective.

Also, the code you provided seems to stall at the Regularizing step unless I've done something horribly wrong between point A and point B.

@iperov
Copy link
Contributor Author

iperov commented Mar 22, 2018

Would it be less redundant to gather this information at the extraction step?
impossible, because we gather thousands of faces from various sources and filter them through sorttool.

Also, the code you provided seems to stall at the Regularizing step unless I've done something horribly wrong between point A and point B.
can you debug the problem? because I cannot reproduce the bug

@Enyakk
Copy link

Enyakk commented Mar 22, 2018

Sort Tool:
Face-Cnn (here the default face-sort) works badly on image collections of many people. I find it lumps together different faces a lot and provides less stable results than the old face-detection. This is not unique to this pull request and was already the case for me on the regular build. I recommend keeping the old face mode since it provides functionality not replicated by face-cnn.

@iperov
Copy link
Contributor Author

iperov commented Mar 22, 2018

Face-Cnn (here the default face-sort) works badly on image collections of many people
I recommend keeping the old face mode since it provides functionality not replicated by face-cnn.

rejected
--sort-by face was not created to filter one face from many people
use 'hist' instead

sort by face was created to group together similar positions of ONE face

@h1vem1nd85
Copy link

impossible, because we gather thousands of faces from various sources and filter them through sorttool.

Your current implementation is redundant. Workflow is:
Raw images > extract faces + landmarks > sort faces by extracting landmarks again > batch loader

You've also made the alignments_yaw.json a requirement for any training. If your intention is to run sorttool independently from training I strongly suggest this be a separate generator.

can you debug the problem? because I cannot reproduce the bug

Process runs face detection on 923 aligned faces and writes alignments_yaw.json with no actual data just filenames. Begins Regularization with no data to process, CPU and memory usage locked at 13% 194,412K (single thread, 8 cores). This is a using a direct clone of your repo.

@iperov
Copy link
Contributor Author

iperov commented Mar 22, 2018

sort faces by extracting landmarks again

srsly ?

if you cannot help, just leave this pr, dont flood.

@h1vem1nd85
Copy link

I seriously dont know what set you off on your power trip, but Ive lurked this repo for quite some time and have read enough of your comments to know that you dont accept criticism.

Discussions are meant to be point : counterpoint, not point: nope youre wrong, I'm, right leave

The landmark data you are producing already exists in the original image alignments as XY pixel coords. It isnt unreasonable to have sorttool read and coord transform those coords to the aligned image space, then writing the yaw outputs as you intended.

@iperov
Copy link
Contributor Author

iperov commented Mar 23, 2018

h1vem1nd85, so you purposely came to flood ?
FIRST read the code and understand the logic of the program, before make conclusion.

@Clorr , this man doesnt help at all at this PR. He just flooding.

@iperov
Copy link
Contributor Author

iperov commented Mar 23, 2018

@h1vem1nd85, also, offer a solution.
Fork from my fork, and make better.
If you cannot offer a solution, then you have to stop flood and leave this PR.

Am I wrong, @Clorr ?

@iperov
Copy link
Contributor Author

iperov commented Mar 23, 2018

architecture of faceswap not so good to make new improvements absolutely clear and logical

Copy link
Contributor

@AbysmalBiscuit AbysmalBiscuit left a comment

Choose a reason for hiding this comment

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

Besides the alignments_yaw.json FileNotFoundError, I have encountered another strange bug.

The new/empty alignments_yaw.json file doesn't seem to get populated with the alignments. I'm not sure why exactly, but if I find the reason I'll post it.

Besides that there is also strange behaviour that I have failed to find the cause of.
When the train process gets to the Loading Trainer from Model_Original plugin... stage, it loads the interactive Python shell for some reason. I haven't been able to find in the faceswap code where it gets prompted to do so. My suspicion is that it's due to the alignments_yaw.json file remaining empty, so maybe there's a general except that's masking the error.

Steps to reproduce:
Add the alignments_yaw.json not found fix I added as a comment.
Have a directory setup like:
parent_dir > face_A
parent_dir > face_B
Make sure there is no alignments_yaw.json file in parent_dir or its only contents are {}.
Run the following command:
python faceswap.py train -A <path to parent_dir>/face_A -B <path to parent_dir>/face_B -m <path to model_dir> -p

images_dir = os.path.dirname (images[0])
images_parent_dir = os.path.join ( images_dir, os.pardir )
images_parent_dir_alignments_yaw_json = os.path.join ( images_parent_dir, "alignments_yaw.json" )

Copy link
Contributor

Choose a reason for hiding this comment

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

There is a bug where if there is no alignments_yaw.json in the images_parent_dir, then the while block will throw a FileNotFoundError when trying to run execute_yaw_sorter on line 40.
The alignments_yaw.json can be empty with at least {} as the only contents.
I think this is what the people saying that alignments_yaw.json being required now were referring to.

An easy fix is:

if not os.path.isfile(images_parent_dir_alignments_yaw_json):
    with open(images_parent_dir_alignments_yaw_json, 'w') as f:
        f.write('{}')

Copy link
Contributor Author

Choose a reason for hiding this comment

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

execute_yaw_sorter (images_dir, images_parent_dir_alignments_yaw_json )

will create file, so
not os.path.isfile(images_parent_dir_alignments_yaw_json) will not occur

@iperov
Copy link
Contributor Author

iperov commented Mar 23, 2018

@AbysmalBiscuit you got FileNotFoundError because
execute_yaw_sorter was not executed for some reason, so no file created.
Can you debug why execute_yaw_sorter not executing?

@iperov
Copy link
Contributor Author

iperov commented Mar 23, 2018

it loads the interactive Python shell for some reason
fixed

@AbysmalBiscuit
Copy link
Contributor

@iperov sure I'll look into it. :)

@AbysmalBiscuit
Copy link
Contributor

@iperov I've fixed it by using the shlex module to prepare the arguments for the subprocess, here's a gist with my changes:
https://gist.github.com/AbysmalBiscuit/5dfed6d29c014f8bb74906acea79bb51

The tracebacks always led to subprocess having trouble with processing the argument string, so I think this is a safer way of doing it. :)

I have a few ideas for how the new yaw preparation functionality can be improved (if you like them I can handle the implementation):

  1. Allow users to specify via cli the paths to alignments_yaw.json.

  2. Instead of using a generic alignments_yaw.json file, perhaps use something like alignments_yaw_personA.json and alignments_yaw_personX.json. (Where personA is the data directory given by the user for -A and personX is given by -B.)
    This way you could keep the alignments_yaw files. By keeping them you can save time by avoiding having to reload and sort them.
    The issue of adding and/or removing faces from the data directory can be handled by checking if all faces in the data directory are in the alignments_yaw file corresponding to that directory (which will be a lot faster than reloading all the images as a default).

2.1) If images were removed from the data directory you could simply delete them from the alignments_yaw file and save the new file.

2.2) If images were added; this is trickier and would mean a change to how the alignments_yaw file is saved and processed after being read. My idea would be that after you regularize the images, you sort the image list to match it and save the image list. You could save it in the same format, here's what I mean:
Right now it's a list of lists:
[[<path to img1>, <path to img2>], [<path to img1>]]
Using img_list, list of lists of lists:
[[[<path to img1>, nparray, face_pitch, face_yaw], [<path to img2>, nparray, face_pitch, face_yaw]], [[<path to img3>, nparray, face_pitch, face_yaw]]]
This way when you add new images you have most of the data ready and you only need to load the extra images and resort (with the rest being read from the saved file), which can save time.
Then when you need the alignments_yaw file to be formatted as it is now, you can just reformat it in memory to be of the form that the rest of train expects. Alternitavely you could change the other parts of the train process to get the file path from the lists.

  1. You said that the point of sorting by face isn't to sort 1 face from many, but to group similar positions of 1 face, so would you be willing to add back in the dlib method as sorting by face-dlib since it can be quite useful for when you extract from a video where there are a lot of faces (like a movie scene with people in the background)? This way it wouldn't be the default face method, but it would still be there for those who need it.

@iperov
Copy link
Contributor Author

iperov commented Mar 23, 2018

@AbysmalBiscuit now its not working on windows :)
FileNotFoundError: [WinError 2] The system cannot find the file specified

>>> args
['E:FaceSwap_internalbinpython.exe', 'E:FaceSwap_internalbinfaceswaplibyaw_sorter.py', 'E:\\FaceSwap\\workspace\\data_dst\\aligned', 'E:\\FaceSwap\\workspace\\data_dst\\aligned\\..\\alignments_yaw.json']

@iperov
Copy link
Contributor Author

iperov commented Mar 23, 2018

@AbysmalBiscuit check plz latest commit

@AbysmalBiscuit
Copy link
Contributor

@iperov haha, yeah I did have a feeling that it may not work on widows from when I read the Python documentation while trying to fix it. :p

I'll give the latest commit a check.

@h1vem1nd85
Copy link

@iperov We certainly got off on the wrong foot, what you're doing here is fantastic work and something I hesitated to approach on my own. I misunderstood you intended to store in the alignments_yaw.json as no actual yaw data is saved, but rather a sorted list. I was able to get the last commit to run on a small batch of images so it seems to be functioning as intended.

One thing I noticed is your yaw values range from positive to negative (left or right gaze) but the random_transform in the generator can flip the image causing "duplicate" poses in your batch. This may or may not be a bad thing depending on how targeted you want your training data to be but it led me to my next thought.

I'm investigating sorting by absolute yaw and producing pairs of mirrored test images. This would allow left or right yaw images to be used for both pose directions in training, while also addressing missing pose information (i.e. training data has all left facing images and no right). What are your thoughts?

@iperov
Copy link
Contributor Author

iperov commented Mar 23, 2018

As I said - fork from this fork, and do what you want.

@AbysmalBiscuit
Copy link
Contributor

@iperov I tested the commit and it works fine now. :)

Any ideas about the other things I mentioned? Or should I also just go ahead and fork and have a go at implementing them. :p

@iperov
Copy link
Contributor Author

iperov commented Mar 23, 2018

@AbysmalBiscuit

alignments_yaw_personA.json and alignments_yaw_personX.json

whats this ?

my folder structure:

workspace
  data_src
    alignments_yaw.json
    aligned
      256x256faces*.png
  data_dst
    videoimages*.png
    alignments.json
    alignments_yaw.json
    aligned
      256x256faces*.png
  model

alignments.json for data_src absolutely useless and unused in production

alignments.json for data_dst actually need only for merge to detect rect in videoimages*.png where need to change face.

@iperov
Copy link
Contributor Author

iperov commented Mar 23, 2018

I collecting faces to data_src from various videos, so all your offers about storing data in alignments.json are pontless.

@iperov
Copy link
Contributor Author

iperov commented Mar 24, 2018

dont merge this PR, I decided it is only for testing, because whole architecture must be refactored first.

@iperov iperov closed this Mar 24, 2018
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.

5 participants