fix(player): make native Android video opt-in again — it shipped as audio with no picture
Publish Documentation / Build & publish docs to gitea-pages (push) Canceled after 0s
🏗️ Build and Test JellyTau / Run Tests (push) Failing after 4m55s
🏗️ Build and Test JellyTau / Android Compile Check (push) Skipped
Traceability Validation / Check Requirement Traces (push) Successful in 20s
Publish Documentation / Build & publish docs to gitea-pages (push) Canceled after 0s
🏗️ Build and Test JellyTau / Run Tests (push) Failing after 4m55s
🏗️ Build and Test JellyTau / Android Compile Check (push) Skipped
Traceability Validation / Check Requirement Traces (push) Successful in 20s
DR-161 flipped experimentalNativeVideo on by default so picture-in-picture could shrink a real video surface. On a device that shipped sound with a blank screen. The decode path was never at fault. Logcat shows ExoPlayer running and feeding a live SurfaceView with an active BufferQueue. The compositing was: the SurfaceView sits behind the WebView, and the step that clears the opaque layers above it never took effect — `WebView transparent = false` is logged, `= true` never appears. The video was rendering correctly the whole time, behind an opaque page. This is precisely the defect the flag existed to contain; VideoPlayer.scrubRegression.test.ts had already recorded that "the native SurfaceView has never been visible through the webview". Enabling it by default shipped a verified decode path on top of an unverified display path. Reverting costs nothing that matters: PiP does not depend on it — DR-160 drives PiP from the WebView <video> — and working video outranks PiP showing a native surface. The flag stays in Settings, now described as incomplete rather than as a performance win, so anyone helping test it still can. Fixing the compositing is the prerequisite for trying this default again (DR-172).
This commit is contained in:
@@ -23,12 +23,12 @@ import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
// ---- Mocks (must precede component import) --------------------------------
|
||||
|
||||
const channelHandlers: Record<string, (event: any) => void> = {};
|
||||
// These tests pin the **flag-off** interim behaviour: when `experimentalNativeVideo`
|
||||
// is off, VideoPlayer overrides Android's native backend response to HTML5
|
||||
// rendering and stops the native backend. That flag now defaults to *on*
|
||||
// (DR-160, so picture-in-picture has a real surface to shrink into), so the
|
||||
// default no longer selects this path and the tests have to say which path they
|
||||
// are guarding rather than inherit it. (DR-161)
|
||||
// These tests pin the **flag-off** behaviour: when `experimentalNativeVideo` is
|
||||
// off, VideoPlayer overrides Android's native backend response to HTML5
|
||||
// rendering and stops the native backend. That is the default again (DR-172,
|
||||
// after native video shipped as audio with no picture), so this mock now agrees
|
||||
// with the default rather than opposing it — kept explicit so the tests state
|
||||
// which path they guard instead of inheriting whatever the default happens to be.
|
||||
vi.mock("$lib/stores/nativeVideo", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("$lib/stores/nativeVideo")>();
|
||||
return {
|
||||
|
||||
@@ -26,12 +26,12 @@ import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
// ---- Mocks (must precede component import) --------------------------------
|
||||
|
||||
const channelHandlers: Record<string, (event: any) => void> = {};
|
||||
// These tests pin the **flag-off** interim behaviour: when `experimentalNativeVideo`
|
||||
// is off, VideoPlayer overrides Android's native backend response to HTML5
|
||||
// rendering and stops the native backend. That flag now defaults to *on*
|
||||
// (DR-160, so picture-in-picture has a real surface to shrink into), so the
|
||||
// default no longer selects this path and the tests have to say which path they
|
||||
// are guarding rather than inherit it. (DR-161)
|
||||
// These tests pin the **flag-off** behaviour: when `experimentalNativeVideo` is
|
||||
// off, VideoPlayer overrides Android's native backend response to HTML5
|
||||
// rendering and stops the native backend. That is the default again (DR-172,
|
||||
// after native video shipped as audio with no picture), so this mock now agrees
|
||||
// with the default rather than opposing it — kept explicit so the tests state
|
||||
// which path they guard instead of inheriting whatever the default happens to be.
|
||||
vi.mock("$lib/stores/nativeVideo", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("$lib/stores/nativeVideo")>();
|
||||
return {
|
||||
|
||||
@@ -27,26 +27,32 @@ const STORAGE_KEY = "jellytau-experimental-native-video";
|
||||
const NATIVE_VIDEO_ATTR = "data-native-video";
|
||||
|
||||
/**
|
||||
* Whether the native path is on, defaulting to **on** when the user has never
|
||||
* chosen.
|
||||
* Whether the native path is on. **Off** unless the user turned it on.
|
||||
*
|
||||
* It shipped defaulting to off while the native path was a spike. It is now the
|
||||
* default because picture-in-picture is built on it: PiP shrinks the *Activity*,
|
||||
* so it needs a real video surface behind the WebView to show, and on the HTML5
|
||||
* path there is nothing for it to shrink into but the UI itself (DR-160).
|
||||
* DR-161 briefly made this default to on, so picture-in-picture could shrink a
|
||||
* real video surface. On a device that shipped as **audio with no picture**:
|
||||
* ExoPlayer decoded correctly and fed its SurfaceView, but the SurfaceView sits
|
||||
* *behind* the WebView and the compositing that clears the opaque layers above it
|
||||
* never took effect — logcat showed `WebView transparent = false` and never
|
||||
* `= true`. So the video was rendering the whole time, behind the page.
|
||||
*
|
||||
* That is the defect the flag existed to contain, and it is why the default is
|
||||
* back off: video working matters more than PiP showing the native surface, and
|
||||
* PiP still works without it via the HTML5 path (DR-160). Native video remains
|
||||
* available in Settings for anyone testing it.
|
||||
*
|
||||
* An explicit stored choice still wins in both directions, so anyone who turned
|
||||
* it off keeps it off.
|
||||
* it on keeps it on.
|
||||
*
|
||||
* TRACES: UR-003, UR-004 | DR-172
|
||||
*/
|
||||
function load(): boolean {
|
||||
if (typeof localStorage === "undefined") return true;
|
||||
if (typeof localStorage === "undefined") return false;
|
||||
try {
|
||||
const stored = localStorage.getItem(STORAGE_KEY);
|
||||
return stored === null ? true : stored === "true";
|
||||
return localStorage.getItem(STORAGE_KEY) === "true";
|
||||
} catch {
|
||||
// Private-mode / disabled storage — no stored choice is readable, so this is
|
||||
// the same case as "never chosen".
|
||||
return true;
|
||||
// Private-mode / disabled storage — default to the safe (HTML5) path.
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -74,8 +80,8 @@ function createExperimentalNativeVideoStore() {
|
||||
}
|
||||
|
||||
/**
|
||||
* User opt-out for the native Android video path. **Defaults to on** since
|
||||
* DR-161 — see `load()`. The name still says "experimental" because the flag
|
||||
* User opt-in for the native Android video path. **Defaults to off** again since
|
||||
* DR-172 — see `load()`. The name says "experimental" because the flag
|
||||
* remains a suppressor of Rust's backend choice, not a promoter of it.
|
||||
*/
|
||||
export const experimentalNativeVideo = createExperimentalNativeVideoStore();
|
||||
|
||||
@@ -97,9 +97,9 @@ export function setAutoEnterEnabled(enabled: boolean): void {
|
||||
* needs for the PiP window's aspect ratio and the play state for its play/pause
|
||||
* action.
|
||||
*
|
||||
* The flag defaults to **on** now (DR-161), so Android normally shrinks the real
|
||||
* ExoPlayer surface instead; this remains the path for Linux and for anyone who
|
||||
* turned the flag off.
|
||||
* The flag is back to defaulting **off** (DR-172, after native video shipped as
|
||||
* audio with no picture), so this is once again the path Android normally takes —
|
||||
* which is why PiP does not depend on that flag being on.
|
||||
*
|
||||
* Pass `active: false` when the element goes away, or PiP would be offered over a
|
||||
* video that is no longer there.
|
||||
|
||||
@@ -744,10 +744,10 @@
|
||||
</h3>
|
||||
<p class="text-sm text-gray-400 mt-1">
|
||||
Decode video with the device's hardware decoder instead of the
|
||||
built-in web player. Better performance and battery life, and
|
||||
required for picture-in-picture to show the video rather than
|
||||
the app. Still less tested — turn this off if video fails to
|
||||
appear or seeking misbehaves.
|
||||
built-in web player. Better performance and battery life in
|
||||
principle, but incomplete: on some devices the picture does not
|
||||
appear at all and only the sound plays. Leave this off unless
|
||||
you are helping test it.
|
||||
</p>
|
||||
</div>
|
||||
<button
|
||||
|
||||
Reference in New Issue
Block a user