Fix pixel_format in FFMPEG_VideoWriter #2359
Conversation
jhadida
changed the title
| @@ -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.
Split the comment here to satisfy flake8 pre-commit hook.
|
Hi, you are right in that pixel format is indeed not respected in the current implementation, many thanks for pointing this out. This being said, I think you made a few errors. FFMPEG is a pretty uncommon cli tools, and some arguments can be duplicated to apply them either to input (before As for the Would you be willing to implement those changes ? |
| @@ -226,6 +226,7 @@ def write_videofile( | |||
| ffmpeg_params=None, | |||
| logger="bar", | |||
| pixel_format=None, | |||
| print_cmd=False, | |||
There was a problem hiding this comment.
This should be documented
| @@ -111,12 +110,8 @@ def __init__( | |||
| "error" if logfile == sp.PIPE else "info", | |||
| "-f", | |||
| "rawvideo", | |||
| "-vcodec", | |||
| @@ -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.
We should keep this logic, but change the variable name to avoid overriding the param
tests/I will be going through the checkpoints above later on.
The main issues that this PR fixes are with the implementation of
FFMPEG_VideoWriter:pixel_formatargument, and overrides it with eitherrgb24orrgba.-pix_fmtis duplicated in some cases.-vcodec(or-c:v) is systematically duplicated.This PR addresses these issues by honoring the
pixel_formatargument when it is notNoneregardless of other arguments. It also introduces an argumentprint_cmdto print the FFMPEG command to the terminal for debugging purposes.