Recover from decoder failures, and let mods actually see the player - #44
Merged
Merged
Conversation
…me category - VideoListener regained the codec blacklist + retry machine and the MediaCodecSelector that filters it; these exist in TGC 0.34.5 and were missing here, so a failing decoder or a transient network error killed playback permanently instead of retrying. - New CanvasQueryHostNumber(key, out): a generic, keyed channel for exposing host-side values to user libs. Every exported symbol here is ABI we carry forever, so this is one weak-linked entry point rather than one per feature - new capabilities become a string key, not more lock-in. extern "C" keeps mangled names out of that contract, and older Canvas builds resolve null. Number rather than Value because a double is atomic and reads lock-free from any thread, while strings need a caller buffer and a lock; widening this later would put that cost on every numeric key, so they belong in a separate CanvasQueryHostString. Video is just the first consumer - duration, position, is_live, is_seekable, is_buffering, is_ended - sampled in UpdateMediaPlayer, which already runs on the thread owning the player, since ExoPlayer must not be touched elsewhere. - android:appCategory="game", which Sky declares and this did not.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this unlocks
Today a user lib can start a video and stop it. That is the entire vocabulary.
It cannot tell how long the video is, how far into it you are, whether it is a live stream, or whether it is still buffering.
So nobody can build a seek bar. Not because seeking is missing - the engine has always had it - but because a mod has no idea where it currently is or how far it is allowed to go. It is a steering wheel with the windscreen painted over.
This opens the windscreen.
video-timeline-srubbing.mp4
Why this belongs in Canvas and not in a mod
The player lives here. It also insists on being touched from exactly one thread, and a mod is native code that has no business reaching across into Java to go looking for it.
So these values have to be read on this side, where that is safe, and handed over. There is no version of this that lives in a mod.
One door, instead of a hallway of doors
The obvious way to expose this is a method per thing:
Six values means six exported symbols, each one ABI this project carries forever and its own "does this Canvas have it yet" check on the mod side. Then someone wants playback speed and it is seven. Then a track title, and it is eight. Each perfectly reasonable alone, and collectively a hallway of single-purpose doors that can never be closed again.
CanvasQueryHostNumberis one door. Mods ask for a value by name, so the next one costs a name rather than a symbol. Older builds simply answer "I do not have that", and the mod hides the feature instead of breaking.It is deliberately
CanvasQueryHostNUMBERand notCanvasQueryHostVALUE. A number can be handed instantly and safely from any thread. Text cannot: it needs a buffer to copy into and a lock to copy under, and folding that in would put the cost on every numeric read forever. So if text is ever wanted it belongs in a separateCanvasQueryHostString, and the narrower name is what keeps that door open.For the same reason, absence is signalled with NaN rather than a magic number like -1. A sentinel would quietly fence off part of the range for every key that ever exists: an audio balance, a live-edge offset, a pitch shift in semitones are all legitimately negative, and all would read as "no value" at exactly the readings that matter. NaN is not a number any key would want to return, so nothing is off-limits.
For mod authors
The whole of the mod side:
Both failure modes land in the same branch, so there is no version check to get wrong.
Keys currently answered:
video.duration_msvideo.position_msvideo.is_livevideo.is_seekablevideo.is_bufferingvideo.is_endedAlso in here
Decoder recovery, restored. TGC's own player retries a failed video a few times, and blacklists a decoder that keeps failing so it can fall back to a different one. Canvas lost that at some point, so one failing decoder or a brief network hiccup killed playback permanently. This puts it back, matching TGC's behaviour.
The app declares itself a game. Sky does. Canvas did not. One line.
Testing
The host values are confirmed on a device - the video above is a mod reading them and driving a real seek bar.
The decoder recovery is verified against TGC's shipped code, but not exercised at runtime: it only fires on an actual decoder or network failure, which is hard to stage on purpose.