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

Don't crash if there's a missing face #13

Closed
wants to merge 1 commit into from

Conversation

kousu
Copy link

@kousu kousu commented Sep 5, 2020

I tried this out and waited over an hour for face_detect() to run, only to have the program crash during actually applying the mels to the faces because of this exception.

This is my quick workaround that got the code to work for me on my sample.

A robust solution would be something more like: dropping the frame (which would mean having to drop the equivalent space from the mels too).

@prajwalkr
Copy link
Collaborator

Your idea is good :) So, you have added a dummy face box, which I do not think is ideal? As it would create artifacts in the frames. It would be good if you can handle the None condition and skip the frame and mel window.

@prajwalkr
Copy link
Collaborator

Please re-open after changes.

@prajwalkr prajwalkr closed this Sep 7, 2020
@kousu
Copy link
Author

kousu commented Sep 7, 2020

Sure. Dropping the mels is going to be a lot more complicated though, and will create its own kinds of artifacts so it's going to take me a lot longer to get around to it. I've got other research projects I'm involved with to take care of.

The advantage of this solution is that the artifacts are limited to a tiny 5x5 region. The only reason I didn't make it 0x0 of 1x1 was because I thought that might cause other exceptions. Do you think it would/wouldn't, by the way?

Thanks for the feedback 👍

@prajwalkr
Copy link
Collaborator

I personally feel that if the code does not handle missing faces smoothly without further issues/artefacts etc., then it is best to let the user crop the video before providing it to the code.

@kousu
Copy link
Author

kousu commented Sep 7, 2020

Here's my use case, to maybe explain where I'm coming from: in the video I discovered this on I did crop it to be a continuous segment with a front-on view. The frame that failed had a face, it was just partially obscured by a hand, including partially covering the mouth.

So the detector failed on it, which is a reasonable thing to do! But it's extremely tedious to ask a user to fix all the frames; I did my best to get a clear shot but I still failed because I didn't realize there would be a problem; the way the code is set up there's no way to be sure what frames will cause failures without running it to completion (which takes between 1 and 60 minutes depending on the video!), and that run has to be done for each failing frame, meaning it's an almost unbounded process (it's bounded by..the number of frames in the video; on colab I was able to process about .5 minutes of video per minute of GPU time, so for n minutes of video it will take 2n minutes to do each fact_detect() and that has to be run for up to each of the 60*60*n frames, giving an upper bound of 2*(60*n)^2 minutes per minute of video to process just to identify the problem frames). And then editing them out is also extremely tedious: the user has to learn a video editing tool like kdenlive or perhaps how to use ffmpeg to crop single frames from a video -- and that only would work if it output the exact index of the problem frames. And that whole process will definitely leave artifacts as the video gains a bunch of jump-cuts.

So rather than cropping a failing frame, I now think it'd be a lot better to just not edit it at all. I can make that happen properly if I sit down and trace it out -- and I am willing to once I fix up a bunch of other research code I'm working on elsewhere. But that solution is very nearly what this quick hack already accomplished.

🤸‍♂️ 🍾 🧮 🏐 🎾 🐰

@prajwalkr
Copy link
Collaborator

Yes, I completely understand! I will try to upload a fix when I am free 👍

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.

None yet

2 participants