Skip to content

feat: allow uploading manually timestamped videos #156

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Nov 28, 2024

Conversation

drake-nominal
Copy link
Contributor

@drake-nominal drake-nominal commented Nov 27, 2024

When dealing with videos that don't match 1:1 with the playback speed / recording speed (e.g., original video was 200fps, slowed down to 30fps for ingest into nominal), we must provide a timestamp manifest when ingesting. This takes the form of a json file, containing a single array, of all of the per-frame timestamps (in nanos since epoch).

Annoyingly, the backend requires this to come in the form of an s3 path-- this PR allows users to input a simple array of timestamps, and have it be uploaded and ingested into nominal.

Part of this PR required me to change the pydocstyle to google style-- we were already using the style-- but I was getting a crazy amount of false-positives on documented methods:

just fix
poetry run ruff check --fix
nominal/core/_multipart.py:57:5: D417 Missing argument descriptions in the docstring for `put_multipart_upload`: `chunk_size`, `f`, `filename`, `max_workers`, `mimetype`, `upload_client`
nominal/core/_multipart.py:140:5: D417 Missing argument descriptions in the docstring for `upload_multipart_io`: `chunk_size`, `f`, `file_type`, `max_workers`, `name`, `upload_client`
nominal/core/_multipart.py:180:5: D417 Missing argument descriptions in the docstring for `upload_multipart_file`: `chunk_size`, `file`, `file_type`, `max_workers`, `upload_client`
Found 3 errors.
error: Recipe `fix-imports` failed on line 37 with exit code 1

@drake-nominal drake-nominal force-pushed the deidukas/allow-timestamped-video-manifest branch from 582587f to b4bbb07 Compare November 27, 2024 19:18
@drake-nominal drake-nominal marked this pull request as ready for review November 27, 2024 19:20
@drake-nominal drake-nominal requested a review from alkasm November 27, 2024 19:21
@drake-nominal drake-nominal merged commit 67f2867 into main Nov 28, 2024
5 checks passed
@drake-nominal drake-nominal deleted the deidukas/allow-timestamped-video-manifest branch November 28, 2024 01:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants