-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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 pixel_format in FFMPEG_VideoWriter #2359
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -226,6 +226,7 @@ def write_videofile( | |
ffmpeg_params=None, | ||
logger="bar", | ||
pixel_format=None, | ||
print_cmd=False, | ||
): | ||
"""Write the clip to a videofile. | ||
|
||
|
@@ -404,6 +405,7 @@ def write_videofile( | |
ffmpeg_params=ffmpeg_params, | ||
logger=logger, | ||
pixel_format=pixel_format, | ||
print_cmd=print_cmd, | ||
) | ||
|
||
if remove_temp and make_audio: | ||
|
@@ -1867,7 +1869,8 @@ def __find_text_size( | |
To summarize, the real height of the text is: | ||
``initial padding + (lines - 1) * height + end padding`` | ||
or: | ||
``(ascent + stroke_width) + (lines - 1) * height + (descent + stroke_width)`` | ||
``(ascent + stroke_width) + (lines - 1) * height | ||
+ (descent + stroke_width)`` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Split the comment here to satisfy flake8 pre-commit hook. |
||
or: | ||
``real_font_size + (stroke_width * 2) + (lines - 1) * height`` | ||
""" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -92,6 +92,7 @@ def __init__( | |
threads=None, | ||
ffmpeg_params=None, | ||
pixel_format=None, | ||
print_cmd=False, | ||
): | ||
if logfile is None: | ||
logfile = sp.PIPE | ||
|
@@ -101,8 +102,6 @@ def __init__( | |
self.audio_codec = audio_codec | ||
self.ext = self.filename.split(".")[-1] | ||
|
||
pixel_format = "rgba" if with_mask else "rgb24" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should keep this logic, but change the variable name to avoid overriding the param |
||
|
||
# order is important | ||
cmd = [ | ||
FFMPEG_BINARY, | ||
|
@@ -111,12 +110,8 @@ def __init__( | |
"error" if logfile == sp.PIPE else "info", | ||
"-f", | ||
"rawvideo", | ||
"-vcodec", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We actually need thoses |
||
"rawvideo", | ||
"-s", | ||
"%dx%d" % (size[0], size[1]), | ||
"-pix_fmt", | ||
pixel_format, | ||
"-r", | ||
"%.02f" % fps, | ||
"-an", | ||
|
@@ -133,29 +128,40 @@ def __init__( | |
else: | ||
cmd.extend(["-vcodec", codec]) | ||
|
||
cmd.extend(["-preset", preset]) | ||
|
||
if ffmpeg_params is not None: | ||
cmd.extend(ffmpeg_params) | ||
# Disable auto alt ref for transparent webm | ||
if codec == "libvpx" and with_mask: | ||
cmd.extend(["-auto-alt-ref", "0"]) | ||
|
||
if bitrate is not None: | ||
cmd.extend(["-b", bitrate]) | ||
|
||
if threads is not None: | ||
cmd.extend(["-threads", str(threads)]) | ||
|
||
# Disable auto alt ref for transparent webm and set pix format yo yuva420p | ||
if codec == "libvpx" and with_mask: | ||
cmd.extend(["-pix_fmt", "yuva420p"]) | ||
cmd.extend(["-auto-alt-ref", "0"]) | ||
elif ( | ||
(codec == "libx264" or codec == "h264_nvenc") | ||
and (size[0] % 2 == 0) | ||
and (size[1] % 2 == 0) | ||
): | ||
cmd.extend(["-pix_fmt", "yuva420p"]) | ||
# Honor choice of pixel_format if specified manually | ||
OsaAjani marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if pixel_format is None: | ||
if ( # use yuva420p for transparent webm | ||
codec == "libvpx" and with_mask | ||
) or ( | ||
(codec == "libx264" or codec == "h264_nvenc") | ||
and (size[0] % 2 == 0) | ||
and (size[1] % 2 == 0) | ||
): | ||
pixel_format = "yuva420p" | ||
elif with_mask: | ||
pixel_format = "rgba" | ||
else: | ||
pixel_format = "rgb24" | ||
|
||
cmd.extend(["-pix_fmt", pixel_format]) | ||
cmd.extend(["-preset", preset]) | ||
|
||
if ffmpeg_params is not None: | ||
cmd.extend(ffmpeg_params) | ||
|
||
cmd.extend([ffmpeg_escape_filename(filename)]) | ||
if print_cmd: | ||
print(" ".join(cmd)) | ||
|
||
popen_params = cross_platform_popen_params( | ||
{"stdout": sp.DEVNULL, "stderr": logfile, "stdin": sp.PIPE} | ||
|
@@ -253,6 +259,7 @@ def ffmpeg_write_video( | |
ffmpeg_params=None, | ||
logger="bar", | ||
pixel_format=None, | ||
print_cmd=False, | ||
): | ||
"""Write the clip to a videofile. See VideoClip.write_videofile for details | ||
on the parameters. | ||
|
@@ -282,6 +289,7 @@ def ffmpeg_write_video( | |
threads=threads, | ||
ffmpeg_params=ffmpeg_params, | ||
pixel_format=pixel_format, | ||
print_cmd=print_cmd, | ||
) as writer: | ||
for t, frame in clip.iter_frames( | ||
logger=logger, with_times=True, fps=fps, dtype="uint8" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be documented