Optimizing pl_mpeg

[bitbank2]

Hi Dominic,
I recently got approved for a grant from NLnet and one of the projects I proposed was to optimize the pl_mpeg player to make it suitable for use on constrained devices (e.g. Microcontrollers). I've forked it and will be sharing my optimizations as I make them. Besides speed optimizations, I will make some #ifdef changes for the different memory challenges on MCUs (e.g. PSRAM).

[phoboslab]

Very cool! I can't promise to merge any of your changes (if that's even desired), but I'll certainly have a look.

For performance optimizations, have a look at the n64 port of pl_mpeg in libdragon: https://github.com/DragonMinded/libdragon/tree/preview/src/video – from what I understand they added a bunch of general optimizations for bit reading and VLC lookups (and of course some very n64 specific YUV conversion functions etc).

[bitbank2]

I've actually gone beyond most of the optimizations in that repo. I still have a few more to go...

[bitbank2]

One of the strategies I need to implement for MCUs is knowing which MBs have changed each frame and only drawing those. A significant amount of time on MCUs is spent converting pixel colorspace and pushing those pixels to the display. Any MBs that can be avoided per frame helps a lot.

[siteswapv4]

@bitbank2 There's no issues page on your fork page so I'm messaging you here. I tried your optimized version and it's really impressive ! But currently it's unusable for me because it randomly hangs and takes 100% of the CPU, stopping decoding entirely. It happens with every mpg file I tested when seeking through it, sometimes for particular files I don't even have to seek through.
I hope you can replicate this behavior

gcc pl_mpeg_player_sdl.c -o player -O3 -lSDL2
./player bjork-all-is-full-of-love.mpg

It happens with the example video in the readme too.
When I click on the screen to seek through the video, it stops completely and hangs. This behavior also happens on PSVita, which is the platform I target with my app.

[bitbank2]

Thanks for letting me know - I'll figure it out.

[bitbank2]

@siteswapv4 I'm not able to reproduce your hang on the mac. I'm playing the bjork example video and seeking at the beginning to get the duration. I'm not familiar with other player programs. Can you give me the specific function to call with the specific parameter that makes it hang on the bjork video? I didn't add a seek function yet to my test player.

[siteswapv4]

#define PL_MPEG_IMPLEMENTATION
#include "pl_mpeg.h"

#include <stdio.h>

int main(int argc, char* argv[])
{
	plm_t* plm;
	plm_frame_t* frame;

	plm = plm_create_with_filename(argv[1]);
	
	for (int i = 0; i < 100; i++)
	{
		printf("Loop : %d\n", i);
		frame = plm_decode_video(plm);
		
		plm_seek(plm, i, TRUE);
	}
	
	return 0;
}

This hangs forever at loop 4 for me playing bjork example
I compile and run with :

gcc test.c -o test
./test bjork-all-is-full-of-love.mpg 

[siteswapv4]

Maybe you could open your github repo for issues also so people can report bugs better later
@bitbank2

Hope you can reproduce it this time

[siteswapv4]

Also I'm testing this on linux but same behavior on psvita so I don't think it's relevant

[bitbank2]

I was able to reproduce the problem on my MacOS project. It has to do with the changes I made to the buffer/file access. It gets messed up when seeking. I think I can resolve it. How much faster does it run in your tests? On the Mac it's about 230% faster than the original (so far).

[siteswapv4]

I haven't been able to benchmark it yet because of the crash issue but on desktop my whole game went from 25% to 20% single core usage just with that. I'd have to run tests on the Vita where it's most relevant.

[siteswapv4]

@bitbank2 Just tested your version on psvita (without seeking) and I can play videos with a bitrate 2 to 3 times higher than before (I disable audio and only play video), so yup the improvement is crazy lol

[bitbank2]

I fixed the problem, but discovered something in the design that I really don't like. Because of the way it seems to seek audio and video separately, it gums up the buffer such that it reallocates the file buffer to 2x size mid-decode. I will push my fix for you to try, but I would like to redesign the logic to not ever use memrealloc().

[siteswapv4]

@bitbank2 Completely fixed, both the seek issue and the random crashes I was getting, tested with the same files that were failing before. Thanks a lot !

[siteswapv4]

Do you think you can push the optimizations further or are you about done ?

[bitbank2]

I'm at the point where the next set of optimizations would require breaking the code and then fixing the breakage. Most of the hot spots are in already in good shape. One option I've tested is to have a macroblock callback function instead of frame. This works on some videos, but not all because the switch between showing a B frame or an F frame mess things up. It's a dramatic speedup because redrawing the entire frame every time is wasteful because most pixels don't change. The problem is also that dramatic movement will cause a worst-case scenario, so optimizing for slim frame changes can lead to uneven playback load. Here's the main profile so far. The code taking the most time is doing the necessary macroblock decoding/copying/moving.

[siteswapv4]

I see ! Well the improvement is already incredible right now. I'll take a peek at your fork from time to time but right now there's nothing more I can ask for the PSVita, it decodes up to native resolution (960x540) 3000K video bitrate now, thanks

[bitbank2]

There are plenty of places in the current code where some 128-bit SIMD would help significantly, but that's a little beyond the scope of what I'm doing. I wanted to maintain a pure C project that can compile on any target CPU (32-bits or better).

[DeeJayLSP]

If SIMD is to be considered, the suggestions would be SSE2 for X86(_64) and NEON for ARM. Virtually every CPU launched in the past 20 years support them. And you can always hide them behind preprocessor directives.

(I don't know much about implementing SIMD, just wanted to point that out)

[siteswapv4]

Yup same, if SIMD make the thing faster I'm all in, but not at the cost of compatibility with most CPUs
I know multiple libs (like cglm for matrices) that use SIMD and can compile fine with header only on all architectures I target so I'm sure it's possible though

[siteswapv4]

There's also "SIMD everywhere" that exists, though I've never actually used SIMD myself, just checked the github page it seems promising ?
https://github.com/simd-everywhere/simde

[bitbank2]

I can add SIMD for x86 and Arm NEON easily and in a way that doesn't break it for generic C targets. I had assumed that pl_mpeg was only used on "strange/odd" targets because x86/Arm would have optimized playback in some other form already. If you're saying that your use of pl_mpeg is on an x86 or Arm CPU with SIMD capability, then sure, I'm in!

[siteswapv4]

PSVita has an ARM Cortex 9A which supports NEON SIMD
I'm all in !
pl_mpeg is used on every platform possible from what I could see, from N64 to Switch, on desktop etc... It's just so easy to put everywhere there are no deps to manage

[bitbank2]

I've already begun and the NEON code is "erasing" the time spent in the macroblock processing. >100ms turns into 8ms with SIMD:
This could lead to another 20+% overall speedup. What code do you use for converting the YCbCr->RGB? I can add SIMD for that too.

[siteswapv4]

I'm not converting anything I use SDL2 (now 3 but same behavior) and I directly update a YUV texture. The SDL2 only example in the pl_mpeg github is from me if you wanna take a look.

[siteswapv4]

wow great !

[DeeJayLSP]

I did a test by compiling pl_mpeg_extract_frames before and after the optimizations (as of 2bb3b474b82f5ebc76ee5eb4717e16c422b7c887), using it on a 4k 60fps video, then re-encoding it again.

Before: https://0x0.st/s/SyCGgHGrTtYPHlMHMVSlFg/88wG.mp4

After: https://0x0.st/s/uJT7rguTFuNecxx2FDyvmA/88wD.mp4

Outside pl_mpeg_extract_frames or on less demanding videos (1080p or less, 24-30fps) the decoded result is a bit less broken. This can be observed since the first commit that implemented the fast VLC.

I get that the optimization process isn't finished yet, but I thought pointing this out could help in some way.

Despite this, the performance gains are quite impressive. Even Theora with its SSE2 can't stand against.

[bitbank2]

I did a test by compiling pl_mpeg_extract_frames before and after the optimizations (as of 2bb3b474b82f5ebc76ee5eb4717e16c422b7c887), using it on a 4k 60fps video, then re-encoding it again.

Before: https://0x0.st/s/SyCGgHGrTtYPHlMHMVSlFg/88wG.mp4

After: https://0x0.st/s/uJT7rguTFuNecxx2FDyvmA/88wD.mp4

Outside pl_mpeg_extract_frames or on less demanding videos (1080p or less, 24-30fps) the decoded result is a bit less broken. This can be observed since the first commit that implemented the fast VLC.

I get that the optimization process isn't finished yet, but I thought pointing this out could help in some way.

Despite this, the performance gains are quite impressive. Even Theora with its SSE2 can't stand against.

Thanks for the info. My initial changes reduced the frequency of checking the buffer state. The original code was checking for data available after each VLC decode. I need to implement a highwater mark check at the start of each macroblock decode to fix this.

[bitbank2]

@siteswapv4 I just pushed a change which adds some NEON SIMD and the overall video decode speedup is about 21% compared to the previous build. I will now work on fixing the stream issues since removing some of the "has enough" checks.

[siteswapv4]

It doesn't compile on PSVita right now because

expected 'uint16x8_t' but argument is of type 'uint8x16_t'

[siteswapv4]

/home/siteswap/pld/include/pl_mpeg.h:3617:17: note: use '-flax-vector-conversions' to permit conversions between vectors with differing element types or numbers of subparts
 3617 |                 u168_ab0L = vaddl_u8(vget_low_u8(u816_a1), vget_low_u16(u816_b1)); // horizontal add of current line
      |                 ^~~~~~~~~
/home/siteswap/pld/include/pl_mpeg.h:3617:73: error: incompatible type for argument 1 of 'vget_low_u16'
 3617 | u168_ab0L = vaddl_u8(vget_low_u8(u816_a1), vget_low_u16(u816_b1)); // horizontal add of current line
      |                                                         ^~~~~~~
      |                                                         |
      |                                                         uint8x16_t

In file included from /usr/local/vitasdk/arm-vita-eabi/include/SDL2/SDL_cpuinfo.h:83,
                 from /usr/local/vitasdk/arm-vita-eabi/include/SDL2/SDL.h:38,
                 from /home/siteswap/pld/include/PLD.h:4,
                 from /home/siteswap/pld/include/PLDVideo.h:2,
                 from /home/siteswap/pld/src/PLDVideo.c:1:
/usr/local/vitasdk/lib/gcc/arm-vita-eabi/10.3.0/include/arm_neon.h:7364:26: note: expected 'uint16x8_t' but argument is of type 'uint8x16_t'
 7364 | vget_low_u16 (uint16x8_t __a)
      |               ~~~~~~~~~~~^~~
In file included from /home/siteswap/pld/src/PLDVideo.c:4:
/home/siteswap/pld/include/pl_mpeg.h:3618:75: error: incompatible type for argument 1 of 'vget_high_u16'
 3618 | 168_ab0H = vaddl_u8(vget_high_u8(u816_a1), vget_high_u16(u816_b1));
      |                                                          ^~~~~~~
      |                                                          |
      |                                                          uint8x16_t

In file included from /usr/local/vitasdk/arm-vita-eabi/include/SDL2/SDL_cpuinfo.h:83,
                 from /usr/local/vitasdk/arm-vita-eabi/include/SDL2/SDL.h:38,
                 from /home/siteswap/pld/include/PLD.h:4,
                 from /home/siteswap/pld/include/PLDVideo.h:2,
                 from /home/siteswap/pld/src/PLDVideo.c:1:
/usr/local/vitasdk/lib/gcc/arm-vita-eabi/10.3.0/include/arm_neon.h:7285:27: note: expected 'uint16x8_t' but argument is of type 'uint8x16_t'
 7285 | vget_high_u16 (uint16x8_t __a)
      |                ~~~~~~~~~~~^~~
In file included from /home/siteswap/pld/src/PLDVideo.c:4:
/home/siteswap/pld/include/pl_mpeg.h: In function 'plm_video_decode_block':
/home/siteswap/pld/include/pl_mpeg.h:3884:43: error: incompatible type for argument 1 of 'vcombine_u16'
 3884 |                     in0_16 = vcombine_u16(vqmovn_s32(in0_32), vqmovn_s32(in1_32));
      |                                           ^~~~~~~~~~~~~~~~~~
      |                                           |
      |                                           int16x4_t
In file included from /usr/local/vitasdk/arm-vita-eabi/include/SDL2/SDL_cpuinfo.h:83,
                 from /usr/local/vitasdk/arm-vita-eabi/include/SDL2/SDL.h:38,
                 from /home/siteswap/pld/include/PLD.h:4,
                 from /home/siteswap/pld/include/PLDVideo.h:2,
                 from /home/siteswap/pld/src/PLDVideo.c:1:
/usr/local/vitasdk/lib/gcc/arm-vita-eabi/10.3.0/include/arm_neon.h:7189:26: note: expected 'uint16x4_t' but argument is of type 'int16x4_t'
 7189 | vcombine_u16 (uint16x4_t __a, uint16x4_t __b)
      |               ~~~~~~~~~~~^~~
In file included from /home/siteswap/pld/src/PLDVideo.c:4:
/home/siteswap/pld/include/pl_mpeg.h:3884:63: error: incompatible type for argument 2 of 'vcombine_u16'
 3884 |               in0_16 = vcombine_u16(vqmovn_s32(in0_32), vqmovn_s32(in1_32));
      |                                                         ^~~~~~~~~~~~~~~~~~
      |                                                         |
      |                                                         int16x4_t

In file included from /usr/local/vitasdk/arm-vita-eabi/include/SDL2/SDL_cpuinfo.h:83,
                 from /usr/local/vitasdk/arm-vita-eabi/include/SDL2/SDL.h:38,
                 from /home/siteswap/pld/include/PLD.h:4,
                 from /home/siteswap/pld/include/PLDVideo.h:2,
                 from /home/siteswap/pld/src/PLDVideo.c:1:
/usr/local/vitasdk/lib/gcc/arm-vita-eabi/10.3.0/include/arm_neon.h:7189:42: note: expected 'uint16x4_t' but argument is of type 'int16x4_t'
 7189 | vcombine_u16 (uint16x4_t __a, uint16x4_t __b)

Here's the log

[siteswapv4]

enabling the hinted flag fixed it
-flax-vector-conversions
idk if I'm losing performance with that or not

[siteswapv4]

Can confirm there's a noticeable improvement on PSVita ! Very nice !

[bitbank2]

@siteswapv4 some compilers are picky about casting SIMD registers; LLVM isn't. I can fix that with a NEON macro. Glad you're able to see the improvement. Now to try rewriting it in ESP32-S3 SIMD. Did you say that someone needed a SSE2 version?

[siteswapv4]

Well I'm not super familiar with SIMD but if I understand correctly and it means it'll be faster on x86/64 CPUs it's a full win

[bitbank2]

Well I'm not super familiar with SIMD but if I understand correctly and it means it'll be faster on x86/64 CPUs it's a full win

Each CPU has it's own variation of these instructions. Most allow manipulating 128-bit values as if they're 4 x 32-bits or 8 x 16-bits or 16 x 8-bits. I can support the unique instructions needed to speed up the code on x86 targets, but it would be helpful to have at least 1 person specifically needing that code. I can always do it later, but the ESP32-S3 is more important to me at the moment.

[siteswapv4]

I won't have immediate use of this myself. Though I'm guessing most people that use pl_mpeg will use it on recent desktops/laptops, as I said before pl_mpeg really is about its easiness to include in all possible projects.
That said I don't think anyone has an urgent need to have x86 SIMD in like right now, so you should focus on what matters to you. The current level of optimization is already enough.

[DeeJayLSP]

I've been using pl_mpeg in my godot-mpg module to add support for MPEG-1 videos in Godot. It was meant to replace Theora, a video codec that was supported by Godot since before it was open-sourced and to this day is full of bugs (which might probably get solved soon).

pl_mpeg itself turned out to be slower than Theora, which has SSE2. But after just the VLC optimizations it was able to surpass it to the point even unoptimized editor builds are able to play without stuttering.

I pointed to SSE2 and NEON because not only those are the only SIMD accepted there, but virtually every amd64 CPU supports SSE2 and every armv8 supports NEON.

(also shouldn't it use #ifdef __ARM_NEON instead of detecting the arch? This is the way NEON macros are defined in most source codes and whether it should be used or not is decided by the buildsystem)

[bitbank2]

#ifdef __ARM_NEON

Good points. I fixed the macro name (not pushed yet). After I get ESP32-S3 SIMD working, I'll return to do SSE2. The functions are all really simple, someone else can also do a PR with the SSE2 code.

[crudelios]

I completely missed the boat on this one.

As someone who really wants to have videos working at good speeds on psvita, I really must have a good look at this one.

Also, not sure if it's even useful at this point, but I had posted a PR that yielded a small (7%) optimization - check #33. SIMD and video decoding is way beyond my knowledge, so that was the best I could do.

[bitbank2]

I completely missed the boat on this one.

As someone who really wants to have videos working at good speeds on psvita, I really must have a good look at this one.

Also, not sure if it's even useful at this point, but I had posted a PR that yielded a small (7%) optimization - check #33. SIMD and video decoding is way beyond my knowledge, so that was the best I could do.

This was one of the first changes I made. I can see all of the hotspots with Xcode Instruments.