Implement the videos feature#770
Conversation
- VideoCard now uses the video element to display frame at 1s as thumbnail - Fixed folder_id type mismatch (TEXT instead of INTEGER) in videos table - Added error handling for video thumbnail loading
- Add db_get_favourite_videos() database function - Add /videos/favourites API endpoint - Add fetchFavouriteVideos() frontend API function - Update MyFav page to display both favourite images and videos - Add video player modal for playing favourite videos - Videos show with thumbnail preview, duration badge, and video indicator
WalkthroughAdds end-to-end video support: database table and data layer, FFmpeg-based utilities for metadata and thumbnails, FastAPI endpoints for video retrieval and favourite toggling, a scanner to index files, and frontend gallery, player, API clients, routes, and UI for videos and memories. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Frontend as Browser/App
participant API as FastAPI Backend
participant DB as SQLite
participant FF as ffmpeg/ffprobe
User->>Frontend: Open Videos page
Frontend->>API: GET /videos/
API->>DB: SELECT videos ORDER BY created_at DESC
DB-->>API: video rows (metadata JSON)
API-->>Frontend: GetAllVideosResponse
User->>Frontend: Toggle favourite
Frontend->>API: POST /videos/toggle-favourite {video_id}
API->>DB: UPDATE videos SET isFavourite = NOT isFavourite WHERE id=...
DB-->>API: rows affected
API-->>Frontend: Toggle response
Note over API,FF: Indexing flow (async/scan)
API->>DB: SELECT folders
loop per video file
API->>FF: run ffprobe -> metadata
FF-->>API: metadata
API->>FF: run ffmpeg -> thumbnail
FF-->>API: thumbnail path
end
API->>DB: INSERT OR IGNORE video records (bulk)
DB-->>API: inserted count
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (20)
frontend/src-tauri/tauri.conf.json (1)
61-66: Minor observation: redundancy betweenasset:andhttp://asset.localhost.Both
asset:(protocol handler) andhttp://asset.localhost(development host) appear in themedia-srcandimg-srcdirectives. Depending on how the Tauri asset protocol routes requests, one may be sufficient. This is a minor point and does not affect functionality, but you may want to verify which is actually needed (or if both are intentional for robustness across different scenarios).frontend/src/components/VideoPlayer/NetflixStylePlayer.tsx (1)
131-139:playsInlineis a good add; consider hardening play/fullscreen state syncing.
video.play()can reject (autoplay/policy) and the element can pause/end independently, sosetIsPlaying(!isPlaying)can desync; similarlyisFullscreencan desync when user exits via ESC. Consider wiringonPlay/onPause/onEnded(andfullscreenchange) to set state, and guard progress calc whendurationis falsy to avoidNaN%.backend/app/database/videos.py (2)
12-16: AlignFolderId/folder_idstyping (TEXT vsList[int]).
You defineFolderId = strand storefolder_id TEXT, butdb_get_videos_by_folder_ids()takesList[int]. Consider switching toList[str](orSequence[FolderId]) to match the DB and the folders API.Also applies to: 115-143
64-89:cursor.rowcountafterexecutemany()may be misleading in sqlite3.
Depending on driver behavior,rowcountcan be-1or otherwise unreliable for multi-row ops; if you need accurate inserted counts (esp. withOR IGNORE), consider usingconn.total_changesdeltas or a follow-up query.backend/scan_videos.py (1)
26-40: Align folder id typing end-to-end.
This script passesfolder_idthrough to record prep; ensure the type (strvsint) matches whatvideo_util_prepare_video_records()and the videos table actually store.frontend/src/api/api-functions/memories.ts (1)
5-26: Thin API wrappers look fine; optional: normalize error handling.
If the rest of the app expectsAPIResponseeven on failures, consider wrapping in try/catch and returning{ success: false, error: ... }consistently.frontend/src/pages/VideosPage/Videos.tsx (4)
31-52: Consider null check beforeconvertFileSrccall.On line 48,
convertFileSrc(video.thumbnailPath)is called. WhilehasThumbnailchecksvideo.thumbnailPath.length > 0, ifvideo.thumbnailPathis exactly an empty string,convertFileSrcwill still be called with it. This should be safe sincehasThumbnailguards the render, but for defensive coding, you might want to ensure the path is valid.- <img - src={convertFileSrc(video.thumbnailPath)} + <img + src={convertFileSrc(video.thumbnailPath!)}
160-201: Add keyboard support for closing the modal.The modal lacks keyboard accessibility. Users should be able to press
Escapeto close it, which is standard UX for modals.const VideoPlayerModal = ({ video, onClose }: { video: VideoData; onClose: () => void; }) => { + useEffect(() => { + const handleKeyDown = (e: KeyboardEvent) => { + if (e.key === 'Escape') onClose(); + }; + window.addEventListener('keydown', handleKeyDown); + return () => window.removeEventListener('keydown', handleKeyDown); + }, [onClose]); + return (
228-240: Clarify: This is not an optimistic update.The comment on line 231 says "Update local state" but this happens after the API call succeeds. For a true optimistic update, you'd update state immediately and revert on failure. Consider either:
- Renaming the comment to clarify it's a post-success update, or
- Implementing actual optimistic update with rollback on error.
const handleToggleFavourite = async (videoId: string) => { try { + // Optimistic update + setVideos(prev => + prev.map(v => + v.id === videoId ? { ...v, isFavourite: !v.isFavourite } : v + ) + ); await toggleVideoFavourite(videoId); - // Update local state - setVideos(prev => - prev.map(v => - v.id === videoId ? { ...v, isFavourite: !v.isFavourite } : v - ) - ); } catch (err) { console.error('Failed to toggle favourite:', err); + // Revert on failure + setVideos(prev => + prev.map(v => + v.id === videoId ? { ...v, isFavourite: !v.isFavourite } : v + ) + ); } };
242-250: Consider rendering modal alongside gallery instead of replacing it.The current pattern unmounts the gallery when a video is selected, which may cause unnecessary refetching when the modal closes. Consider rendering the modal as an overlay while keeping the gallery mounted:
- // If a video is selected, show the player - if (selectedVideo) { - return ( - <VideoPlayerModal - video={selectedVideo} - onClose={() => setSelectedVideo(null)} - /> - ); - } - return ( <div className="flex h-full flex-col"> + {selectedVideo && ( + <VideoPlayerModal + video={selectedVideo} + onClose={() => setSelectedVideo(null)} + /> + )} {/* Header */}backend/app/routes/videos.py (2)
1-1: Remove unused importQuery.The
Queryimport from FastAPI is not used anywhere in this file.-from fastapi import APIRouter, HTTPException, status, Query +from fastapi import APIRouter, HTTPException, status
52-68: Extract VideoData construction to a helper to reduce duplication.The same VideoData construction logic appears in
get_all_videos,get_favourite_videos, andget_video. Consider extracting it:+def _build_video_data(video: dict) -> VideoData: + """Build VideoData from database record.""" + return VideoData( + id=video["id"], + path=video["path"], + folder_id=str(video["folder_id"]), + thumbnailPath=video["thumbnailPath"] or "", + duration=video.get("duration"), + width=video.get("width"), + height=video.get("height"), + isFavourite=bool(video.get("isFavourite", False)), + metadata=_parse_video_metadata(video.get("metadata")), + ) + @router.get( "/",Then use
videos.append(_build_video_data(video))in each endpoint.frontend/src/pages/Home/MyFav.tsx (3)
39-141: Consider extracting shared video card component.
FavouriteVideoCardshares substantial logic withVideoCardinVideos.tsx. Consider creating a shared base component in@/components/Video/VideoCard.tsxwith configurable props for the differences (thumbnail handling, badge display, heart state).
245-253: Consider adding user feedback on error.When the toggle fails, the error is only logged to console. Users won't know the operation failed. Consider showing a toast notification or re-fetching the data.
} catch (err) { console.error('Failed to toggle favourite:', err); + // Consider: show toast notification or refetch videos + // toast.error('Failed to update favourite status'); }
255-263: Same modal pattern issue as Videos.tsx.Consider rendering the modal as an overlay alongside the content to prevent unnecessary unmounting/remounting of the favourites lists.
backend/app/schemas/videos.py (2)
2-2: Remove unused importAny.The
Anytype is imported but not used in this file.-from typing import List, Optional, Any +from typing import List, Optional
17-27: Naming convention inconsistency is intentional but could be cleaner.
VideoDatamixes snake_case (folder_id) and camelCase (thumbnailPath,isFavourite). This appears intentional for frontend compatibility. For cleaner Python code while maintaining API compatibility, consider using Pydantic'saliasorserialization_alias:from pydantic import BaseModel, Field class VideoData(BaseModel): id: str path: str folder_id: str thumbnail_path: str = Field(serialization_alias="thumbnailPath") duration: Optional[float] = None width: Optional[int] = None height: Optional[int] = None is_favourite: bool = Field(default=False, serialization_alias="isFavourite") metadata: Optional[VideoMetadata] = NoneThis keeps Python code consistent with snake_case while serializing to camelCase for the frontend.
docs/backend/backend_python/openapi.json (1)
3061-3137: Consider addingcreated_atto theVideoDataschema.Per the PR objectives, the videos table includes a
created_atfield for chronological grouping. This field is not exposed in theVideoDataschema, which may limit frontend sorting/filtering capabilities.backend/app/utils/videos.py (2)
48-114: Movedatetimeimport to top of file.The
datetimeimport on line 67 is inside the function. Moving it to the module level improves readability and follows Python conventions.# Standard library imports import os import uuid import json import subprocess +from datetime import datetime from typing import List, Dict, Any, Tuple, Optional from pathlib import PathThen update line 68:
- from datetime import datetime - metadata["date_created"] = datetime.fromtimestamp(mtime).isoformat() + metadata["date_created"] = datetime.fromtimestamp(mtime).isoformat()
146-149: Avoid bareexceptclause.The bare
except:on line 148 catches all exceptions includingKeyboardInterruptandSystemExit. Use a more specific exception type.- except: - pass + except Exception: + pass
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
backend/app/database/videos.py(1 hunks)backend/app/routes/videos.py(1 hunks)backend/app/schemas/videos.py(1 hunks)backend/app/utils/videos.py(1 hunks)backend/main.py(3 hunks)backend/scan_videos.py(1 hunks)docs/backend/backend_python/openapi.json(6 hunks)frontend/src-tauri/tauri.conf.json(1 hunks)frontend/src/api/api-functions/index.ts(1 hunks)frontend/src/api/api-functions/memories.ts(1 hunks)frontend/src/api/api-functions/videos.ts(1 hunks)frontend/src/api/apiEndpoints.ts(1 hunks)frontend/src/components/VideoPlayer/NetflixStylePlayer.tsx(1 hunks)frontend/src/pages/Home/MyFav.tsx(2 hunks)frontend/src/pages/VideosPage/Videos.tsx(1 hunks)frontend/src/routes/AppRoutes.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
frontend/src/api/api-functions/memories.ts (3)
frontend/src/types/API.ts (1)
APIResponse(1-8)frontend/src/api/axiosConfig.ts (1)
apiClient(5-12)frontend/src/api/apiEndpoints.ts (1)
memoriesEndpoints(41-45)
backend/scan_videos.py (3)
backend/app/database/folders.py (1)
db_get_all_folder_details(397-418)backend/app/utils/videos.py (2)
video_util_get_videos_from_folder(26-45)video_util_prepare_video_records(189-238)backend/app/database/videos.py (1)
db_bulk_insert_videos(64-88)
frontend/src/api/api-functions/videos.ts (4)
backend/app/schemas/videos.py (2)
VideoData(17-27)GetAllVideosResponse(30-34)frontend/src/types/API.ts (1)
APIResponse(1-8)frontend/src/api/axiosConfig.ts (1)
apiClient(5-12)frontend/src/api/apiEndpoints.ts (1)
videosEndpoints(34-39)
backend/app/schemas/videos.py (1)
frontend/src/api/api-functions/videos.ts (2)
VideoData(5-24)GetAllVideosResponse(26-28)
backend/app/routes/videos.py (3)
backend/app/database/videos.py (4)
db_get_all_videos(91-112)db_get_video_by_id(145-169)db_toggle_video_favourite_status(172-193)db_get_favourite_videos(237-259)frontend/src/api/api-functions/videos.ts (2)
VideoData(5-24)GetAllVideosResponse(26-28)backend/app/schemas/videos.py (6)
VideoData(17-27)VideoMetadata(5-14)GetAllVideosResponse(30-34)ToggleVideoFavouriteRequest(37-39)ToggleVideoFavouriteResponse(42-45)ErrorResponse(48-52)
backend/main.py (2)
backend/app/database/videos.py (1)
db_create_videos_table(36-61)backend/app/utils/microservice.py (1)
microservice_util_start_sync_service(41-71)
🔇 Additional comments (23)
frontend/src-tauri/tauri.conf.json (1)
65-65: ✓ CSP correctly updated to serve video assets.The addition of
asset: http://asset.localhostto themedia-srcdirective is correct and necessary for the video feature. The configuration mirrors the existingimg-srcdirective and aligns with the enabledassetProtocolconfiguration (lines 61–64).backend/main.py (1)
22-23: Wiring for videos table + router looks correct.
Table creation runs during lifespan init and the router is registered under/videos.Also applies to: 45-58, 138-139
frontend/src/routes/AppRoutes.tsx (1)
11-14: Route mapping change is fine (ComingSoon → real pages).Also applies to: 20-27
frontend/src/api/api-functions/index.ts (1)
1-8: Additive re-exports look good.frontend/src/api/apiEndpoints.ts (1)
34-45: Endpoints additions look consistent.frontend/src/pages/VideosPage/Videos.tsx (2)
14-23: LGTM! Well-implemented duration formatter.The
formatDurationfunction correctly handles edge cases (falsy values), and properly formats durations with conditional hour display.
53-82: LGTM! Good fallback thumbnail handling.The video element approach for generating thumbnails when no pre-generated thumbnail exists is well-implemented with proper loading states and error handling.
backend/app/routes/videos.py (3)
26-35: LGTM! Robust metadata parsing.The helper gracefully handles None values, JSON strings, and already-parsed dicts with appropriate logging on failures.
136-183: LGTM! Proper error handling with re-raise pattern.The endpoint correctly re-raises
HTTPExceptionto avoid double-wrapping errors, and provides appropriate 404/500 responses.
195-223: Minor TOCTOU consideration, but handled gracefully.There's a time-of-check-to-time-of-use gap between checking video existence (line 197) and toggling (line 208). However, since
db_toggle_video_favourite_statusreturnsrowcount > 0, a concurrent deletion would result insuccess=Falseand an appropriate error. The current design provides better error messages for the common case.frontend/src/api/api-functions/videos.ts (2)
5-24: LGTM! Interface matches backend schema.The
VideoDatainterface correctly mirrors the backend's PydanticVideoDatamodel, including nestedmetadatastructure.
51-57: LGTM! Correct request payload structure.The function correctly sends
{ video_id: videoId }matching the backend'sToggleVideoFavouriteRequestschema.docs/backend/backend_python/openapi.json (7)
1308-1338: LGTM! Well-structured video endpoints.The
/videos/endpoint follows the existing patterns established by the images API, with consistent response structure and appropriate error handling for 500 status.
1340-1370: LGTM! Favourites endpoint is consistent.The
/videos/favouritesendpoint correctly reusesGetAllVideosResponsefor consistency.
1372-1433: LGTM! Single video retrieval endpoint.Proper 404 handling for missing videos and 422 for validation errors.
1435-1495: LGTM! Toggle favourite endpoint follows existing pattern.Consistent with the images toggle-favourite endpoint design.
2057-2083: LGTM! Response schema is well-defined.
GetAllVideosResponsemirrorsGetAllImagesResponsestructure appropriately.
3138-3216: LGTM! VideoMetadata schema is comprehensive.Covers essential video metadata fields including codec info as specified in requirements.
3330-3353: LGTM! Error response schema follows existing patterns.Consistent with other module error responses (
app__schemas__images__ErrorResponse, etc.).backend/app/utils/videos.py (4)
1-17: LGTM! Clean module setup with comprehensive video format support.The supported extensions cover common video formats as specified in the PR objectives.
20-45: LGTM! Folder scanning implementation is robust.Properly handles recursive and non-recursive modes with appropriate error handling.
117-186: LGTM! Thumbnail generation with proper fallbacks.Good use of timeouts on subprocess calls and appropriate error handling for missing ffmpeg.
241-253: LGTM! Duration formatting utility is correct.Properly handles edge cases (None input, hours > 0) and produces readable output.
backend/app/schemas/videos.py
Outdated
| class ErrorResponse(BaseModel): | ||
| """Error response model.""" | ||
| success: bool = False | ||
| error: str | ||
| message: str |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for ErrorResponse pattern in Python files
rg -n "class.*ErrorResponse" --type=py -A 3Repository: AOSSIE-Org/PictoPy
Length of output: 2617
🏁 Script executed:
#!/bin/bash
# Search for error response patterns more broadly
rg -n "error.*response\|Error.*Response" --type=py -i | head -50Repository: AOSSIE-Org/PictoPy
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Check the project structure for shared schemas
find . -name "*.py" -path "*/schemas/*" | head -20Repository: AOSSIE-Org/PictoPy
Length of output: 509
🏁 Script executed:
#!/bin/bash
# Check if there's a central schemas module or shared folder
ls -la backend/app/ | grep -E "schema|shared|common"Repository: AOSSIE-Org/PictoPy
Length of output: 117
🏁 Script executed:
#!/bin/bash
# Check if there's a base or common schemas file
ls -la backend/app/schemas/ | grep -E "base|common|__init__|error"Repository: AOSSIE-Org/PictoPy
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Get full definitions to understand field differences
rg -A 5 "^class ErrorResponse" backend/app/schemas/*.pyRepository: AOSSIE-Org/PictoPy
Length of output: 2272
Move ErrorResponse to a shared schemas module—it is currently duplicated 8 times across backend schema files with inconsistent field definitions.
ErrorResponse is defined in face_clusters.py, user_preferences.py, images.py, test.py, folders.py, facetagging.py, album.py, and videos.py. Variations include:
user_preferences.pyusessuccess: bool(no default), while others usesuccess: bool = Falsefolders.pyandface_clusters.pyuseOptional[str]fields; others use requiredstrfields- Field ordering differs across files
Create a shared backend/app/schemas/common.py (or similar) with a canonical ErrorResponse definition and import it across all modules.
frontend/src/pages/Home/MyFav.tsx
Outdated
| // Video Player Modal | ||
| const VideoPlayerModal = ({ | ||
| video, | ||
| onClose, | ||
| }: { | ||
| video: VideoData; | ||
| onClose: () => void; | ||
| }) => { | ||
| return ( | ||
| <div | ||
| className="fixed inset-0 z-50 flex items-center justify-center bg-black/95" | ||
| onClick={onClose} | ||
| > | ||
| <Button | ||
| variant="ghost" | ||
| size="icon" | ||
| className="absolute top-4 right-4 z-10 text-white hover:bg-white/20" | ||
| onClick={onClose} | ||
| > | ||
| <X className="h-6 w-6" /> | ||
| </Button> | ||
|
|
||
| <div className="mx-4 w-full max-w-5xl" onClick={(e) => e.stopPropagation()}> | ||
| <NetflixStylePlayer | ||
| videoSrc={convertFileSrc(video.path)} | ||
| title={video.metadata?.name || 'Video'} | ||
| description="" | ||
| /> | ||
| <div className="mt-4 text-white"> | ||
| <h2 className="text-xl font-semibold">{video.metadata?.name}</h2> | ||
| <div className="mt-2 flex items-center gap-4 text-sm text-gray-400"> | ||
| {video.duration && <span>{formatDuration(video.duration)}</span>} | ||
| {video.width && video.height && ( | ||
| <span> | ||
| {video.width}x{video.height} | ||
| </span> | ||
| )} | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Extract VideoPlayerModal to a shared component.
This component is duplicated in Videos.tsx. Extract to @/components/Video/VideoPlayerModal.tsx and import in both pages.
🤖 Prompt for AI Agents
In frontend/src/pages/Home/MyFav.tsx around lines 144 to 186, the
VideoPlayerModal component is duplicated in Videos.tsx and should be extracted
to a shared component; create a new file at
frontend/src/components/Video/VideoPlayerModal.tsx that exports the
VideoPlayerModal (with the same props signature: video: VideoData, onClose: ()
=> void), move the JSX and related helper usage there, and import all
dependencies used inside (Button, X icon, NetflixStylePlayer, convertFileSrc,
formatDuration, VideoData type) at the top of the new file; then replace the
local component in both MyFav.tsx and Videos.tsx with an import from
'@/components/Video/VideoPlayerModal' and remove the duplicated definitions so
both pages use the single shared component.
Backend fixes: - Add _serialize_metadata() helper to ensure metadata is always JSON string - db_bulk_insert_videos() now returns actual inserted count (not attempted) - Fix folder_path_to_id type hint from Dict[str, int] to Dict[str, str] - Add _normalize_path() and _find_matching_folder_id() for safe path matching - Use normalized paths with trailing separator to prevent partial matches - Select longest (most specific) matching folder for nested directories - Update scan_videos.py to use actual inserted count in logs Frontend fixes: - Extract formatDuration() to shared utility @/utils/formatDuration.ts - Extract VideoPlayerModal to shared component @/components/Video/VideoPlayerModal.tsx - Fix fetchVideoById() return type from APIResponse to VideoData - Remove duplicate code from Videos.tsx and MyFav.tsx
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (7)
backend/app/utils/videos.py (2)
67-68: Consider moving the datetime import to module level.The
datetimeimport on line 67 is inside the function. While functional, importing at the module level (line 2) would be more conventional.Apply this diff to move the import:
# Standard library imports import os import uuid import json import subprocess +from datetime import datetime from typing import List, Dict, Any, Tuple, OptionalThen update line 68:
mtime = os.path.getmtime(video_path) - from datetime import datetime metadata["date_created"] = datetime.fromtimestamp(mtime).isoformat()
148-149: Consider making the exception handler more specific.The bare
except:at line 148 catches all exceptions during duration probing. While this provides robustness, it could mask unexpected errors. Consider catching specific exceptions like(subprocess.CalledProcessError, ValueError, subprocess.TimeoutExpired).Apply this diff:
if result.returncode == 0 and result.stdout.strip(): duration = float(result.stdout.strip()) - except: + except (subprocess.CalledProcessError, ValueError, subprocess.TimeoutExpired, FileNotFoundError): passfrontend/src/components/Video/VideoPlayerModal.tsx (1)
15-54: Add minimal modal accessibility + Escape-to-close (recommended).
At least addrole="dialog"/aria-modal="true"and an Escape handler; also consider keeping underlying page mounted and rendering this modal as an overlay to preserve scroll position/state.frontend/src/pages/Home/MyFav.tsx (2)
187-195: User-facing failure handling for toggle favourite.
Right now this onlyconsole.errors. Consider reusinguseMutationFeedback/toast so users know the action failed.
198-206: RenderVideoPlayerModalas an overlay instead of early-returning the whole page.
Early return can reset scroll position and discard the mounted DOM tree; prefer{selectedVideo && <VideoPlayerModal .../>}layered over the page content.frontend/src/pages/VideosPage/Videos.tsx (2)
97-108: Harden favourite button withtype="button"(minor).
Avoid accidental form submission if this card ever renders inside a<form>.
198-206: Prefer overlay modal vs early return (preserve scroll/UI state).
Same asMyFav: renderVideoPlayerModalconditionally on top of the gallery rather than replacing the entire page.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
backend/app/database/videos.py(1 hunks)backend/app/utils/videos.py(1 hunks)backend/scan_videos.py(1 hunks)frontend/src/api/api-functions/videos.ts(1 hunks)frontend/src/components/Video/VideoPlayerModal.tsx(1 hunks)frontend/src/pages/Home/MyFav.tsx(3 hunks)frontend/src/pages/VideosPage/Videos.tsx(1 hunks)frontend/src/utils/formatDuration.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
frontend/src/components/Video/VideoPlayerModal.tsx (3)
frontend/src/api/api-functions/videos.ts (1)
VideoData(7-26)frontend/src/components/VideoPlayer/NetflixStylePlayer.tsx (1)
NetflixStylePlayer(21-197)frontend/src/utils/formatDuration.ts (1)
formatDuration(6-15)
backend/scan_videos.py (3)
backend/app/database/folders.py (1)
db_get_all_folder_details(397-418)backend/app/utils/videos.py (2)
video_util_get_videos_from_folder(26-45)video_util_prepare_video_records(223-267)backend/app/database/videos.py (1)
db_bulk_insert_videos(78-123)
frontend/src/pages/Home/MyFav.tsx (5)
frontend/src/api/api-functions/videos.ts (3)
VideoData(7-26)fetchFavouriteVideos(39-44)toggleVideoFavourite(53-59)frontend/src/utils/formatDuration.ts (1)
formatDuration(6-15)frontend/src/components/Media/ChronologicalGallery.tsx (2)
MonthMarker(10-14)ChronologicalGallery(25-187)frontend/src/hooks/useQueryExtension.ts (1)
usePictoQuery(80-108)frontend/src/components/Video/VideoPlayerModal.tsx (1)
VideoPlayerModal(13-54)
frontend/src/api/api-functions/videos.ts (4)
backend/app/schemas/videos.py (2)
VideoData(17-27)GetAllVideosResponse(30-34)frontend/src/types/API.ts (1)
APIResponse(1-8)frontend/src/api/axiosConfig.ts (1)
apiClient(5-12)frontend/src/api/apiEndpoints.ts (1)
videosEndpoints(34-39)
🔇 Additional comments (12)
backend/scan_videos.py (1)
19-63: LGTM! Well-structured scanning implementation.The function correctly uses the returned
inserted_countfromdb_bulk_insert_videos()to track actual insertions vs. duplicates. Error handling is comprehensive with proper logging at each stage, and the per-folder processing approach is appropriate for this use case.backend/app/utils/videos.py (4)
20-45: LGTM! Clean video discovery implementation.The extension-based validation is straightforward, and the folder scanning function provides both recursive and flat options with appropriate error handling.
189-220: LGTM! Robust path matching implementation.The path normalization and folder matching logic correctly handles edge cases like similar paths and nested folders. The use of
os.sepseparator and longest-match selection ensures the most specific folder ID is returned.
223-267: LGTM! Video record preparation is well-designed.The function correctly matches videos to folders, extracts metadata, and generates thumbnails. The design choice to continue with empty
thumbnailPathif thumbnail generation fails (line 255) is appropriate—videos shouldn't be excluded due to thumbnail issues.
270-282: LGTM! Duration formatting helper is correct.The function appropriately formats durations with proper zero-padding and conditional hour display.
backend/app/database/videos.py (5)
19-47: LGTM! Well-designed type definitions and serialization.The
VideoRecordTypedDict with flexible metadata type and the safe_serialize_metadatahelper with fallback handling ensure robust database operations.
50-75: LGTM! Proper table schema with appropriate constraints.The videos table includes necessary foreign key constraints with CASCADE delete, UNIQUE constraint on path, and sensible defaults for
isFavouriteandcreated_at.
78-123: LGTM! Correct bulk insert with accurate count tracking.The function properly serializes metadata before insertion and accurately tracks the number of inserted rows by comparing counts before and after. The
INSERT OR IGNOREstrategy with count-based tracking addresses the duplicate handling requirement.
126-229: LGTM! Safe and well-structured database operations.All queries use parameterized statements to prevent SQL injection. The dynamic placeholder generation for
INclauses is correct, and the toggle operation usingNOTis idiomatic. Error handling with rollback is appropriate.
231-294: LGTM! Complete set of database operations.The delete, existence check, and favourites retrieval functions are all correctly implemented with proper error handling and efficient query patterns.
frontend/src/pages/VideosPage/Videos.tsx (1)
39-75: Thumbnail fallback logic looks solid.
PreferringthumbnailPathand falling back to a first-frame preview covers both indexed and non-indexed thumbnails well.frontend/src/api/api-functions/videos.ts (1)
32-59: Endpoint wiring and return values look consistent.
fetchVideoByIdreturningVideoData(not wrapped) andtoggleVideoFavouritereturningAPIResponsematches the described backend behavior.
Backend: - Create shared ErrorResponse in app/schemas/common.py - Update videos.py schema to import from common module Frontend: - Make APIResponse generic: APIResponse<T = Record<string, unknown>> - Use type alias for GetAllVideosResponse: APIResponse<VideoData[]> - Fix VideoPlayerModal close button to call e.stopPropagation() - Fix formatDuration to handle zero seconds and pad minutes (00:05 not 0:05) - Add error feedback for favourite videos fetch in MyFav.tsx - FavouriteVideoCard now prefers thumbnail image over video preview - Add fallback from thumbnail to video element on image load error
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
frontend/src/types/API.ts (1)
1-6: GenericAPIResponse<T>is a solid improvement for typed payloads.
Consider whetherdatacan benullin practice; if so, model it asdata?: T | null(or enforce omission server-side).frontend/src/api/api-functions/videos.ts (2)
5-24:VideoDatashape looks consistent, but avoid duplicating top-level vsmetadatafields unless intentional.
If API sometimes returnsduration/width/heightonly inmetadata(or vice-versa), consider normalizing to one source to reduce UI ambiguity.
50-58: TightentoggleVideoFavouritereturn typing to match the backend contract.
If the endpoint returns only{ success, message }, considerPromise<APIResponse<void>>(or a dedicatedToggleVideoFavouriteResponse) to prevent consumers from assumingdataexists.backend/app/schemas/videos.py (2)
5-15: Prefer a typed datetime fordate_createdif you control the payload.
Usingdatetime | None(Pydantic will serialize/parse cleanly) reduces format drift vs a free-form string.
48-49: MoveErrorResponseimport to the top unless you’re avoiding a real circular dependency.
It reads as a workaround; if there’s no cycle, keep imports grouped at the top for clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
backend/app/schemas/common.py(1 hunks)backend/app/schemas/videos.py(1 hunks)frontend/src/api/api-functions/videos.ts(1 hunks)frontend/src/components/Video/VideoPlayerModal.tsx(1 hunks)frontend/src/pages/Home/MyFav.tsx(3 hunks)frontend/src/types/API.ts(1 hunks)frontend/src/utils/formatDuration.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/src/utils/formatDuration.ts
- frontend/src/components/Video/VideoPlayerModal.tsx
- frontend/src/pages/Home/MyFav.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
backend/app/schemas/videos.py (2)
frontend/src/api/api-functions/videos.ts (2)
VideoData(5-24)GetAllVideosResponse(27-27)backend/app/schemas/common.py (1)
ErrorResponse(7-12)
🔇 Additional comments (1)
backend/app/schemas/common.py (1)
1-12: CentralizedErrorResponselooks correct and reusable.
Good lightweight model + sensible defaults for error cases.
3 participants
🎬 Feature: Videos Page Implementation
Summary
This PR implements a complete Videos feature for PictoPy, allowing users to browse, play, and favourite videos from their media library. Videos are automatically scanned when folders are added and displayed alongside images in the Favourites page.
Screen.Recording.2025-12-14.at.2.25.17.AM.mov
Features
Architecture
System Overview
Data Flow
Component Structure
Database Schema
Videos Table
API Endpoints
/videos//videos/favourites/videos/{video_id}/videos/toggle-favouriteExample Response
{ "success": true, "message": "Successfully retrieved 16 videos", "data": [ { "id": "uuid-string", "path": "/path/to/video.mp4", "folder_id": "folder-uuid", "thumbnailPath": "", "duration": 120.5, "width": 1920, "height": 1080, "isFavourite": false, "metadata": { "name": "video.mp4", "file_size": 52428800, "codec": "h264", "date_created": "2025-12-14T10:30:00" } } ] }Supported Video Formats
Configuration Changes
Tauri CSP Update
Added
asset:protocol support for media sources to allow video playback:{ "security": { "csp": "... media-src 'self' blob: data: asset: http://asset.localhost; ..." } }Related Issues
Closes #767
Checklist
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.