Closed stratty7 closed 3 months ago
@stratty7 Looks like there are problems with formatting which would need fixing.
Also, looking at changes that came before this addition: shouldn't this be propagated to elsewhere as well? E.g. when pixel_format
was introduced, it got added in more than one place.
Hmm, looking at the above examples as well as other files, I guess I'm confused about two things:
has_mask
and with_mask
?with_mask
?When I mentioned pixel_format
above, which is a line above your change, I was specifically talking about that it got added to VideoClip.py
and VideoFileClip.py
simultaneously (as well as other places) in #1237 (where it was still called pix_fmt
). Your change is only for the former. Looking at the latter, I can see that has_mask
originates there. Do they set the same thing?
Hmm, looking at the above examples as well as other files, I guess I'm confused about two things:
- What's the difference between
has_mask
andwith_mask
?- What's the difference between your addition and this instance of
with_mask
?When I mentioned
pixel_format
above, which is a line above your change, I was specifically talking about that it got added toVideoClip.py
andVideoFileClip.py
simultaneously (as well as other places) in #1237 (where it was still calledpix_fmt
). Your change is only for the former. Looking at the latter, I can see thathas_mask
originates there. Do they set the same thing?
@keikoro I understand your points and see what you mean, I think.
With mask is not used in the ffmpeg writer at all, and using the pixel_format
can achieve the same result without the need of adding any more code. I have changed the code to reflect that and removed the has_mask arg from ffmpeg_writer.
transparentclip = VideoFileClip("1.mov", has_mask=True)
transparentclip.write_videofile("test.mov", codec="png", fps=30, pixel_format="rgba")
I think using pixel_format="rgba" addresses my needs.
However, with_mask
is currently not passed to ffmpeg_write_video
anywhere, I think this could probably be cleaned up. I will close this PR and if I get time clean that up and open a new one. Thanks for the reviews.
To clarify my last comment: I only used pixel_format
as an example for a change that was done in both files, VideoClip.py and VideoFileClip.py, so thought that maybe your suggested change should also happen in both files. Which then led me to the with_mask
vs. has_mask
thing.
I didn't look at it in more detail, but am I understanding correctly that this discrepancy was actually introduced earlier, and pixel_format
was potentially, partially (?) included to address the issue of one argument not getting passed down the chain? Or were you more saying that you just had the realisation pixel_format
could be used to replace the other argument, even though it wasn't so far?
Or were you more saying that you just had the realisation pixel_format could be used to replace the other argument, even though it wasn't so far?
Yeah this.
I think pixel_format
could be used in the place of with_mask
at the moment. Only if with_mask
is implemented properly into write_videofile
and ffmpeg_write_video
(which it currently isn't, but would be nice to have) then I understand having both args, but currently having both seems somewhat pointless. I think implementing with_mask
properly and fully (instead of my bandaid fix in the original code of this pr) into write_videofile
and ffmpeg_write_video
is the true fix here, but @convert_masks_to_RGB
may be a large obstacle to overcome..
That is why I have closed this for now and will work on it when i find time.
This will allow for clips that are loaded with an alpha channel (a .mov file for example) to be writen back out as a .mov file with an alpha layer (transparent background). The code already largely exists for this, the write_videofile function simply needs to be allowed to pass the with_mask arg to the ffmpeg_write_video, which then handles the rest.
For example:
Longer example:
Even longer