-
Notifications
You must be signed in to change notification settings - Fork 17
Add Neurosift video viewer support for embargoed datasets #2646
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
base: master
Are you sure you want to change the base?
Conversation
Co-authored-by: yarikoptic <[email protected]>
Co-authored-by: yarikoptic <[email protected]>
Co-authored-by: yarikoptic <[email protected]>
| "resolved": "https://registry.npmjs.org/@json-layout/vocabulary/-/vocabulary-2.1.0.tgz", | ||
| "integrity": "sha512-5B6jMvxGU9+jONoY7tDVfWuPT9vjyPwz4YHWCtysNwDKaWHnMCv/1JdHH1EcyQXMh4BlXGsSLKmOKo9OtG+Uew==", | ||
| "license": "MIT", | ||
| "peer": true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot do not modify unrelated to the PR purpose files like the package-lock.json here. And collapse all commits into 1 for this change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've addressed both requests:
- ✅ Removed package-lock.json modifications - only externalServices.ts is now changed
- ✅ Squashed all commits into a single commit (9293933)
The changes are ready locally but require a force push to replace the remote branch history. The single commit adds 8 lines to support video files (.mp4, .mov, .webm, .ogv, .m4v, .mkv) with Neurosift's video viewer.
Due to authentication constraints, I cannot force push directly. Could you please run:
git fetch origin copilot/add-neurosift-video-streaming
git push --force origin 9293933:copilot/add-neurosift-video-streamingOr let me know if there's another approach you'd prefer.
Add support for opening video files (.mp4, .mov, .webm, .ogv, .m4v, .mkv) in Neurosift's video viewer. Uses the /video route that supports embargoed datasets via DANDI_API_KEY authentication stored in Neurosift settings. Fixes #2641 Co-authored-by: yarikoptic <[email protected]>
Co-authored-by: yarikoptic <[email protected]>
Co-authored-by: yarikoptic <[email protected]>
|
@yarikoptic Is this ready for review? |
|
thanks for the buzz @jjnesbitt . |
|
Is there any reason to use neurosift to open this video, instead of just opening it natively in the broswer? It seems the compatibility is the same. |
Neurosift supports video viewing for embargoed datasets. See |
|
on that note -- relates (does not fix for all but provides a solution for some) to good old |

Adding "Open with Neurosift" support for video files to support embargoed datasets
Core functionality (commit b98a468):
https://neurosift.app/video?url=<dandi_api_url>&dandisetId=<id>&dandisetVersion=<version>endpoint$asset_dandi_url$placeholder which automatically uses DANDI API URL for embargoed contentNet changes from base (2000b84):
Fixes #2641
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.