Route native logging through the log crate - #174
Draft
ruccho wants to merge 2 commits into
Draft
Conversation
Replace every println!/eprintln! across the unienc crates with the `log` facade, and install a single log::Log implementation in unienc_c that picks a sink per record: - Android: __android_log_write under the `unienc` tag, regardless of the `unity` feature. - Unity (non-Android): IUnityLog, once UnityPluginLoad has provided the plugin interfaces. - Otherwise: stdout/stderr. Remove set_stdout_redirect, which redirected the process-wide stdout and stderr into logcat from JNI_OnLoad. It captured output from every other library in the process and collided with anything else doing the same. Add unienc_set_log_level so the managed side can change the threshold; the default is Info for release builds and Debug for debug builds. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The IUnityLog sink added in the previous commit was unreachable on Windows and Linux: build-unienc.yml passed only -F mimalloc for their `unity` variant, so those players fell back to println and nothing reached the Unity console. -F unity did not compile there. `PlatformEncodingSystem::unity_plugin_load` is called unconditionally from unienc_c, but `UnityPlugin` was implemented only by the Metal and Vulkan backends, which are the ones with a rendering integration. Add empty implementations for Media Foundation, FFmpeg, and WebCodecs so the trait's no-op defaults apply. Each backend gets its own `unity` feature forwarding to `unienc_common/unity` rather than depending on it unconditionally, so non-Unity builds of those crates keep excluding unity-native-plugin. wasm stays on --no-default-features: unity-native-plugin-sys 0.9.0 asserts 64-bit struct layouts and cannot compile for a 32-bit target. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Closes #170.
What
Native-side logging was ad-hoc
println!throughout, with an Android-specific hack thatredirected the process-wide stdout/stderr into logcat. This replaces all of it with the
logfacade plus a single logger implementation in
unienc_c.All 47
println!/eprintln!call sites acrossunienc_android_mc,unienc_apple_vt,unienc_ffmpeg,unienc_windows_mf,unienc_webcodecs, andunienc_cnow uselog::{error,warn,info,debug,trace}!, assigned by the nature of each message.Sinks
The logger picks a destination per record:
__android_log_writeunder theunienctag, regardless of theunityfeature.IUnityLog, onceUnityPluginLoadhas supplied the plugininterfaces.
the plugin) — stdout/stderr.
Choosing per record rather than at install time means records emitted before
UnityPluginLoadfall back instead of being dropped, and records emitted after
UnityPluginUnloadstop using aninterface that is being torn down. A Unity version that does not expose
IUnityLogalso fallsback.
Android stays on logcat even under the
unityfeature:JNI_OnLoadruns well beforeUnityPluginLoad, the encoder logs from MediaCodec, tokio, and Vulkan threads that are notUnity's, and logcat is where native failures are actually diagnosed.
Windows and Linux now build with
-F unityThe
unityfeature was introduced for the Metal and Vulkan blit paths, sobuild-unienc.ymlpassed
-F unityonly for macOS, iOS, and Android. Windows and Linux received-F mimallocalone.
That meant the Unity builds shipped for Windows and Linux never used
IUnityLog— with thechange above they would have silently fallen back to
println, and native diagnostics wouldnever have reached the Unity console on those platforms. The main goal of this issue would have
been missed on two of five platforms.
-F unitydid not previously compile there.unienc_ccallsPlatformEncodingSystem::unity_plugin_loadunconditionally, butUnityPluginwas implementedonly by the two backends that have a rendering integration. This adds empty implementations for
Media Foundation, FFmpeg, and WebCodecs, which fall through to the trait's existing no-op
defaults, and switches the Windows and Linux
unityvariants to-F unity,mimalloc.Each backend gains its own
unityfeature forwarding tounienc_common/unity, rather thandepending on it unconditionally the way the Metal and Vulkan backends do. This keeps
unity-native-pluginout of the non-Unity (NuGet) builds of those platforms, which currentlyexclude it.
This changes the shipped Windows and Linux Unity binaries. They now export
UnityPluginLoad/UnityPluginUnloadand Unity will call them. Besides enablingIUnityLog,that entry point runs
mimalloc::unity::init, routing mimalloc's OS-level allocations throughIUnityMemoryManager— the same path already used on macOS, iOS, and Android, but notpreviously exercised on Windows or Linux. Please validate on real player builds for both
platforms before release.
wasm keeps
--no-default-features:unity-native-plugin-sysasserts 64-bit struct layouts andcannot compile for a 32-bit target. It was never built with
unitybefore this change either.Log level
Defaults to
Infofor release builds of the native library andDebugfor debug builds, sodebug!/trace!never ship enabled. A newunienc_set_log_levelFFI entry point, exposed asUniEnc.NativeLogging.SetLevel, changes the threshold at runtime. Several previouslycommented-out per-frame diagnostic dumps are now live at
trace!rather than requiring a sourceedit to enable.
Behavior change on Android
set_stdout_redirectis removed. Itdup2'dSTDOUT_FILENOandSTDERR_FILENOprocess-widefrom
JNI_OnLoad, so every library's stdout/stderr in the process — including Unity's own —was re-tagged as
uniencin logcat, and it collided with any other library doing the same.After this change, only UniEnc's own records carry the
unienctag. Anything that previouslyappeared under
uniencmerely because it was written to stdout by unrelated code will no longerappear there. Native code outside UniEnc that relied on this redirect to reach logcat at all
will now need its own logging.
Verification
cargo check -p unienc_c --features unitypasses foraarch64-apple-darwin,aarch64-apple-ios,aarch64-linux-android,x86_64-pc-windows-msvc, andx86_64-unknown-linux-gnu. Builds without the feature pass for macOS, Windows, Linux, and wasm.cargo fmt --checkis clean andCargo.lockgains no new dependencies (logwas alreadypresent transitively, so
THIRD-PARTY-NOTICES.mdis unchanged).Native binaries need a
build-unienc.ymlrun before this is usable from the package.