ffmpeg: optimize ffmpeg_postprocess #116309

Aras Pranckevicius · 2023-12-18T19:38:50+01:00

2023-12-18 19:38:50 +01:00

After doing regular movie frame decoding, there's a "postprocess" step for each incoming frame, that does deinterlacing if needed, then YUV->RGB conversion, then vertical image flip and additional interlace filtering if needed. While this postprocess step is not the "heavy" part of movie playback, it still takes 2-3ms per each 1080p resolution input frame that is being played.

This PR does two things:

Similar to #116008, uses multi-threaded sws_scale to do YUV->RGB conversion.
Reintroduces "do vertical flip while converting to RGB", where possible. That was removed in 2ed73fc97e due to issues on arm64 platform, and theory that negative strides passed to sws_scale is not an officially supported usage.

My take on the last point: negative strides to sws_scale is a fine and supported usage, just ffmpeg had a bug specifically on arm64 where they were accidentally not respected. They fixed that for ffmpeg 6.0, and backported it to all versions back to 3.4.13 -- you would not backport something to 10 releases unless that was an actual bug fix!

I have tested the glitch_480p.mp4 that was originally attached to the bug report #94237 back then, and it works fine both on x64 (Windows) and arm64 (Mac).

Timings, ffmpeg_postprocess cost for a single 1920x1080 resolution movie strip inside VSE:

Windows/VS2022 Ryzen 5950X: 3.04ms -> 1.18ms
Mac/clang15 M1 Max: 1.10ms -> 0.71ms

After doing regular movie frame decoding, there's a "postprocess" step for each incoming frame, that does deinterlacing if needed, then YUV->RGB conversion, then vertical image flip and additional interlace filtering if needed. While this postprocess step is not the "heavy" part of movie playback, it still takes 2-3ms per each 1080p resolution input frame that is being played. This PR does two things: - Similar to #116008, uses multi-threaded `sws_scale` to do YUV->RGB conversion. - Reintroduces "do vertical flip while converting to RGB", where possible. That was removed in 2ed73fc97e due to issues on arm64 platform, and theory that negative strides passed to sws_scale is not an officially supported usage. My take on the last point: negative strides to sws_scale is a fine and supported usage, just ffmpeg had a bug specifically on arm64 where they were accidentally not respected. They fixed that for ffmpeg 6.0, and backported it to all versions back to 3.4.13 -- you would not backport something to 10 releases unless that was an actual bug fix! I have tested the `glitch_480p.mp4` that was originally attached to the bug report #94237 back then, and it works fine both on x64 (Windows) and arm64 (Mac). Timings, `ffmpeg_postprocess` cost for a single 1920x1080 resolution movie strip inside VSE: - Windows/VS2022 Ryzen 5950X: 3.04ms -> 1.18ms - Mac/clang15 M1 Max: 1.10ms -> 0.71ms

👍 1 ❤️ 8 🚀 1

Aras Pranckevicius added 2 commits 2023-12-18 19:39:01 +01:00

ffmpeg: multithreaded YUV->RGB conversion on movie file reading 4647c80f52

Playing back 1080p resolution video in VSE, the ffmpeg_postprocess
step (deinterlace, YUV->RGB, vertical flip) on Windows / Ryzen 5950X
goes from 3.05ms down to 2.41ms

ffmpeg: speed up video playback by reintroducing vertical flip in same step as scale where possible d0d94974dd

Blender used to do video frame YUV->RGB conversion and image vertical
flip in the same step, but that was removed in 2ed73fc97e in order
to fix problems with video widths that aren't multiple of SIMD width
(e.g. 32). However many common video resolutions (e.g. 1920x1080) are
compatible with that, so try to reintroduce that.

Playing back 1080p resolution video in VSE, the ffmpeg_postprocess
step (deinterlace, YUV->RGB, vertical flip) on Windows / Ryzen 5950X
goes from 2.41ms down to 1.18ms

Aras Pranckevicius added 1 commit 2023-12-18 20:16:56 +01:00

ffmpeg: scale and flip has been broken before ffmpeg 6 on arm64, account for that

buildbot/vexp-code-patch-darwin-x86_64 Build done.

Details

buildbot/vexp-code-patch-linux-x86_64 Build done.

Details

buildbot/vexp-code-patch-lint Build done.

Details

buildbot/vexp-code-patch-darwin-arm64 Build done.

Details

buildbot/vexp-code-patch-windows-amd64 Build done.

Details

buildbot/vexp-code-patch-coordinator Build done.

Details

833ebb9087

Aras Pranckevicius added 1 commit 2023-12-18 20:46:24 +01:00

ffmpeg: move threaded sws context helper functions to a better place

buildbot/vexp-code-patch-lint Build done.

Details

buildbot/vexp-code-patch-linux-x86_64 Build done.

Details

buildbot/vexp-code-patch-windows-amd64 Build done.

Details

buildbot/vexp-code-patch-darwin-x86_64 Build done.

Details

buildbot/vexp-code-patch-darwin-arm64 Build done.

Details

buildbot/vexp-code-patch-coordinator Build done.

Details

1a8154fb6d

Aras Pranckevicius commented

2023-12-18 20:46:55 +01:00

@blender-bot build

Aras Pranckevicius changed title from ~~WIP: ffmpeg: optimize ffmpeg_postprocess~~ to ffmpeg: optimize ffmpeg_postprocess

2023-12-19 05:49:47 +01:00

Aras Pranckevicius added this to the Video Sequencer project 2023-12-19 05:49:57 +01:00

Aras Pranckevicius requested review from Richard Antalik 2023-12-19 05:50:08 +01:00

Richard Antalik commented

2023-12-19 16:42:25 +01:00

Maybe not heaviest part, but 8K playback downscaled to 1080p is 25fps vs 11fps. Pretty good IMO. Or 9fps vs 6fps at original resolution.

Richard Antalik approved these changes 2023-12-19 17:11:40 +01:00

source/blender/blenkernel/BKE_writeffmpeg.hh

						
				@ -75,1 +77,4 @@

				SwsContext *BKE_ffmpeg_sws_get_context(

				    int width, int height, int av_src_format, int av_dst_format, int sws_flags);

				void BKE_ffmpeg_sws_scale_frame(SwsContext *ctx, AVFrame *dst, const AVFrame *src);

Richard Antalik commented

2023-12-19 16:45:23 +01:00

I am bit sad, that you can't use AVPixelFormat type. You should be able to add include file with the type to BKE_writeffmpeg.hh but it says, that it can't find the file, which is weird to me.

I am bit sad, that you can't use `AVPixelFormat` type. You should be able to add include file with the type to `BKE_writeffmpeg.hh` but it says, that it can't find the file, which is weird to me.

Aras Pranckevicius commented

2023-12-19 18:27:04 +01:00

That's because at least blenloader library that includes BKE_writeffmpeg.hh does not setup FFMPEG include directories for itself. I'd rather keep it that way, i.e. sure it would be nicer to use AVPixelFormat here, but the downside is that any users of this header file would now need to take care of setting up FFMPEG directories. Sounds like a larger hassle.

That's because at least `blenloader` library that includes `BKE_writeffmpeg.hh` does not setup FFMPEG include directories for itself. I'd rather keep it that way, i.e. sure it would be nicer to use `AVPixelFormat` here, but the downside is that any users of this header file would now need to take care of setting up FFMPEG directories. Sounds like a larger hassle.

Richard Antalik commented

2023-12-19 19:31:36 +01:00

Thanks for checking this. IMO FFMpegCodecData::type enum belongs to DNA. I would say, that BLO code should have no business caring about libs we build with, unless it uses them to process some data.

Thanks for checking this. IMO `FFMpegCodecData::type` enum belongs to DNA. I would say, that BLO code should have no business caring about libs we build with, unless it uses them to process some data.