Repository navigation
feat(playback): bring back and widen 3D controls and format coverage (#210) - #221
Conversation
BGLuis
left a comment
There was a problem hiding this comment.
The epic lands in five clean, one-issue commits, and the hard parts are done carefully: the FOV is packed into polar180 instead of growing StereoPushConstants (now pinned by static_assert), the 2D view is a single edit in GetStereoParams that both RecordStereoFrame paths pick up through sp.eyeIndex, MONO_VIEW is cleared in reset_process_state() next to SWAP_EYES, and the GLES fallback gets the JNI symbols so it can't hit UnsatisfiedLinkError. ManifestGuardTest follows the UiSourceGuardsTest pattern and fails on new and stale entries, as #215 asks.
No blocking findings. My comments are mostly about the filename detector, where the tokenizer changes more than #212 describes.
Main points
- Projection order in
detect_from_filename: the bare-360check now runs before the cubemap checks, sohas_token("360")in the 6x1 rule can never fire, and dot/dash names liketour.360.6x1.mp4go fromCubemap6x1MonotoSpherical360Mono(reproduced; inline). - The detector gains matches #212 didn't ask for: a bare
vrtoken now means VR180 SBS (family_vr.mp4), and barefisheye/fisheye180|200|220/mkx200map to fisheye, while the FOV from the name is never used, soclip_fisheye_220plays at 190° (inline). - 2D view stays on across files and the swap is sticky too. #213 left that as an open question; could the answer be written down in the issue?
Smaller
- The
detect_from_filenamedoc comment andFilenameProjectionvariant docs still describe substring matching. - New and edited comments in
ScreenFormatModal.ktandstereo.fragare in Portuguese (CLAUDE.md asks for English). polar_mode.hisn't in the CIclang-formatfile list.ManifestGuardTestreads the source manifest, so permissions merged in from a dependency's AAR aren't covered.15 || 16and190are repeated as literals.
Dependencies: no lockfile or manifest version changed (dependency_diff.py). New library calls: SharedPreferences.edit().putInt().apply(), the same pattern as Format3DPreferenceStore; std::atomic and slice::windows. Nothing deprecated there, and the build showed no deprecation warnings on changed lines. Toolchain pins and dependency placement unchanged.
Checked: every changed line against CLAUDE.md (layering Kotlin→JNI→C++→Rust, GLES frozen, i18n positional placeholders and parity, English comments), the four-place screen-mode sync (no index changed), and the acceptance criteria of #211–#215.
Ran (in a separate worktree at 7b844b0): cargo fmt --check, cargo clippy -p protocols -p media-logic --all-targets --all-features -- -D warnings, cargo test -p protocols -p media-logic (all pass); ./scripts/check-shaders.sh (22 OK); ENABLE_ASAN=1 ./scripts/test-native-host.sh (all pass, including TestPolarModeEncodeDecode); clang-format --dry-run -Werror on polar_mode.h and test_screen_mode.cpp (clean); ./gradlew ktlintCheck testDebugUnitTest :app:lintDebug -PrustBuildMode=skip (pass).
Probes (thrown away afterwards): an old-vs-new run of detect_from_filename on about 40 names (the source of the detector points). #215 negatives: adding READ_PHONE_STATE, or removing ACCESS_WIFI_STATE from the manifest, makes ManifestGuardTest fail. Note that the literal criterion implementation("com.google.firebase:firebase-analytics:1.0.0") fails in checkDebugAarMetadata because that version doesn't exist, before the guard runs; with the same coordinate as a plain string in build.gradle.kts, testNoAnalyticsOrCrashReportingDependenciesInBuildGradle fails as intended.
Not run: assembleDebug with the real native + Rust build (the static_assert and the Vulkan/GLES TUs compiled only in CI's build-apk, which is green), and anything on the Quest 3: the headset criteria of #211, #213 and #214 are still unverified.
…ted markers Addresses review feedback on #221 (threads #r4366250178, #r4366250180, #r4366250186): - Check EAC and Cubemap projections before bare 360 so that names like tour.360.6x1.mp4 resolve to Cubemap6x1Mono instead of Spherical360Mono. - Require vr token to be an interior token flanked on both sides (restoring the old _vr_ invariant across separators) so boundary tokens like family_vr.mp4 or Movie.VR.mkv do not false-positive as VR180. - Remove unrequested fisheye variants (mkx200, fisheye200, fisheye220, fisheye180, bare fisheye/eyefish) to maintain strict scope of #212. - Update doc comments for detect_from_filename and FilenameProjection variants to describe whole-token matching.
…tants Addresses review feedback on #221 (thread #r4366250193): - Add ScreenFormatCatalog.isFisheye(index) alongside isStereo to centralize checking modes 15 and 16. - Replace repeated literal 190 with FisheyeFovPreferenceStore.DEFAULT_FOV across ScreenFormatModal, VRControlsPresentation, and VRModalPresentation. - Add testIsFisheye in ScreenFormatCatalogTest.
Addresses review feedback on #221 (thread #r4366250188): - Translate Portuguese comments in ScreenFormatModal.kt (3D controls and fisheye sections) to English. - Translate Portuguese comments in native/shaders/vulkan/stereo.frag (mapping, feathering, fisheye projection, crop, SGSR1 bypass, night mode) to English per repository conventions.
…t manifest guard scope Addresses review feedback on #221 (threads #r4366250195, #r4366250200): - Add native/include/polar_mode.h to the clang-format header list in .github/workflows/main.yml and GEMINI.md. - Document in ManifestGuardTest that the test checks source AndroidManifest.xml and does not cover merged permissions from dependency AARs.
…og KDoc to English - Accurately state in detect_from_filename doc comment that specific projections (Fisheye, EAC, Cubemap) precede 360, which precedes VR180-specific markers and bare 180. - Add tests for flight.360.eac.mp4 and space.360.cubemap.mp4 asserting precedence over bare 360. - Translate isFisheye KDoc in ScreenFormatCatalog.kt from Portuguese to English.
|
Addressed in 3d50613: 2D view is kept sticky across video loads during the playback session (matching Swap Eyes), and is only reset on full player teardown in |
Summary
Implements Epic #210 across 5 atomic commits covering container allowlists, whole-word 3D filename token detection, 3D view controls in the format modal, per-file fisheye lens FOV, and manifest/analytics CI guards:
141d626) — File browser container allowlists:.m4v,.m2ts,.mts, and.mk3dtoDirectoryLister.VIDEO_EXTENSIONSandfolder_scan::MEDIA_EXTENSIONSwhile keeping.vob,.asf,.ogv, and.rmexcluded.a4c7041) — Whole-word 3D filename token matching:rust/media-logic/src/format3d.rs(detect_from_filename) and matches whole tokens and adjacent token pairs (half+sbs,3d+sbs, etc.), addingfsbs,fou, andtb.180,360,360x180,180x180) only as whole tokens so resolution tags (360p,180p) and words sharing a prefix (out,ou2,lrx,table) no longer trigger false positives..fsbs.,.fou., and.tb.patterns toMediaFilterEngine.kt.3d50613) — Restore Swap Eyes and add 2D view inScreenFormatModal:MONO_VIEWatomic flag (toggle_mono_view,get_mono_view) inrust/bridge/src/lib.rs, cleared onreset_process_state().eyeIndex = 0andswapEyes = 0for both eyes inGetStereoParamswhenMONO_VIEWis active on a stereo layout.nativeGetSwapEyes,nativeToggleMonoView,nativeGetMonoView) in bothvr_player_jni_vulkan.cppandvr_player_app.cpp.ScreenFormatCatalog.isStereoand a "3D view" toggle row (player_btn_swap_eyesandplayer_btn_2d_view) inScreenFormatModal.ktacross all 3 locales.017a362) — Per-file Fisheye lens FOV (180°, 190°, 200°, 220°):polarMode(bits0..7) andfisheyeFov(bits8..23) into the 32-bitpolar180field vianative/include/polar_mode.h, preservingsizeof(StereoPushConstants) == 128(enforced bystatic_assert).native/shaders/vulkan/stereo.frag(defaulting to190°when unset) and maskspolar180 & 0xFFin debug stats and logs.FisheyeFovPreferenceStorekeyed byhistoryKeyand a 4-option angle selector inScreenFormatModalforFisheye190andFisheye190SBS.7b844b0) — Manifest permission & analytics dependency guard test:app/config/manifest-guards/permissions.txtfromAndroidManifest.xmland addsManifestGuardTestto fail on unapproved or stale<uses-permission>/<uses-feature>entries and on analytics/crash-reporting artifacts inapp/build.gradle.kts.Verification
cd rust && cargo fmt --checkcd rust && cargo clippy -p protocols -p media-logic --all-targets --all-features -- -D warningscd rust && cargo test -p protocols -p media-logic./scripts/check-shaders.shclang-format --dry-run -Werroron native test and header filesENABLE_ASAN=1 ./scripts/test-native-host.sh(includingTestPolarModeEncodeDecode)./gradlew ktlintCheck testDebugUnitTest :app:lintDebug -PrustBuildMode=skip(includingDirectoryListerTest,MediaFilterEngineTest,ScreenFormatCatalogTest,FisheyeFovPreferenceStoreTest,ManifestGuardTest,I18nParityTest,UiSourceGuardsTest)needs-headsetitems in [FEATURE] File browser: list .m4v, .m2ts, .mts and .mk3d videos #211, [FEATURE] Screen format modal: restore swap eyes and add a 2D view of 3D #213, [FEATURE] Fisheye: make the lens field of view a per-file setting #214)Closes #211
Closes #212
Closes #213
Closes #214
Closes #215
Closes #210