Repository navigation
Review of Potentially Useful Improvements from a Fork #186
Replies: 1 comment
|
Thanks for sharing your work! You've clearly put a lot of effort into these changes, and some of the improvements look potentially useful. However, the discussion description is quite extensive, making it difficult to go through everything in detail, especially when much of the work would require a thorough review and testing before considering it for upstream integration. That said, I already have several improvements and new features planned for upcoming updates. As mentioned in the important notice in the README, this is likely my final year of active development, and I'm currently focusing on my remaining planned updates while managing my limited time. I'm also preparing to restrict new pull request submissions to help manage my workload during this period. Given that, I'd encourage you to continue developing your fork independently, where you can experiment with these changes and tailor the experience to your TV. With that in mind, I'll close this discussion here. |
Uh oh!
There was an error while loading. Please reload this page.
Hi BrightCraft,
Thank you for Moonlight Tizen! I own a Samsung S90C (55", Tizen 9), and I made a fork of 1.19.0 that is tuned for it (branch ccr-749486c7-cvis0k, release v1.19.0-s90c.1). It started as an S90C project, but most of the changes aren't specific to the S90C, so they may help on other Samsung TVs too (I could only test on mine). I wanted to show you what changed in case some of it is useful for the main version. No pressure at all.
Highlights
2 s picture freeze fixed: Moonlight Tizen 1.19.0 never enables RFI. Without RFI, a wrongly predicted frame loss made the depacketizer drop every frame until the 120-frame limit forced an IDR frame (2 s at 60 FPS). The prediction now only runs with RFI. This is the same change as upstream moonlight-stream/moonlight-common-c@62e0663, so you can take it straight from upstream.
The stream no longer keeps running in the background: after Home or a switch to another app, it now ends after 1 minute, or right away when the TV takes the video away. Before, it ran on with no limit, kept the network busy and could make the TV hang when another app started its video.
Stop and launch races fixed: pressing RED during "Starting…" (the stream could start anyway, without audio), a double stop (it could tear the connection down twice and in the worst case crash the WASM engine), and a Smart Hub tile starting a second stream on top of a running one.
Faster FEC recovery: a matrix multiply in reed_solomon_new() that wasn't needed blocked the video receive thread for about 15 ms per recovered block (242 data shards, desktop CPU, longer on a TV). It now takes about 0.4 ms and the output is identical. This only matters while the vendored rs.c is kept, because upstream moved to nanors.
Steadier frame pacing (Video frame pacing, on by default): one late frame no longer holds back the following frames by up to tens of ms.
Codec fallbacks: the app now uses the codec modes the host reports. A host without 10-bit HEVC gets 8-bit HEVC instead of an encoder failure. A host that can't encode the selected AV1 gets HEVC instead of 4K H.264 without HDR.
A message instead of a black or frozen picture when the EMSS player keeps rejecting frames, the media source fails or the WASM module crashes.
Less load while streaming: UI gamepad polling pauses during the stream (the WASM input thread already polls the gamepads). Controller state is only sent when it changes, plus a refresh every 100 ms. With an idle controller that is about 10 instead of 200 packets per second, so there is less Wi-Fi uplink traffic.
Web Audio backend only: lost packets and underruns no longer leave the audio delay raised. Before, each lost packet added 5 ms, and after an underrun the delay stayed high for minutes.
All changes since 1.19.0
Video
Main files: wasm/wasmplayer.cpp, wasm/main.cpp, wasm/platform/index.js, wasm/platform/messages.js
Frame pacing rework (Video frame pacing, on by default):
Frames are released at the median arrival time plus 3 standard deviations, estimated from the median absolute deviation. This is capped at half a frame above the median.
Arrival is measured from when the frame was queued, so the pacer's own delay doesn't feed back into it.
A later arrival time is only followed once two windows in a row confirm it. Before, the burst after a network stall held back the next second of frames, which overflowed the decode unit queue at 120 FPS. An earlier arrival time applies right away.
In simulations of Wi-Fi jitter and stalls, the added delay went from up to about 40 ms down to a few ms. Only in the case with many large stalls was the cadence slightly less even.
One slice per frame: the app asks the host for one slice per frame instead of CAPABILITY_SLICES_PER_FRAME(4). This compresses better at the same bitrate, and the hardware decoder gets whole frames anyway.
Rec. 709 for SDR: 1.19.0 leaves colorSpace at its default, Rec. 601, which is meant for SD video. The app now requests Rec. 709, the standard for HD and 4K. This avoids a colour shift when the TV ignores the colour space signalled in the stream. HDR keeps Rec. 2020.
Codec negotiation uses the serverCodecModeSupport the host reports, like moonlight-qt:
The value is read again after the launch or resume request (limited to 1.5 s, with the earlier value as fallback), because Sunshine probes its encoders again at launch, for example when a virtual display only appears at that point.
A value that lists neither HEVC nor AV1 (a failed or stale probe) is not trusted.
A host without 10-bit HEVC gets HEVC Main (8-bit, no HDR) instead of an encoder failure.
When AV1 is selected, HEVC with the same bit depth is offered as a fallback. AV1 is left out if the host can't encode it at all. It is also left out if the host can't encode 10-bit AV1 for HDR but can encode HEVC Main10, so HDR is kept.
A notice tells the user when the host could only provide another codec, or SDR instead of HDR.
The AV1 warning now says what AV1 needs (an encoder on the host, a decoder in the TV) and that the stream falls back to HEVC.
HEVC/AV1 level 5.2 is declared for streams above the luma sample rate of level 5.1 (for example 4K90/120/144 or 1800p120). If the TV rejects it, the app falls back to level 5.1. Streams up to 4K60 are unchanged.
New resolution 3200 x 1800 (1800p) with bitrate presets, as a step between 1440p and 4K. It is only offered on 4K and 8K TVs.
Experimental: HDR static metadata (Video Settings, off by default):
Inserts the HDR10 metadata of the host display into HEVC IDR frames as prefix SEI NAL units: the mastering display colour volume and, when the host provides it, MaxCLL/MaxFALL.
Sunshine (at least with NVENC) only sends this metadata over the control channel, and the app discarded it, so the TV had to assume defaults for its tone mapping.
A keyframe is requested when usable metadata arrives or changes during the stream.
Only applies to HEVC HDR streams.
TV screensaver is turned off while streaming (also in gamepad-only sessions) and turned back on afterwards.
Audio (Web Audio backend only; EMSS stays the default)
Main files: wasm/platform/audio.js, wasm/auddec.cpp, wasm/index.html, wasm/static/locales/*.json
Fix: each lost packet added 5 ms of audio delay. libopus filled the whole 10 ms slot to conceal the loss. Now it conceals the loss with exactly one negotiated frame.
Fix: after an underrun, the backlog that arrives back to back is trimmed to the jitter buffer plus two frames. Before, the delay stayed raised for minutes, up to the drift bound. Trimmed frames are counted separately from skipped frames.
The drift bound is lowered from 2× to 1.5× the jitter buffer. Audio jitter buffer can now go down to 25 ms (was 50 ms).
Experimental: Audio clock sync (Audio Settings, off by default):
Changes the resampling ratio by up to 500 ppm (0.05 %, not audible) to keep the delay at the jitter buffer.
Without it, drifting host and TV clocks lead to skipped 5 ms frames every few minutes, or to a gap as long as the buffer.
It only ends a correction on a whole sample, so there are no clicks, and it smooths the measured lead over about 1 s.
The resampler uses a four-point cubic kernel instead of linear interpolation, which keeps the treble within 0.5 dB at 10 kHz. It only resamples when the TV output isn't 48 kHz or clock sync is active. At the plain ratio it passes the samples through unchanged.
Host silence (Sunshine sends no packets) no longer counts as an underrun in the stats and the log. The rule is a gap longer than twice the jitter buffer, and at least 250 ms. The sound itself doesn't change.
The help texts of Audio clock sync and Audio jitter buffer now describe the real behaviour and point to the suggested buffer size in the overlay.
Connection and stability
Main files: wasm/platform/index.js, wasm/main.cpp, wasm/wasmplayer.cpp, wasm/platform/common.js, wasm/platform/messages.js, wasm/connectionlistener.cpp, moonlight-common-c/ (see the separate block below)
Fix: FEC recovery speed (reedsolomon/rs.c): removed a multiply that wasn't needed (see Highlights). The parity output was checked to be identical over about 1,200 block sizes.
Fix: picture freeze of up to 2 s (120 frames at 60 FPS) after a wrongly predicted frame loss without RFI. The prediction now only runs with RFI (upstream moonlight-stream/moonlight-common-c@62e0663). With RFI, a partly received IDR frame now leads to a new IDR request instead of an RFI attempt (upstream moonlight-stream/moonlight-common-c@be43885).
Experimental: Reference frame invalidation (Video Settings, HEVC and AV1 only, off by default): recovers from packet loss without a full keyframe, which avoids traffic spikes and blurry IDR frames. If errors stay in the picture after a loss, turn it off again. See the note on moonlight-stream/moonlight-common-c@d85371c in the moonlight-common-c block below.
LAN/Wi-Fi detection: Samsung's Ethernet port is usually 100 Mbps, so over Ethernet the bitrate presets are capped at 75 Mbps and a warning appears above 80 Mbps.
"Slow connection to PC. Reduce your bitrate!" now only appears for real network loss. Frames the TV drops itself (decode unit queue overflow, the wait after a decoder reset) no longer trigger it.
Fix: stream left running in the background:
It now ends after 1 minute hidden. A hidden page alone doesn't end it sooner, so TV overlays such as the quick settings are meant to leave it running. I'm still checking this on my TV.
It ends right away when the TV closes the video track for a reason other than a suspension, or while the connection is still being set up.
A notice explains how to resume. The game keeps running on the host.
A start that is still in progress no longer starts a stream once the app is hidden. This covers the server info refresh, the delayed start after quitting another game, and deep links.
EMSS player errors:
A full player buffer is retried for up to one frame duration.
If the player keeps rejecting the format for 3 s, the stream ends with a message instead of requesting keyframes forever on a black screen. This uses the new LiTerminateConnectionAsync.
A media source failure during the stream or during setup ends the stream with a message instead of a frozen or black picture.
If the WASM module aborts (for example on OOM) or one of its threads crashes, a dialog appears and the app restarts instead of freezing silently.
Fix: stop races:
A repeated stop (RED twice, the gamepad combo and a host termination at the same time) now starts only one teardown and one return to the app list (m_StopIssued, atomic m_Running).
The teardown after a host or player termination is left to the stop thread, so LiStopConnection can't run twice at the same time.
Fix: RED during "Starting…" now cancels the start and ignores the late reply from the host. Before, the stream could start anyway, without audio and with RED/YELLOW not working. Holding RED triggers only one stop, because key auto-repeat is ignored.
Fix: a Smart Hub deep link could start a second stream on top of a running one. It now shows "Stop the current stream first (RED button)." Host selection and a second start are also ignored while a stream is active.
More logging (no effect on the picture):
the UDP receive buffer size the TV grants (no longer only logged in LC_DEBUG builds)
a line when the TV doesn't support SO_RCVTIMEO (the receive thread then polls before each receive)
why the TV closed a media track
when the video track reopens, and how many frames were discarded in the meantime
when the app is hidden and shown
player rejection reasons by name
the WASM heap top at the end of each stream
UI and settings
Main files: wasm/platform/index.js, wasm/platform/navigation.js, wasm/platform/remote_controller.js, wasm/platform/gamepad.js, wasm/input.cpp, wasm/index.html, wasm/static/locales/*.json, wasm/static/css/style.css
The bitrate is kept when the resolution, frame rate, codec or HDR changes. This applies to a hand-picked bitrate and to a recommended one.
Before, it jumped back to the preset on every resolution or frame rate change, even when the current value was selected again. On codec or HDR changes it only jumped back with Optimize bitrate presets on.
Toggling Optimize bitrate presets makes the presets apply again until the bitrate is changed by hand. moonlight-qt works the same way.
A one-time notice explains how to get the presets back. Another one-time notice appears when a preset is capped at 75 Mbps over Ethernet.
After an update, an install keeps following the presets if the stored bitrate was the preset for the stored resolution and frame rate, or if Optimize bitrate presets was on.
Bitrate menu: UP/DOWN now change the bitrate in 5 Mbps steps. LEFT/RIGHT keep the 0.5 Mbps steps. The navigation help says so.
New warnings:
H.264 above 1080p, also at startup when H.264 is stored with a resolution above 1080p
90 FPS, which doesn't fit evenly into the TV's 60/120 Hz output
4K above 120 FPS (the limit comes from the S90C spec, see the fork-specific section)
The one-time warning checks run again after a settings profile or the defaults are applied.
Fix: the resolution/frame rate warning compared the values as text instead of numbers.
Fix: Restore Defaults turned off Video frame pacing and Host address field mode, although both are on by default.
Fix: with the pointer locked, the Back key pressed the host's left mouse button and never released it. Locking the pointer also no longer requests a keyframe, and an unused focus handler was removed.
Gamepad buttons still held when the stream ends (for example the stop combo) no longer trigger menu actions. A held D-pad no longer keeps scrolling at stream start.
"Loading Apps..." is shown after a stream ends until the app list has loaded.
Help texts:
Optimize game settings: what it does on Sunshine
Game mode: leave it off on Tizen 9+ with both packages
Optimize bitrate presets: it computes a low bitrate (about 45 Mbps for 4K60 HEVC HDR)
Video HDR: HDR must also be on at the host display, otherwise the stream is 10-bit SDR
Translations: new strings are in en-US.json. pt-BR.json has the new keys, but about half of them are still in English.
Performance (less load on the TV)
Main files: CMakeLists.txt, wasm/gamepad.cpp, wasm/platform/gamepad.js, wasm/connectionlistener.cpp, wasm/wasmplayer.cpp, wasm/main.cpp
The engine is built with -O2 instead of -Os (compile and link).
UI gamepad polling pauses while streaming. It ran every 5 ms on the main thread (getGamepads, button comparison, events), although the WASM input thread polls the gamepads itself. It resumes with a fresh snapshot.
One call per gamepad poll: all gamepads are read with one main-thread call per poll instead of 2 + 1 per slot. That is about 200 instead of about 1,200 calls per second. If the bundled call isn't available, it falls back to the Emscripten gamepad functions.
Controller state is only sent on change, with a refresh every 100 ms and a retry if the input queue drops an event. Before, it was 200 reliable packets per second. Now it is about 10 per second with an idle controller, which means less Wi-Fi uplink traffic.
The stats overlay, connection warnings and rumble are handed to the UI without blocking the decoder or receive thread.
Fix: PostToJsAsync (unused in 1.19.0) handed the main thread a pointer to a string that could already be freed before the main thread read it. It now passes a copy, which the main thread frees.
The frame pacer only spins for the last 0.2 ms instead of 1 ms. usleep in a worker thread already waits with Atomics.wait.
Performance overlay (Settings → Advanced Settings → Performance statistics, off by default)
Main files: wasm/wasmplayer.cpp, wasm/platform/index.js, wasm/platform/audio.js, moonlight-common-c/src/{Limelight.h,RtpVideoQueue.c,VideoDepacketizer.c,VideoStream.c}
Configured bitrate.
Host frame rate (30 s), measured from the host's capture timestamps. It shows when a host display runs at 59.94 instead of 60 Hz. Frames the host encodes again from the previous image are left out.
Timings split up: Average frame receive time, Average frame wait time and Pending frames (with the maximum per second). These lines replace Average decoding time, because it measured queue wait and pacing together rather than decoding. The pacing line is now called Average frame pacing delay.
Session totals:
Session frames dropped by network
Session frames dropped by the TV: queue overflow or rejected by the player, with the number of video pipeline resets
Session recovery keyframes: only real recoveries. Keyframes the app requests itself (track open, HDR metadata) and keyframes after pipeline resets are not counted. Each client-side cause covers one IDR frame and expires after a short time.
Frames that arrive while the TV's video track is closed (app hidden) count as neither network nor TV drops.
Network connection: Ethernet (with the note "TV port limited to 100 Mbps") or Wi-Fi. On Wi-Fi it adds the signal strength, refreshed every 5 s. The strength is left out when the TV reports NaN (the S90C does).
FEC and UDP buffer:
Session FEC recoveries: count, longest recovery, slow recoveries (8 ms or more), failed recoveries
Worst FEC block: lost packets compared with parity packets
UDP receive buffer: requested compared with reported
TV player rejections by reason: buffer full, keyframe required, track not ready, other. It also shows how many frames the retry saved.
WASM heap top, against the fixed 128 MB, to spot leaks in long sessions.
Moonlight hidden during the stream: how often the app was hidden and how often the TV closed the video, with the last reason.
Web Audio lines: jitter buffer, schedule lead, browser latency, underruns, skipped and lost frames, and the clock sync correction. They also suggest a buffer size: with clock sync, after 10 minutes without underruns; without clock sync, after an underrun as a hint to turn clock sync on.
Build and CI
Main files: .github/workflows/release-stable.yml, .github/workflows/release-development.yml, Dockerfile, CMakeLists.txt
If both release workflows run at the same time, they push the ForceGM image to the same :forcegm tag, so one run could upload the other run's package. A shared concurrency group (moonlight-release-images, no cancel-in-progress) queues them. It's a 3-line change per workflow.
The Dockerfile takes the one WGT file the Tizen CLI produced, whatever its name, and fails clearly if there isn't exactly one. The Tizen CLI names the package after in config.xml, so the rename needed this. Otherwise it changes nothing. The download names stay Moonlight.wgt and Moonlight-ForceGM.wgt.
-O2 instead of -Os (see Performance).
Changes inside the vendored moonlight-common-c
moonlight-common-c/ is a vendored copy here, not a submodule: the entry in .gitmodules is commented out, and submodulesVer.txt lists 8599b60 (June 2024). These changes are in that copy (8 files, +195/−33):
reedsolomon/rs.c: the FEC speed-up described above. Upstream moonlight-common-c has since moved its FEC to nanors (moonlight-stream/moonlight-common-c#125), so updating the copy might be the cleaner long-term route. I haven't compared the two.
src/RtpVideoQueue.c, src/VideoDepacketizer.c: backports of upstream moonlight-stream/moonlight-common-c@62e0663 ("Don't speculatively report losses if RFI is disabled", fixes moonlight-stream/moonlight-common-c#147) and moonlight-stream/moonlight-common-c@be43885 ("Fix handling of a partially dropped IDR frame"). Both are newer than the vendored 8599b60.
src/RtpVideoQueue.c also gets per-session FEC statistics.
src/VideoDepacketizer.c also counts the frames the client dropped itself (LiGetClientDroppedVideoFrames, LiGetClientVideoResets) and keeps them out of the connection quality warning.
src/VideoStream.c, src/PlatformSockets.c:
the reported UDP receive buffer size and LiGetVideoNetworkStats()
the receive buffer logs ("Selected receive buffer size", "Unable to set receive buffer size", "Actual receive buffer size") no longer need LC_DEBUG
a log line when SO_RCVTIMEO isn't supported
src/Connection.c, src/Limelight.h: new LiTerminateConnectionAsync(), so decoder and renderer callbacks can end the connection safely.
Updating the copy to current upstream would bring both backported fixes and nanors at once, but your Tizen patches in Platform.h/PlatformSockets.c would have to be carried over.
The copy doesn't have upstream moonlight-stream/moonlight-common-c@d85371c ("Fix RFI after a multi-block frame loss") yet. If you take the experimental RFI setting, that fix (or an update of the copy) should come with it.
Fork-specific (probably not for upstream)
Name and IDs: the app is called "Moonlight S90C", with package ID MLightS90C, its own widget, application and service IDs, and an "S90C" badge on the icon. That lets it install next to the regular Moonlight. For the same reason, the Smart Hub preview uses port 8889 (tiles saved by earlier builds are moved to it) and its own preview file prefix, and clearing the box art cache leaves the regular app's files alone. All of this only matters for two installs side by side. (Files: res/config.xml, res/icon.png, res/service/service.js, wasm/static/js/utils.js.)
"Samsung S90C optimization" button (Basic Settings): it shows the values for the S90C and applies them after confirmation. It gives tips for the detected Ethernet or Wi-Fi connection and warns when the TV isn't an S90C. On Tizen 9+ it points to the ForceGM package, or says that ForceGM is already installed. The idea of a "recommended settings" dialog that asks before applying might be reusable, but the values are for one model.
Defaults: 720p / 10 Mbps / H.264 became the highest resolution up to 4K, 75/100/80 Mbps (LAN / Wi-Fi / unknown), HEVC, HDR on HDR-capable TVs, and Unlock all frame rates on. These suit the S90C, but they are probably too aggressive as upstream defaults for older or weaker TVs. The same goes for the 100 Mbps Wi-Fi value and the 4K-above-120-FPS warning limit, which both come from the S90C's decoder spec.
German docs: an optimization plan, a quick guide, release notes, and a note in the README.
Release workflow:
The pre-release description is built from docs/RELEASE_NOTES_DEV.md.
A tested pre-release can be promoted to a full release under a new tag (promote_tag/release_tag). This is needed because the in-app update check reads the timestamp tag of a pre-release as newer than 1.19.0.
Testing
The release contains the unchanged files of the pre-release built from 95f5b12. It runs on my S90C (Tizen 9, ForceGM package) with Sunshine on an RTX 5080, over 5 GHz Wi-Fi through a repeater: 4K60 HDR with HEVC at 100–150 Mbps and with AV1 at 130–140 Mbps. In my overlay tests up to 140 Mbps there were no dropped frames and no recovery keyframes. At 150 Mbps with the EMSS audio backend I sometimes heard audio crackles.
I haven't tested other TV models, and I haven't tested the experimental settings systematically.
Both packages build with the existing Docker toolchain (Samsung's Emscripten 1.39.4.7) in the release workflow.
Some parts were also checked off the TV. For example, I compared the Reed-Solomon parity over about 1,200 block sizes and ran the frame pacer and the audio logic in simulations. These were one-off scripts and aren't in the repo.
Links
Fork: https://github.com/Emperior90/moonlight-tizen-S90c (its default branch master is still plain 1.19.0; the changes are on the branch below)
Branch: https://github.com/Emperior90/moonlight-tizen-S90c/tree/ccr-749486c7-cvis0k
Release (the notes there are in German; the changelog below is the English list): https://github.com/Emperior90/moonlight-tizen-S90c/releases/tag/v1.19.0-s90c.1
Changelog: https://github.com/Emperior90/moonlight-tizen-S90c/blob/27950d5a4bb19bd8a62051690f157852d2106b37/CHANGELOG.md#unreleased-samsung-s90c-optimizations
Compare: master...Emperior90:moonlight-tizen-S90c:ccr-749486c7-cvis0k
The branch is based on the v1.19.0 tag, so it doesn't include #185 (the Embind std::string fix) yet. #185 also touches the Dockerfile, which my branch changes too, so I'd rebase that part when splitting into PRs.
I wrote these changes with the help of Claude Code (an AI coding assistant). I tested the result on my TV as described above, but please review them with that in mind. I'm happy to explain or rework anything.
In total it's about 3,000 added lines in 32 code files (not counting docs), mostly in wasm/platform/index.js and wasm/wasmplayer.cpp, so separate PRs would probably be much easier to review. If any of this is interesting for you, I'm happy to split it into separate, focused pull requests without the fork-specific parts and rebased on current master. For example, one for the moonlight-common-c fixes, one for the background and stop handling, and one for the frame pacer. Just tell me which parts you'd like. Thanks again for all your work on Moonlight Tizen!
All reactions