dtourolle/jellytau
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases 38 Wiki Activity

refactor: delete two orphans, and record why the reparent design changed

Browse Source 
`resolveVideoSource` chose between a local file and a remote URL for video
playback. Backend-owned stream selection took that decision into Rust —
`media_local_selection` for a downloaded file, `get_stream_selection` for a
streamed one — and its last caller went with it. What remained was the function
plus sixty lines of tests exercising nothing that ships.

`fittedVideoSize` computed the rendered size of a video letterboxed into its
container. Nothing has ever called it: it arrived with the fix that made the
video fill its viewport and was superseded by `object-fit: contain` in the same
change. There is some irony in a helper that models letterboxing sitting unused
beside a container that was not letterboxing at all — the bug fixed in the
previous commit was CSS, and this function would not have helped.

A survey for exported symbols referenced only by their own tests finds 22 more.
Most are legitimate — test mocks, deliberate reset hooks, public utility APIs —
and the rest are unrelated to this work, so they are left for a cleanup that can
be reviewed on its own terms rather than smuggled into a playback branch.

Also records in the spec why DR-231's design changed. Reparenting Tauri's
webview into a GtkOverlay aborts the process on the first click: Linux calls
`attach_resize_handler` unconditionally (the Windows path guards it with
`is_decorated()`), and its handler walks webview -> GtkBox -> GtkWindow with an
unwrap that an overlay breaks. So the webview is not moved at all — mpv draws
into the default vbox's own `draw` handler via `gdk_cairo_draw_from_gl()`, and
GTK's container-before-children order puts the webview on top for free. No
reparent, one less widget, and nothing a Tauri upgrade can invalidate by
assuming its own layout.
This commit is contained in:
dtourolle
2026-08-22 13:45:04 +02:00
parent 0445a6d0aa
commit 7545de6cc7
6 changed files with 211 additions and 387 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/lib/components/player/videoFit.test.ts
+1 -37
View File
@@ -1,5 +1,5 @@
import { describe, it, expect } from "vitest";
import { videoFitClass, fittedVideoSize } from "./videoFit";
import { videoFitClass } from "./videoFit";
describe("videoFitClass", () => {
it("fills the container instead of capping at the source's intrinsic size", () => {
@@ -19,39 +19,3 @@ describe("videoFitClass", () => {
expect(cls).not.toContain("object-fill");
});
});
describe("fittedVideoSize", () => {
it("scales a 480p source up to fill a larger window (the reported bug)", () => {
// Exact 16:9 480p in a 1920x1080 window -> scales up to fill, rather than
// staying a 854x480 box in the middle.
const size = fittedVideoSize(853.33, 480, 1920, 1080);
expect(size.width).toBeCloseTo(1920, 0);
expect(size.height).toBeCloseTo(1080, 0);
});
it("fits to the constraining dimension when aspect ratios differ", () => {
// 4:3 source in a 16:9 window -> height-constrained, pillarboxed.
const size = fittedVideoSize(640, 480, 1920, 1080);
expect(size.height).toBeCloseTo(1080, 0);
expect(size.width).toBeCloseTo(1440, 0);
expect(size.width).toBeLessThan(1920);
});
it("fits to width when the source is wider than the window", () => {
// 21:9 source in a 16:9 window -> width-constrained, letterboxed.
const size = fittedVideoSize(2560, 1080, 1920, 1080);
expect(size.width).toBeCloseTo(1920, 0);
expect(size.height).toBeCloseTo(810, 0);
expect(size.height).toBeLessThan(1080);
});
it("shrinks oversized media to fit rather than overflowing", () => {
const size = fittedVideoSize(3840, 2160, 1280, 720);
expect(size.width).toBeCloseTo(1280, 0);
expect(size.height).toBeCloseTo(720, 0);
});
it("returns a zero size for unknown intrinsic dimensions", () => {
expect(fittedVideoSize(0, 0, 1920, 1080)).toEqual({ width: 0, height: 0 });
});
});
src/lib/components/player/videoFit.ts
-29
View File
@@ -15,32 +15,3 @@
export function videoFitClass(): string {
return "w-full h-full object-contain";
}
export interface FittedSize {
width: number;
height: number;
}
/**
* The rendered size of a video of the given intrinsic dimensions once it has
* been fitted into the container - i.e. scaled (up or down) so that it touches
* the container on its constraining axis, with the other axis letter/pillar
* boxed. Mirrors what `object-fit: contain` on a full-size element does.
*/
export function fittedVideoSize(
intrinsicWidth: number,
intrinsicHeight: number,
containerWidth: number,
containerHeight: number,
): FittedSize {
if (intrinsicWidth <= 0 || intrinsicHeight <= 0) {
return { width: 0, height: 0 };
}
const scale = Math.min(containerWidth / intrinsicWidth, containerHeight / intrinsicHeight);
return {
width: intrinsicWidth * scale,
height: intrinsicHeight * scale,
};
}
src/lib/player/localSource.test.ts
+1 -72
View File
@@ -1,5 +1,5 @@
import { describe, it, expect } from "vitest";
import { downloadedFilePath, resolveVideoSource } from "./localSource";
import { downloadedFilePath } from "./localSource";
describe("downloadedFilePath", () => {
// The download worker rewrites `downloads.file_path` to the absolute path it
@@ -29,74 +29,3 @@ describe("downloadedFilePath", () => {
expect(downloadedFilePath("C:\\Users\\u\\AppData\\jellytau", stored)).toBe(stored);
});
});
// A stand-in for Tauri's convertFileSrc, so the module stays pure.
const toAssetUrl = (p: string) => `asset://localhost/${encodeURIComponent(p)}`;
describe("resolveVideoSource", () => {
it("plays the downloaded file when one exists", () => {
const decision = resolveVideoSource({
localPath: "/home/u/.local/share/jellytau/movie.mp4",
remoteUrl: "https://server/Videos/abc/master.m3u8",
remoteNeedsTranscoding: true,
toAssetUrl,
});
expect(decision.isLocal).toBe(true);
expect(decision.url).toBe(toAssetUrl("/home/u/.local/share/jellytau/movie.mp4"));
});
it("never marks a local file as needing transcoding, even when the remote did", () => {
// The transcoded path re-requests a whole new stream URL on every seek.
// A local file seeks natively; sending it down that route would ask the
// server for a stream we deliberately avoided.
const decision = resolveVideoSource({
localPath: "/downloads/film.mkv",
remoteUrl: "https://server/Videos/abc/master.m3u8",
remoteNeedsTranscoding: true,
toAssetUrl,
});
expect(decision.needsTranscoding).toBe(false);
});
it("streams when nothing is downloaded, preserving the transcoding flag", () => {
const decision = resolveVideoSource({
localPath: null,
remoteUrl: "https://server/Videos/abc/master.m3u8",
remoteNeedsTranscoding: true,
toAssetUrl,
});
expect(decision).toEqual({
url: "https://server/Videos/abc/master.m3u8",
needsTranscoding: true,
isLocal: false,
});
});
it("streams a direct-play remote without claiming it transcodes", () => {
const decision = resolveVideoSource({
localPath: null,
remoteUrl: "https://server/Videos/abc/stream.mp4",
remoteNeedsTranscoding: false,
toAssetUrl,
});
expect(decision.needsTranscoding).toBe(false);
expect(decision.isLocal).toBe(false);
});
it("falls back to streaming for a blank path rather than building a dead asset URL", () => {
for (const localPath of ["", " "]) {
const decision = resolveVideoSource({
localPath,
remoteUrl: "https://server/stream",
remoteNeedsTranscoding: false,
toAssetUrl,
});
expect(decision.isLocal).toBe(false);
expect(decision.url).toBe("https://server/stream");
}
});
});
src/lib/player/localSource.ts
-50
View File
@@ -1,41 +1,3 @@
/**
* Choosing between a downloaded file and a server stream for video playback.
*
* Audio has preferred local files since the queue is built (the Rust queue
* resolves `MediaSource::Local`), but video asks the repository for a stream URL
* and never consults `downloads` — so a downloaded film was streamed anyway,
* spending bandwidth that had already been spent and failing outright offline.
*
* Pure so it can be unit-tested: the component only supplies the two inputs and
* the asset-URL converter.
*
* TRACES: UR-071 | DR-123 | UT-118
*/
export interface VideoSourceInputs {
/** Absolute on-disk path of a completed download, or null to stream. */
localPath: string | null;
/** Stream URL the repository resolved (already transcoded if it had to be). */
remoteUrl: string;
/** Whether the *remote* stream is a transcode. */
remoteNeedsTranscoding: boolean;
/** Usually Tauri's `convertFileSrc`; injected so this module stays pure. */
toAssetUrl: (path: string) => string;
}
export interface VideoSourceDecision {
/** What to hand the `<video>` element. */
url: string;
/**
* Local files are never transcodes, so this is always false for them. It
* matters because the transcoded path re-requests a whole new stream URL on
* every seek; a local file seeks natively and must not go down that route.
*/
needsTranscoding: boolean;
/** True when playing from disk — for logging and the offline badge. */
isLocal: boolean;
}
/** Absolute on POSIX (`/…`), Windows (`C:\…`, `C:/…`) or a UNC share (`\\…`). */
function isAbsolute(path: string): boolean {
return path.startsWith("/") || path.startsWith("\\") || /^[A-Za-z]:[\\/]/.test(path);
@@ -57,15 +19,3 @@ function isAbsolute(path: string): boolean {
export function downloadedFilePath(storageRoot: string, filePath: string): string {
return isAbsolute(filePath) ? filePath : `${storageRoot}/${filePath}`;
}
export function resolveVideoSource(inputs: VideoSourceInputs): VideoSourceDecision {
const { localPath, remoteUrl, remoteNeedsTranscoding, toAssetUrl } = inputs;
// Treat blank/whitespace paths as absent — a malformed `downloads` row must
// not produce an asset URL pointing at nothing.
if (localPath && localPath.trim() !== "") {
return { url: toAssetUrl(localPath), needsTranscoding: false, isLocal: true };
}
return { url: remoteUrl, needsTranscoding: remoteNeedsTranscoding, isLocal: false };
}