-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
fix issue #401 #403
fix issue #401 #403
Conversation
This does the same as https://github.com/Zulko/moviepy/pull/295/files ? |
Gloin1313.. I didn't see pull 295 before I created my version. Sorry. I like my solution better, but I'll let the maintainers to choose what version they like best. :) |
I'm not experienced so I can't comment which I like better. I'm not even sure the other one works on Python 2, but obviously it works fine with Python 3. |
Looking at this more closely, there is a difference.. self.proc.stdin.write(frames_array.tostring()) |
I tried my version on python 2 and python 3 and it seems to work. At least
for the couple of test cases I used.
…On Fri, Feb 10, 2017 at 9:06 AM, Gloin1313 ***@***.***> wrote:
I'm not experienced so I can't comment which I like better. I'm not even
sure the other one works on Python 2, but obviously it works fine with
Python 3.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#403 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ABGu8pJu2qp5NWIr8z4bAqYxdQxypRF1ks5rbHzkgaJpZM4L9Zsz>
.
|
The tobytes()/tostring() difference causes an imageio crash from somewhere else when I run moviepy until I fix it in the code. However, this difference makes no difference either way. |
that is funny.. I don't get the crash..
I'm running Ubuntu 16.10
My version of imageio is 2.1.2 (for python 3 and for python 2).
What are your versions?
…On Fri, Feb 10, 2017 at 9:12 AM, Gloin1313 ***@***.***> wrote:
The tobytes()/tostring() difference causes an imageio crash from somewhere
else when I run moviepy until I fix it in the code. However, this
difference makes no difference either way.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#403 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ABGu8kxvGqIRd8hifpTMAjTfII-8mu_rks5rbH51gaJpZM4L9Zsz>
.
|
merge latest zulko/moviepy to earney/moviepy
@Earney Thanks for the contribution! I prefer your approach to this. I believe this is the appropriate way to go and it resolves #401. We'll want to resolve #335 before pushing a release. |
Thanks. I will look into the fixing the other issue you mentioned
On Feb 14, 2017 5:53 PM, "Mark Beacom" <notifications@github.com> wrote:
*@mbeacom* approved this pull request.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#403 (review)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ABGu8reY8bJLxBoU6Q4Q37HTyg8d4cBQks5rcj5cgaJpZM4L9Zsz>
.
|
It should be extremely easy to add this to ffmpeg_audiowriter as well |
this fixes a python 3 error dealing with strings and bytes. See issue #401 for more details of this error.