Video upload #5385
Open
Video upload #5385
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
DEPENDENCY
📊 Test coverage report
|
Docker Image SizesCPU
GPU
XPU
|
# Conflicts: # application/backend/tests/unit/routers/test_media.py
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds video upload functionality to the application, extending the existing image-only media support to handle video files. The changes enable users to upload videos through the API, store them in the dataset, and generate thumbnails from video frames.
Changes:
- Extended the
Mediamodel to support optional width/height and added video-specific fields (fps, frame_count) - Added
VideoFormatenum and video upload handling in the API and service layers - Implemented video metadata extraction and thumbnail generation using OpenCV
- Updated tests and database schema to accommodate video media types
Reviewed changes
Copilot reviewed 26 out of 27 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| application/ui/src/shared/annotator/annotator-provider.component.tsx | Added null coalescing for optional width/height to prevent undefined values in ROI |
| application/ui/src/features/annotator/tools/edit-bounding-box/edit-bounding-box.component.tsx | Added null coalescing for optional width/height in ROI calculation |
| application/ui/src/features/annotator/tools/bounding-box-tool/bounding-box-tool.component.tsx | Added null coalescing for optional width/height in ROI calculation |
| application/ui/src/features/annotator/annotator-canvas/annotator-canvas.tsx | Added null coalescing for optional width/height in size calculation |
| application/ui/src/features/annotator/annotations/annotation-shape/annotation-shape.component.tsx | Added null coalescing for optional width/height in SVG attributes |
| application/ui/src/constants/shared-types.ts | Added DatasetRevisionItem type export |
| application/ui/mocks/mock-media.ts | Added fps and frame_count fields to mock media object |
| application/backend/tests/unit/services/test_datumaro_converter.py | Updated to use ImageFormat instead of MediaFormat and added fps/frame_count fields |
| application/backend/tests/unit/services/test_dataset_service.py | Added test coverage for annotation validation with missing dimensions |
| application/backend/tests/unit/services/data_collect/test_data_collector.py | Updated to use ImageFormat instead of MediaFormat |
| application/backend/tests/unit/routers/test_media.py | Extended tests to cover both image and video media types |
| application/backend/tests/unit/routers/test_dataset_revisions.py | Updated to use ImageFormat instead of MediaFormat |
| application/backend/tests/integration/services/test_media_service.py | Added comprehensive video upload and thumbnail generation tests |
| application/backend/pyproject.toml | Added opencv-python-headless dependency for video processing |
| application/backend/app/utils/images.py | Added explicit resampling method to image resize operation |
| application/backend/app/services/video/video_service.py | Implements video metadata extraction and frame extraction using OpenCV |
| application/backend/app/services/video/init.py | Exports video service functions |
| application/backend/app/services/media_service.py | Added create_video method and video thumbnail generation logic |
| application/backend/app/services/datumaro_converter.py | Added null checks for width/height before processing samples |
| application/backend/app/services/dataset_service.py | Added validation to ensure media has dimensions before validating annotations |
| application/backend/app/services/data_collect/data_collector.py | Updated to use ImageFormat instead of MediaFormat |
| application/backend/app/models/media.py | Added VideoFormat enum and made width/height optional in Media model |
| application/backend/app/db/schema.py | Updated database schema to support optional width/height and video-specific fields |
| application/backend/app/api/schemas/media.py | Updated API schema to support optional width/height and video fields |
| application/backend/app/api/routers/media.py | Added video upload handling and format parsing logic |
| application/backend/app/alembic/versions/1127bd7a7649_schema.py | Updated migration to make width/height optional and add fps/frame_count columns |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
itallix
reviewed
Feb 4, 2026
|
|
||
| DEFAULT_THUMBNAIL_SIZE = 256 | ||
|
|
||
| VIDEO_WRITE_CHUNK_SIZE = 1024 * 1024 * 1024 |
Contributor
There was a problem hiding this comment.
Shall it be 1MB instead of 1GB?
leoll2
reviewed
Feb 5, 2026
leoll2
reviewed
Feb 10, 2026
| id=str(dataset_item.id), | ||
| image=image_path, | ||
| image_info=ImageInfo(width=media.width, height=media.height), | ||
| image_info=ImageInfo(width=media.width, height=media.height), # pyrefly: ignore[bad-argument-type] |
Contributor
There was a problem hiding this comment.
What causes the bad-argument-type error here?
| data=file.file, | ||
| name=name, | ||
| format=format, | ||
| ) |
Contributor
There was a problem hiding this comment.
I suggest to add a comment here, explaining that for videos we don't create dataset items on upload, but lazily on individual frames when they get annotated.
| """ | ||
| Extracts a video metadata | ||
|
|
||
| :param video_path: Video binary file path |
Contributor
There was a problem hiding this comment.
Please use google style docstrings (Args instead of :param:)
Comment on lines
432
to
437
| def _ensure_ffmpeg(): | ||
| """Ensure ffmpeg is available on PATH; skip test if not.""" | ||
| exe = shutil.which("ffmpeg") | ||
| if exe is None: | ||
| pytest.skip("ffmpeg not found in PATH; skipping video related tests") | ||
| return exe |
Contributor
There was a problem hiding this comment.
Tests shouldn't depend on ffmpeg if the application doesn't. You can use OpenCV to generate a dummy video for testing purposes (fxt_video_data).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves #5280
Summary
How to test
Checklist