Use custom player in VideoDetails with deep-link start time

[horatiualmasan]

What has changed and why?

Last of 4 PRs splitting #1649.

Switches VideoDetails to the custom player: instead of seeking after load,
it hands the deep-linked frame's timestamp to VideoPlayer via startTimeS,
and remounts on frame-number change via a composite #key. Aligns the
annotation overlay to the <video> box (not the full player chrome) and
fixes min-h-0 layout so the player fills its card.

How has it been tested?

New VideoDetails unit tests covering the on-mount frame-load branch
(deep-link vs. playback-time) and the deep-link startTimeS handoff. Plus
manual testing.

Did you update CHANGELOG.md?

• Yes
• Not needed (internal change)

Summary by CodeRabbit

• New Features
  • Deep-linked video frames now open at the correct playback position.
  • Video annotations render as an overlay that stays precisely aligned and scaled as the video resizes.
• Bug Fixes
  • Improved video details remounting and layout behavior in both grouped and standard views.
  • Updated frame loading logic to avoid stale frame content when navigating between frames and ensured playback sync is properly stopped when leaving the view.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: eda97628-8100-4e19-ad55-2982599e525f

📥 Commits

Reviewing files that changed from the base of the PR and between 50e2836 and 591e697.

📒 Files selected for processing (1)

• lightly_studio_view/src/routes/datasets/[dataset_id]/[collection_type]/[collection_id]/videos/[sample_id]/+page.svelte

🚧 Files skipped from review as they are similar to previous changes (1)

• lightly_studio_view/src/routes/datasets/[dataset_id]/[collection_type]/[collection_id]/videos/[sample_id]/+page.svelte

---

📝 Walkthrough

Walkthrough

VideoDetails now supports frame-based deep-link playback, positions annotations over the measured video element, and stops frame synchronization on unmount. The video route remounts on frame changes, while tests validate loading and resolved playback start times.

Changes

Video frame navigation

Layer / File(s)	Summary
Route frame-keyed remounting lightly_studio_view/src/routes/datasets/[dataset_id]/[collection_type]/[collection_id]/videos/[sample_id]/+page.svelte	The route derives frameNumber, remounts VideoDetails when the sample or frame changes, and adjusts video container sizing.
Frame loading and video overlay lightly_studio_view/src/lib/components/VideoDetails/VideoDetails.svelte	VideoDetails loads either the initial or deep-linked frame, passes the resolved timestamp to VideoPlayer, observes video geometry, and positions annotations over the video.
Stub capture and playback tests lightly_studio_view/src/lib/components/VideoDetails/VideoDetails.stub.svelte, lightly_studio_view/src/lib/components/VideoDetails/VideoDetails.test.ts	The test stub captures player props and renders children; tests cover initial loading, deep-link resolution, timestamp adjustment, and non-matching frames.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant VideoPage
  participant VideoDetails
  participant useVideoFrames
  participant VideoPlayer
  VideoPage->>VideoDetails: remount for sample_id and frameNumber
  VideoDetails->>useVideoFrames: load initial or deep-linked frame
  useVideoFrames-->>VideoDetails: provide current frame
  VideoDetails->>VideoPlayer: pass startTimeS
  VideoDetails->>VideoDetails: measure video and position annotation overlay

Loading

Possibly related PRs

• lightly-ai/lightly-studio#1661: Adds playback hook behavior that applies startTimeS when video metadata loads.
• lightly-ai/lightly-studio#1662: Introduces the VideoPlayer startTimeS API consumed by these changes.
• lightly-ai/lightly-studio#855: Overlaps with the frame-loading and playback initialization logic in VideoDetails.

Suggested reviewers: leonardorosaa

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: switching VideoDetails to the custom player with deep-link start time handling.
Description check	✅ Passed	The description follows the required template and covers the change, testing, and changelog status with adequate detail.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch horatiu-lig-9807-show-event-bars-on-the-timeline.d

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[horatiualmasan]

/review

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

lightly_studio_view/src/lib/components/VideoDetails/VideoDetails.svelte (1)

117-136: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Scope the ResizeObserver tightly within the effect.

resizeObserver is declared at the component level but is only utilized inside the $effect. Moving its declaration inside the effect prevents potential reference leaks or stale closure issues during cleanup, keeping the component state clean.

♻️ Proposed refactor

-    let resizeObserver: ResizeObserver;
-
     // Align the annotation overlay to the <video> box, not the full player chrome.
     $effect(() => {
         if (!videoEl || !videoFrameContainerEl) return;
 
         const updateOverlaySize = () => {
             if (!videoEl || !videoFrameContainerEl) return;
             const videoRect = videoEl.getBoundingClientRect();
             const containerRect = videoFrameContainerEl.getBoundingClientRect();
             overlayLeft = videoRect.left - containerRect.left;
             overlayTop = videoRect.top - containerRect.top;
             videoWidth = videoRect.width;
             videoHeight = videoRect.height;
         };
         updateOverlaySize();
 
-        resizeObserver = new ResizeObserver(updateOverlaySize);
-        resizeObserver.observe(videoEl);
-        resizeObserver.observe(videoFrameContainerEl);
+        const resizeObserver = new ResizeObserver(updateOverlaySize);
+        resizeObserver.observe(videoEl);
+        resizeObserver.observe(videoFrameContainerEl);
 
         return () => resizeObserver.disconnect();
     });

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lightly_studio_view/src/lib/components/VideoDetails/VideoDetails.svelte`
around lines 117 - 136, Move the ResizeObserver declaration from component scope
into the $effect callback alongside its creation and observation logic, while
preserving the existing updateOverlaySize behavior and cleanup handling.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@lightly_studio_view/src/routes/datasets/`[dataset_id]/[collection_type]/[collection_id]/videos/[sample_id]/+page.svelte:
- Line 19: Update the frameNumber derivation to parse data.frameNumber with an
explicit base-10 radix and convert any NaN result to undefined before passing it
to VideoDetails, preserving undefined for absent values.

---

Nitpick comments:
In `@lightly_studio_view/src/lib/components/VideoDetails/VideoDetails.svelte`:
- Around line 117-136: Move the ResizeObserver declaration from component scope
into the $effect callback alongside its creation and observation logic, while
preserving the existing updateOverlaySize behavior and cleanup handling.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 439cb8d7-7cf3-4c93-86f3-d4a52c08789e

📥 Commits

Reviewing files that changed from the base of the PR and between af13049 and 50e2836.

📒 Files selected for processing (4)

• lightly_studio_view/src/lib/components/VideoDetails/VideoDetails.stub.svelte
• lightly_studio_view/src/lib/components/VideoDetails/VideoDetails.svelte
• lightly_studio_view/src/lib/components/VideoDetails/VideoDetails.test.ts
• lightly_studio_view/src/routes/datasets/[dataset_id]/[collection_type]/[collection_id]/videos/[sample_id]/+page.svelte