Skip to content

Reworked default recorder docker FFmpeg profiles

The current default recorder profiles for ffmpeg ran into several issues, primarily involving unwanted PTS reset when the source input stream caused filtergraph reconfiguration. I've rewritten the default recorder presets in a way which fixes most bugs related to the FFmpeg code directly, such as #485514, #485515, #455006, #429326 and #450790

The changes

These are the new sections of the ffmpeg code, which I'll explain each change and why I feel they are needed to fix the issues. I've never submitted a MR before or contributed to a project code-wise, so I was unsure if these sorts of comments were wanted line-by-line in the code itself for future reference. I can move them into comment snippets if necessary.

Overall, I tried to aim to make future fixes easier and the command steps simpler to understand at first glance for someone looking to expand or fix it. Unfortunately this propagated outward and eventually led to almost the entire command being rewritten.

I tried to test for performance, but ultimately didn't notice too much of a difference that would cause issue, especially since the previous code essentially ignored the intro and outro.

-reinit_filter 0 
-framerate $IN_FPS 
-i "$INPUT_DIR%07d.$EXT" 

At the heart of most of the previous issues was the fact that the input file's PTS timings would reset each time the input stream hit a new 'type' of frame. For instance, if the frame changed size. While this does not affect simple ffmpeg commands, more complex ones may contain filters (such as loop or concat) which cannot handle this correctly. To prevent this and allow more complex filters in the chain, the reinit_filter flag is used. This prevents the frame skipping and unwanted section overlap that was previously erasing entire sections. This should fix #485514, #485515, and #450790.

To simplify, the multiple inputs were replaced with a single input.

[0]scale='min($WIDTH, iw*($HEIGHT/ih))':'min($HEIGHT, ih*($WIDTH/iw))':eval=frame[in1];
[in1]pad=$WIDTH:$HEIGHT:(ow-iw)/2:(oh-ih)/2:eval=frame[in2];
[in2]format=yuv420p[in3];
[in3]split=4[main1][preview1][transition1][end1];

Because we've turned off filter reinit, I had figured it's best to keep all input frames the same in size and format.

The first line handles resizing, it resizes all frames to fit within the defined parameters in the docker. It also maintains aspect ratio, fixing bug #429326, and possibly #455006?

eval=frame is necessary on the scale and pad as our input stream can change size at any moment and is not known ahead of time.

I'm not entirely sure if format=yuv420p is necessary, as I haven't run into a situation yet where it has proven useful. I left it in in case such a situation cropped up eventually, or I had missed one.

Finally it splits the main file into all sections to be used as needed.

[transition1]trim=start_frame=$FRAMES-1,tpad=stop_mode=clone:stop_duration=$TRANSITION_LENGTH[transition2];
[transition2]setpts=PTS-STARTPTS[transition3];
[transition3][main1]xfade=transition=smoothright:duration=$TRANSITION_LENGTH:offset=0[main2];

The transition section, with a new variable $TRANSITION_LENGTH which may be useful as the recorder gets more features, but is currently only 1 or 0 depending on if there is an intro preview or not. Was necessary as the trim filter does not accept math or evaluations like loop did. This section creates a 1 second fade between the final picture and the main timelapse to be used in the middle section.

[end1]trim=start_frame=$FRAMES-1,tpad=stop_mode=clone:stop_duration=$LAST_FRAME_SEC[end2];
[end2]setpts=PTS-STARTPTS[end3];

The ending hold frame. Will extend the final frame for the given seconds.

[preview1]trim=start_frame=$FRAMES-1,tpad=stop_mode=clone:stop_duration=$FIRST_FRAME_SEC[preview2];
[preview2]setpts=PTS-STARTPTS[preview3];

The pre-timelapse preview of the final product. This created the unintended side-effect of always having the final product be the first frame. If $FIRST_FRAME_SEC is 0, the final image will show for 1 frame. At first I figured I would remove this with a simple trim after the concat, but as I thought more about it- most art hosting websites that accept gif or video will use the first frame as the thumbnail. This could draw more attention to artist's works on sites that use this preview method. Thought?

[preview3][main2][end3]concat=n=3:v=1:a=0[final1];
[final1]setpts=PTS-STARTPTS;

Final concat, then PTS reset.

Known issues

  • Because of -reinit_filter, each frame outputs a warning if it is not the same size as the first frame of the sequence. As far as I could tell, there is no way to prevent this on a timelapse with different-sized images. It can safely be ignored, however.
  • The encoding progress bar is now inaccurate, it will stay at 0% for about a third of the total time, creep up to ~98% for another third, then finish after another third. I feel that fixing that may be outside of my abilities but I can take a shot at it if needed.

Test Plan

If you don't have a test environment KritaRecorder folder, you can use this ffmpeg command inside a new folder to generate test images quickly:

ffmpeg -f lavfi -i "color=c=red:r=25:size=1280x720:d=4" -frames:v 100 %07d.png \
&& ffmpeg -f lavfi -i "color=c=green:r=25:size=640x720:d=4" -start_number 101 -frames:v 100 %07d.png \
&& ffmpeg -f lavfi -i "color=c=blue:r=25:size=1280x720:d=4" -start_number 201 %07d.png

After that, just navigate to that folder in the recorder docker and export the timelapse with the settings you want

Formalities Checklist

  • I confirmed this builds.
  • I confirmed Krita ran and the relevant functions work.
  • I tested the relevant unit tests and can confirm they are not broken. (If not possible, don't hesitate to ask for help!)
  • I made sure my commits build individually and have good descriptions as per KDE guidelines.
  • I made sure my code conforms to the standards set in the HACKING file.
  • I can confirm the code is licensed and attributed appropriately, and that unattributed code is mine, as per KDE Licensing Policy.

Reminder: the reviewer is responsible for merging the patch, this is to ensure at the least two people can build the patch. In case a patch breaks the build, both the author and the reviewer should be contacted to fix the build. If this is not possible, the commits shall be reverted, and a notification with the reasoning and any relevant logs shall be sent to the mailing list, kimageshop@kde.org.

Edited by Ralek Kolemios

Merge request reports