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

Ability to set BGR of VideoFileClip #1070

Closed
JunweiLiang opened this issue Feb 2, 2020 · 4 comments · Fixed by #1237
Closed

Ability to set BGR of VideoFileClip #1070

JunweiLiang opened this issue Feb 2, 2020 · 4 comments · Fixed by #1237
Labels
feature-request Request for a new feature or additional functionality. good-first-issue Especially suitable for people new to moviepy and/or programming; beginner-friendly.

Comments

@JunweiLiang
Copy link

Currently the pix_fmt for ffmpeg is set in VideoFileClip.py via

pix_fmt = "rgba" if has_mask else "rgb24"

It would be great if VideoFileClip could accept an optional "pix_fmt" arg so that "bgr24" could be used to be consistent with OpenCV.

@tburrows13 tburrows13 added feature-request Request for a new feature or additional functionality. good-first-issue Especially suitable for people new to moviepy and/or programming; beginner-friendly. labels Feb 8, 2020
@tburrows13 tburrows13 added this to the Release v2.0.0 milestone Feb 23, 2020
@Ricardo-HE
Copy link
Contributor

Hi! can I work on this issue?

@tburrows13
Copy link
Collaborator

Yes please! It looks like there's quite a few places where "pix_fmt" appears, and it would probably be better to make all of them user-settable:

  • VideoFileClip (and by extension, FFMPEG_VideoReader which already has it)
  • ffmpeg_read_image()
  • ffmpeg_write_image()
  • VideoClip.write_videofile() (and by extension, ffmpeg_write_video() and FFMPEG_VideoWriter)

And then .write_gif(), if you like.

Note ffmpeg -pix_fmts.

Let me know if you need a hand with anything!

@Ricardo-HE
Copy link
Contributor

The solution should accept any of the pixels format supported by ffmpeg? if that's the case, should I change the depth depending on the pixel format? Or did I misunderstand something?

@tburrows13
Copy link
Collaborator

tburrows13 commented Jun 16, 2020

Well you could in theory change the depth depending on the NB_COMPONENTS output from ffmpeg, but I think that that is probably overkill. I'd be happy to just assume that it is 3 unless it is rgba (or perhaps unless it ends in a to catch brga as well. This would require a comment next to that line expaining it or linking here). This is the only feature request that we've had related to pix_fmt, so I don't think that it is worth adding (and therefore maintaining) a lot of code to interface with ffmpeg to determine the depth when it isn't even required to fulfil this request. If people in the future require this feature, I wouldn't necessarily object to it, but it seems unnecessary at this point in time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Request for a new feature or additional functionality. good-first-issue Especially suitable for people new to moviepy and/or programming; beginner-friendly.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants