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 pixel format for video or image files #1237

Merged
merged 8 commits into from
Jun 18, 2020
Merged

Ability to set pixel format for video or image files #1237

merged 8 commits into from
Jun 18, 2020

Conversation

Ricardo-HE
Copy link
Contributor

@Ricardo-HE Ricardo-HE commented Jun 17, 2020

Closes #1070

Basically just add an pix_fmt for the some classes, functions and methods to allow different pixels formats instead of the defaults 'rgba' and 'rgb24', but these two are kept if pix_fmt is not given:

  • writer_videofile() from VideoClip class.
  • FFMPEG_VideoReader class.
  • ffmpeg_read_image function from ffmpeg_reader.py.
  • FFMPEG_VideoWriter class from ffmpeg_writer.py.
  • ffmpeg_writer_video function from ffmpeg_writer.py.
  • ffmpeg_writer_image function from ffmpeg_writer.py.
  • VideoFileClip class.

I haven't created the tests for these changes because I am a little lost on where to do it. I notice the class FFMPEG_VideoWriter doesn't have a test. VideoFileClip stores an instance for that class but in his unit test only checks one variable from it and because of that I don't know if I should create a test about my change there. A little orientation about this would be very helpful.

I also want to implement this for .write_gif(), and because of this and what I previously mentioned I would label this PR as a work in progress.

Add ability to set pixel format for the following functions/methods and classes:
 - ``write_videofile()`` from ``VideoClip`` class.
 - ``FFMPEG_VideoReader`` class.
 - ``ffmpeg_read_image`` function from 'ffmpeg_reader.py'
 - ``FFMPEG_VideoWriter`` class from 'ffmpeg_writer.py'
 - ``ffmpeg_write_video`` function from 'ffmpeg_writer.py'
 - ``ffmpeg_writer_image`` function from 'ffmpeg_writer.py'
 - ``VideoFileClip`` class.
@coveralls
Copy link

coveralls commented Jun 17, 2020

Coverage Status

Coverage increased (+0.04%) to 67.597% when pulling 510e5fa on RicardoHE97:BGR_option into 74f08ea on Zulko:master.

@tburrows13 tburrows13 added the feature New addition to the API i.e. a new class, method or parameter. label Jun 17, 2020
@tburrows13
Copy link
Collaborator

Thank you so much for this, it looks great! I've made a couple of tiny changes (I could have left the suggestions as a comment, but they were so trivial I just went ahead and did them myself in the web editor).

I wouldn't worry about tests for this PR. I don't really have a clue where to begin writing tests for FFMPEG_VideoReader beyond the basics like making sure that an output file exists and it has seemingly correct properties.

When you're happy with it, I'll merge it into master and it will be available in the next v2.0 dev release.

@tburrows13 tburrows13 changed the title feature: Ability to set pixel format for video or image files Ability to set pixel format for video or image files Jun 18, 2020
Add the functionality to change the pixel format when creating a gif.
But this new feature is only available when ffmpeg is used.

This new feature change ``write_gif`` from 'VideoClip.py'. As well
``write_gif_with_tempfiles`` and ``write_gif`` from 'gif_writers.py'
@Ricardo-HE
Copy link
Contributor Author

Ricardo-HE commented Jun 18, 2020

I added the feature to add a pixel format when creating a gif. But is limited to only do that when the program you use is 'ffmpeg'. I also added a test for it.

Feel free to make any suggestion or comments :)

Add the ability to set the pixel format when create a gif using
tempfiles.
@Ricardo-HE
Copy link
Contributor Author

Ricardo-HE commented Jun 18, 2020

I think I have nothing more to add to this PR. So I think we can close it if there is nothing more to contribute this time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New addition to the API i.e. a new class, method or parameter.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ability to set BGR of VideoFileClip
3 participants