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

WSL fix for terminate process in audio #864

Closed
wants to merge 1 commit into from
Closed

WSL fix for terminate process in audio #864

wants to merge 1 commit into from

Conversation

OgulcanCelik
Copy link

@OgulcanCelik OgulcanCelik commented Oct 22, 2018

added @ryanfox's fix (#861) to the audio:

Prevents proc.terminate() from throwing an exception when the process has already terminated.

It's the final fix for #765 I believe.

  • If this is a bugfix, I have provided code that clearly demonstrates the problem and that works when used with this PR
  • I have added a test to the test suite, if necessary
  • I have properly documented new or changed features in the documention, or the docstrings
  • I have properly documented unusual changes to the code in the comments around it
  • I have made note of any breaking/backwards incompatible changes

phiomy pushed a commit to phiomy/youtube-video-face-swap that referenced this pull request Jan 25, 2019
phiomy pushed a commit to phiomy/youtube-video-face-swap that referenced this pull request Jan 25, 2019
phiomy pushed a commit to phiomy/youtube-video-face-swap that referenced this pull request Jan 25, 2019
@OgulcanCelik
Copy link
Author

Is Travis build stuck?

I'm using this fix by myself since October and did not come up with any issues so far.

@Overdrivr
Copy link
Collaborator

@OgulcanCelik For some reason the notification did not pass through, build seems fine.

However, there are plenty of test cases that load VideoFileClip and Travis build runs fine on master, so it's possible that:

  • this has already been fixed on master, possible with Fix for #926 #927
  • this is specific to certain files, in which case we should have a test case for reproducing the issue and making sure it's fixed

Could you have a look ? Thanks for your contribution

@OgulcanCelik
Copy link
Author

@Overdrivr
Hello,
Just checked and yeah it seems like the issue on WSL fixed with #927. However, I guess pip package isn't updated yet, so I tried with manually implementing the fix, works fine. Thanks!

@Overdrivr
Copy link
Collaborator

Nice, so I'm going to close this, thanks again for your contribution !

@Overdrivr Overdrivr closed this Apr 5, 2019
@keikoro keikoro added needs-more-info Waiting for submitter's reply, feedback, updates,... and removed needs more info labels Oct 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-more-info Waiting for submitter's reply, feedback, updates,...
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants