RetroPlayer: hardware rendering for game clients (OpenGL and OpenGL ES)

[sunlollyking]

Game clients that render with OpenGL — Flycast, Mupen64Plus-Next, melonDS, PPSSPP, YabaSanshiro, Dolphin and others — could not run in Kodi at all. RetroPlayer only accepted frames a client had drawn in software and handed over as pixels. This adds the path that lets a client render on the GPU instead: Kodi gives it a framebuffer, the client draws into it, and Kodi draws the texture that framebuffer is backed by. The frame never leaves the GPU.

Builds on Lukas Rusak's WIP FBO buffer, pool and renderer, preserved with their original authorship.

Rebased onto master, off the achievements base

This used to be stacked on the RetroAchievements callbacks, which made it hard to test independently — @garbear said as much on this PR. It is now rebased onto current xbmc/xbmc master and carries nothing but this work, in eight commits meant to be read in order.

No ABI break any more

Earlier revisions moved the Game API to 7.0.0 with _MIN to match, because the hardware framebuffer structures grow. That is no longer necessary. Measured against master:

	master	this branch
game_stream_properties	28	28
game_stream_buffer	24	24
game_stream_packet	36	36
AddonToKodiFuncTable_Game	88	88
KodiToAddonFuncTable_Game	392	392

The two structures that grow sit inside unions whose largest members are the video and software-framebuffer ones, so nothing an existing add-on was built against moves. 7.2.0 with _MIN left at 7.1.0 — clients built for 7.1.0 keep loading and simply never use the new fields.

GL and GLES

Both, from shared code. The GLES path is a thin delta rather than a second implementation — the differences are a handful of guarded lines, because a GL core profile has no default vertex array object and GLES does not care.

GLES is where this has actually been run: LibreELEC is GLES, and the Piers Linux flatpak has moved to GLES too.

What works

System	Core	Context
Nintendo 64	Mupen64Plus-Next (GLideN64)	OpenGL 3.3 core
GameCube	Dolphin	OpenGL 3.3 core / GLES 3.2
Dreamcast	Flycast	OpenGL 3.0 core
Nintendo DS	melonDS	OpenGL 3.1 core
Sega Saturn	YabaSanshiro	OpenGL compatibility
PlayStation Portable	PPSSPP	OpenGL compatibility

Video filters, scaling, view modes and rotation all work on hardware-rendered games, which they never did before. Rewind and savestates work alongside it. Quitting returns to a working GUI.

Independently reproduced by @kel-mo on the tv.kodi.Kodi flatpak (AMD Radeon 880M, Mesa, GLES 3.2, Wayland), including the negotiation ladder correctly refusing a desktop-GL context on a GLES build before settling on GLES 3.2.

The frame race, since it shaped the design

A client is given one framebuffer and draws into it every frame. Publishing that same framebuffer to the rendering thread means sampling a frame the client is already drawing the next one into, and what goes missing is whatever it drew last — in most games the HUD. Fences do not fix it: a fence orders the writes before it, and the racing writes belong to the next frame. Handing the client alternating buffers is worse, because clients cache the framebuffer they are given.

What works is copying on the client's thread, between its frames, where the frame is whole, and publishing the copy.

Four commits that are not hardware rendering

These rode along on the old branch and are each proposed against xbmc/xbmc master on their own, so this PR can be reviewed as rendering work alone:

Commit	Upstream PR
Games: remember which emulator to open a game with	xbmc#28965
RetroPlayer: don't offer the whole filesystem as savestates	xbmc#29017
RetroPlayer: slow the game down when the audio sink is full, don't flush	xbmc#29031
Games: say when a rumble request goes nowhere	xbmc#29032

They stay here so the branch is testable as one piece, and will drop out as each lands.

Paired add-on change

kodi-game/game.libretro#164. That one now builds against stock xbmc/xbmc master as well, detecting the new fields at configure time, so it is not blocked on this landing.

Testing status, stated plainly

Everything above was verified on hardware before this was rebased onto master. Since the rebase it builds clean (full GLES build, zero errors) but has not been re-run on hardware. I would not want that taken on trust — I am putting it up for review now rather than sitting on it, and will re-run the full set and report back.

[sunlollyking]

Garbear - aware you've got a lot on - this is super exciting functionality i think you'd be interested in. I'm wanting lots of feedback from a wide base of users on code structure and all the errors it probably contains. Ive added as much defensive programming as i can but aware i've not consulted anyone really on what is quite a complicated architecture. Also wanting to open this up to the libreelec team for review too.

Happy to take any feedback on approach curious for you to test but it genuinely works and gets us a long way to full compatibility.

Edit: I'll also eventually squash commits keeping them all visible at the moment in case everyone seems happy and we can look at incremental cherry pick merge.

[garbear]

This is so cool! Only problem is, it's hard for me to test atm as my branch has a lot of unfinished work for B2 that conflicts with the patchset here. If we can start getting everything else merged, we'll get a master branch we can rebase on that I can easily test out.

[sunlollyking]

No stress ! We'll get it all in and Q is going to hopefully be a huge RP release.

[garbear]

B2 will freeze most code, so it'll be a stable point where we can start developing for Q*. Now I just gotta bet B2 finished and shipped 🙂

[sunlollyking]

Tested end to end today on both renderers. The flickering that made hardware-rendered games unplayable is fixed, and video filters, scaling and rotation now work on OpenGL ES for the first time.

Root cause of the flickering. A hardware-rendering client was handed one framebuffer and drew into it every frame, and that same framebuffer was published to the rendering thread. So the thread sampled a frame while the client was already drawing the next one into it, and what came out missing was whatever the game drew last. In Crazy Taxi the sky held still while the cars and the HUD flickered, because the sky is drawn first and they are drawn last.

Two approaches that do not work, both tried:

• Fencing. A fence orders the writes that came before it; the writes racing the read belong to the frame after.
• Handing the client a second framebuffer to alternate with. Clients cache the framebuffer they are given rather than asking for it again, so swapping it out leaves them drawing into one surface while Kodi samples the other — worse than the race it replaces.

What works is copying instead. When the client signals a finished frame the pool blits it into a buffer of its own and publishes the copy, so the client keeps the single stable framebuffer it expects and the rendering thread samples a surface nobody is drawing into. The copy is taken on the client's thread, between its frames, where the frame is whole. Two targets are used in turn because the rendering thread may still be sampling the one published last frame.

Worth noting for anyone debugging something similar: enabling a video filter does not mask this. The filter path copies too, but on the rendering thread at render time, so it faithfully copies a half-drawn frame.

A second fix was needed for desktop GL specifically. A core profile has no default vertex array object, so the draw that puts the game on screen was rejected outright with GL_INVALID_OPERATION and the picture stayed black. OpenGL ES permits the default object, which is why the same code path worked there and hid it. Also fixed alongside: the client's texture was bound without selecting unit 0 first, and the GUI shader's depth uniform was never set. All three are guarded so the OpenGL ES path is unchanged.

Test results

	OpenGL	OpenGL ES
Flickering	fixed	fixed
Video filters	working	working
Scaling / view modes	working	working
Rotation	working	working
Dropped frames	none	none
Audio pacing	unchanged	unchanged

OpenGL was tested on a desktop Wayland build (Mesa, Intel), OpenGL ES on a LibreELEC Generic image on real hardware — GL_VERSION = OpenGL ES 3.2. Games tested with flycast: Crazy Taxi, AeroWings, Alone in the Dark, Army Men. The OpenGL ES run shows no GL, shader or filter errors in the log at all.

One measured cost: an extra blit per frame. No dropped frames and no change in audio pacing on either renderer.

Two known issues, neither a regression here: the GBC and PSP overlay filter presets clip out of bounds (2 of 616 presets, both upstream libretro presets, cosmetic), and a client that fails to boot a disc image can take the Kodi process down with it, which wants an error dialog rather than a hard exit.

I'll push the two commits onto this branch next.

[sunlollyking]

I've replaced this branch with the line of work that is actually tested, so it can be tried on either renderer. The previous branch is preserved at d83c9ac23d if any of it is wanted back.

Why it was replaced rather than added to. The old branch carried three approaches to the flickering that are now known not to work, and leaving them in the history would invite reviewers to reason about them:

• synchronise the client's drawing with our sampling — a fence orders the writes before it; the writes racing the read belong to the frame after
• stop the client and the renderer contending for one texture / one framebuffer per client — handing the client a second framebuffer to alternate with makes it worse, because clients cache the framebuffer they are given and carry on drawing into the old one

What works is copying the finished frame on the client's thread and publishing the copy, so the client keeps the single stable framebuffer it expects and the rendering thread samples a surface nobody is drawing into.

Testing. Both renderers, on real hardware:

	OpenGL	OpenGL ES
Flickering	fixed	fixed
Video filters	working	working
Scaling / view modes	working	working
Rotation	working	working
Dropped frames	none	none
Audio pacing	unchanged	unchanged

OpenGL on a desktop Wayland build (Mesa, Intel). OpenGL ES on LibreELEC, on two machines — Intel, and an AMD Radeon 780M — reporting GL_VERSION = OpenGL ES 3.2. Games via flycast: Crazy Taxi, AeroWings, Alone in the Dark, Army Men, plus GameCube under dolphin. Across 32 game launches the OpenGL ES logs show no GL, shader or filter errors, and the frame-copy path never fell back.

For anyone testing: a LibreELEC Generic image is OpenGL ES and an OpenGL device image is desktop GL, and game add-ons must be built for the same one as the frontend — a mismatch is refused at negotiation and looks like a core that errors on load rather than anything renderer-related.

One thing to be aware of: this branch is stacked on the RetroAchievements callbacks from #150, which is one commit of the 65 here. Everything else is renderer work. Happy to rebase it off if you would rather review the two independently.

Two known issues, neither introduced here: the GBC and PSP overlay filter presets clip out of bounds (2 of 616, both upstream libretro presets, cosmetic), and a client that fails to boot a disc image can take the Kodi process down rather than reporting an error.

[sunlollyking]

Withdrawing one of the two "known issues" I listed earlier: the claim that a client failing to boot a disc image can take the Kodi process down.

That came from scripted launches on my side, and those runs turned out to be unreliable for reasons I never got to the bottom of — the same crash appeared on both renderers, and on a fully reverted tree where my changes could not have caused it. Booting the same image by hand, repeatedly, does not reproduce it. So it is far more likely an artefact of how I was driving Kodi than a real fault, and it should not be in front of reviewers as a defect.

The other known issue stands: the GBC and PSP overlay filter presets clip out of bounds (2 of 616, both upstream libretro presets, cosmetic).

[kel-mo]

Tested this on the tv.kodi.Kodi flatpak (freedesktop 25.08 runtime, APP_RENDER_SYSTEM=gles, Wayland) on x86_64 — AMD Radeon 880M (Mesa reports it as 890M), Mesa 26.1.5, GLES 3.2. Kodi built from this PR's head (d64fa22) paired with kodi-game/game.libretro#164 (893c7b8), and the libretro Dolphin fork (libretro/dolphin@0cd3bb8) built against system libs.

Result: working, and genuinely playable. A GameCube title (.rvz) negotiated the ladder exactly as designed — GL core/2.x correctly refused on the GLES build, then Kodi created a shared OpenGL ES 3.2 context and Dolphin came up on its OGL backend. Extended gamepad play session at full speed; launching the same title twice within one Kodi session also worked cleanly, so the stream close/reopen path got some exercise. Clean exits throughout. This is the first GameCube video Kodi has ever produced here — nice work.

Three observations from the debug log, in case they're useful:

1. The shared-context handshake reports contradictory outcomes — the core logs SetHWRender - unable to set shared context for OpenGL ES 3.2 while Kodi logs Created shared OpenGL ES 3.2 context for the game client immediately after.
2. Dolphin's AsyncShaderCompiler can't start its worker threads (Failed to create shared context for shader compiling), so shader compilation falls back to synchronous. Presumably related to (1).
3. GL_INVALID_FRAMEBUFFER_OPERATION in glClear (incomplete framebuffer) fires exactly once per game launch, shortly after the stream opens — looks like a first-frame race before the FBO is complete; non-fatal and consistent.

Two issues hit along the way turned out to be unrelated to these PRs, noted for anyone reproducing: the generated game.libretro.dolphin addon requires game.controller.nes while its topology only accepts game.controller.gamecube (in-game input is dead until the GameCube profile is installed manually — will report to kodi-game), and Dolphin's default GCI-folder memory card loops at "checking memory card in Slot A"; a raw card (SlotA = 1 in Dolphin.ini) works fine.

An aarch64 build of the same stack is being exercised via flathub/tv.kodi.Kodi#775. Happy to run further tests on this hardware (AMD/Mesa/GLES/Wayland flatpak) if useful.

[sunlollyking]

@kel-mo thank you, that is the first time anyone but me has run this and it is on a stack with nothing in common with mine — different GPU, different Mesa, flatpak rather than a distro or LibreELEC build. Worth saying that your log exercised the refusal path in the negotiation ladder, where a GL core context is turned down on a GLES build before ES 3.2 is accepted. Nobody had tested that but me, and it is the part I was least sure of.

All three observations chased down. Two fixes pushed, one is not ours.

1. The handshake is not contradictory, but the logging deserved the confusion. Two different meanings of "shared" sitting next to each other. Kodi's line means the client's context shares Kodi's objects. Dolphin's warning is about RETRO_ENVIRONMENT_SET_HW_SHARED_CONTEXT, which asks for something else entirely: a promise the core may create further contexts of its own from it. game.libretro declines that, and declined it silently, so all you saw was the core's complaint. Both statements were true and they read like a contradiction.

There were also two log lines announcing the same context, one of which only added the word "shared". Dropped the duplicate, said what the survivor actually shares, and added a line on the add-on side explaining the refusal (kodi-game/game.libretro#164, 33a80e9).

2. Not ours, and not fixable from here. Dolphin's libretro build uses GLContextLR, which doesn't override CreateSharedContext(), so it inherits the base implementation returning nullptr unconditionally. SharedContextAsyncShaderCompiler::WorkerThreadInitMainThread calls exactly that, so the worker threads can never start no matter what the frontend answers — the same under RetroArch. Answering true to the environment call would be a promise we can't keep and would change nothing, so it stays a refusal, now a documented one. Synchronous shader compilation is the cost until the Dolphin port grows a real context implementation.

3. This one was real, and it is the interesting one. The client's context is deliberately made current with no surface — the client draws into its own framebuffer and never needs one. BeginClientFrame() refuses to take that context on a thread while another thread holds it, and returns false without nesting. EndClientFrame() had no matching check, and callers pair a Begin with an End regardless of what Begin answered. So a refused call still decremented the depth, it hit zero a level early, and the context was released out from under the thread that really held it. If that thread is Kodi's rendering thread it is left with no default framebuffer, and the next clear has nothing to write to.

A stream opens across two threads, which is where the mismatch comes from, and the picture recovers on the following frame. That gives exactly the signature you describe: one incomplete-framebuffer error, at glClear, just after the stream opens, non-fatal, consistent. Fixed in b9d94cff4b by giving EndClientFrame() the check its counterpart already makes.

Being straight about what that last one is worth: I found it by reading, not by watching it happen, and I have not confirmed the error is gone. Scripted launches on my machine die at game load for reasons I never got to the bottom of — the same unreliability behind the crash claim I withdrew earlier in this thread — so I could not reproduce your log locally. The defect is real and wrong on its own terms regardless. Whether it is your error is unproven. If you get the chance to re-run on b9d94cff4b I would like to know either way, and if it is still there, the twenty or so lines around it would help.

The GameCube input problem is ours, not a kodi-game one to report. game.libretro.dolphin imports game.controller.nes while the topology only accepts game.controller.gamecube, so on a clean system the profile is never installed and there is nothing for the port to connect to. Fix opened at kodi-game/game.libretro.dolphin#10. It has been there since that repo's initial commit, so it looks like it came from whichever add-on it was templated off.

On the aarch64 builds in flathub/tv.kodi.Kodi#775 — nothing to do with this stack. The runs are failing in build-x86_64 while fetching sources for the libdisplay-info module, The requested URL returned error: 504, retried four times and out. build-aarch64 never ran at all; the matrix cancels it when its sibling fails. So aarch64 is untested rather than broken, and it wants a rebuild when the mirror is behaving rather than a change.

[sunlollyking]

Rebased and out of draft.

Off the achievements base. You said back in August that this was hard to test because it was stacked on unfinished work, and that once things were merged there would be a master to rebase on. There is now, so this sits on current xbmc/xbmc master and carries nothing but the rendering work.

The ABI break is gone. That was the other thing making it awkward — 7.0.0 with _MIN to match meant every existing add-on stopped loading. Measured rather than assumed: the two hardware framebuffer structures do grow, but they sit inside unions dominated by the video and software-framebuffer members, so game_stream_properties, game_stream_buffer, game_stream_packet and both function tables are byte-for-byte the size they are on master. It is 7.2.0 with _MIN left at 7.1.0, and 7.1.0 clients keep loading.

Rewritten as eight commits meant to be read in order: the API, the windowing accessors, the buffer and pool, the renderer, the end-to-end path, then three that are not hardware rendering at all and were only ever tangled in with it. Happy to split those three out if you would rather.

Two things dropped along the way. The gamesgeneral.enableopengl setting is gone — it only ever dodged the DMA path, and xbmc#28983 fixed the coherency bug it was working around. And the rumble commit is now just the reporting, since the delivery fix landed upstream when InputReceiver() was changed to answer with the port's receiver.

One thing I want to be straight about: all the hardware testing in the description happened before this rebase. It builds clean since — full GLES build, no errors — but I have not re-run games on it. I would rather say that than have you find out by trying it. Re-running the set is my next job and I will report back either way.

The paired add-on change, kodi-game/game.libretro#164, now builds against stock master too, so it is not waiting on this.

[garbear]

Can you rebase on master again? I just synced with upstream.

[garbear]

I merged xbmc#28965, good time to rebase that out.

[sunlollyking]

Rebased onto master — now 0 behind, and MERGEABLE/CLEAN where it was conflicting.

Down from 9 commits to 7, because two are upstream now:

• "don't offer the whole filesystem as savestates" — dropped automatically, that is RetroPlayer: don't offer the whole filesystem as savestates xbmc/xbmc#29017 which merged.
• "remember which emulator to open a game with" — skipped deliberately. Games: remember which emulator to open a game with xbmc/xbmc#28965 merged in reworked form, so master already has xbmc/games/database/ and a GameUtils.cpp that uses it. Keeping mine would have been a second, older implementation of the same thing.

Three conflicts, all resolved by taking both sides rather than choosing:

• versions.h — master had already gone to 7.2.0 for both the version and the minimum, so I kept its raised _MIN rather than the branch's 7.1.0.
• RPStreamManager.cpp — master added SetVideoFps(), the branch adds BeginClientFrame()/EndClientFrame(). Both kept.
• GameClient.h — master made the frame and sample rates atomic; the branch made GetSerializeSize() lazy. Kept both: m_framerate.load() with SerializeSize().

One thing worth flagging because skipping the emulator commit nearly hid it. That commit also defined GAME_BIOS_DIRECTORY, the shared BIOS folder, and its only consumer is in "open the hardware rendering path end to end" — so dropping the commit left the tree referencing a constant that no longer existed. Caught it on the build rather than in review; the constant now sits in the commit that uses it, so each commit stands on its own.

Builds clean against current master, zero errors.

[garbear]

Drop unnecessary whitespace

[sunlollyking]

Gone.

[garbear]

I'd say remove this and rename below to GetSerializeSize()

[sunlollyking]

Done — GetSerializeSize() is now the only accessor, declared where the wrapper used to be, and the five ReversiblePlayback callers follow it. No wrapper left.

[garbear]

Some review:

• strings.po needs fixing
• Game.h changes need dropping
• Probably change everything to Game API v7.3.0 (with 7.3.0 also min) (rcheevos PR will bump to 8.0.0, so this can then target 8.1.0 after that's merged)

I went through the rest of the code and it looks good. I noticed earlier that some commits touched code from previous commits. We might want the rendering stuff squashed into 1 atomic commit, unless you can break it up in a way that makes sense, and compiles between each commit.

[sunlollyking]

All four done, force-pushed. Branch is 0 behind master and builds clean.

strings.po — two problems, not one. The two graphics-API messages had taken 35302/35303 while the last defined string is 35298, leaving 35299–35301 unaccounted for with no empty-range marker. And the removed flicker setting had left an orphan behind:

#. Label of a setting that works around speckled or flickering video in games...
#: system/settings/settings.xml
#empty strings from id 35304 to 35504

a comment and a source reference with no msgctxt or msgid under them. The messages now take 35299 and 35300, the orphan is gone, and the file runs contiguously to #empty strings from id 35301 to 35504.

Game.h — dropped. Checking what it actually contained: the whole diff was the AudioAvailable() block and its ADDON_AudioAvailable thunk moved to a different position, byte-identical either side. Pure churn, so the file is back to master.

Version — 7.3.0, minimum 7.3.0. Worth recording why it isn't 7.2.0: master's 7.2.0 is your "Allow setting game FPS/samplerate" (25a2bf39dd), which leaves game_stream_hw_framebuffer_properties as char dummy. This branch grows that structure to carry a size. Two different layouts under one number is the thing that bites later, so it takes the next one. If #150 lands first at 8.0.0 I will re-target this to 8.1.0.

Commits — squashed the rendering work into one, and you were right that it had to be: they genuinely did not stand alone. The history is now:

Game API 7.3.0: give the hardware framebuffer a size
RetroPlayer: hardware rendering for game clients
Games: say when a rumble request goes nowhere
RetroPlayer: slow the game down when the audio sink is full, don't flush

I kept the API bump separate because it is the one commit with an ABI consequence, and that seemed worth being able to point at on its own.

One thing to flag: the bottom two are already open as xbmc#29032 and xbmc#29031. I have left them here so the branch still builds and runs as a unit, but they are not rendering work and can come out the moment either lands — say the word if you would rather they went now.

I also caught a self-inflicted one while restructuring: the first pass swept some untracked scratch files of mine into the squash. They are out, and the tree is exactly the previous content plus the four fixes above — I diffed it against the pre-restructure branch to be sure nothing else moved.

[garbear]

Yes, bottom 2 are upstreamed, we'll continue discussion there, so nix them from here.

2 commits is fine, but the strings.po changes bled into one and screwed up the second. Lets just combine them.

[sunlollyking]

Both done — force-pushed, one commit now, builds clean and 0 behind master.

Rumble and audio are out. Dropped rather than reverted, so nothing of them remains in the tree; they live on in xbmc#29032 and xbmc#29031 as you say.

The two commits are combined. You were right about the bleed and it was worse than cosmetic — strings.po was touched by both: the API commit added the two graphics-API messages, and the rendering commit then renumbered them and cleaned up the orphan. So the first commit committed IDs the second immediately changed, and neither stood alone as a description of what happened. One commit now, strings.po touched once.

Final shape:

RetroPlayer: hardware rendering for game clients (Game API 7.3.0)

I checked the result against the pre-change branch rather than assuming: the only difference is the four rumble and audio files, nothing else moved.

That leaves this PR as exactly the rendering work plus the API bump it depends on, which I think is what you were after.