-
Notifications
You must be signed in to change notification settings - Fork 190
[ENH] Add audio/video recordings to behavioral experiments #2231
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
Add comprehensive support for audio and video recordings in behavioral experiments: - Add audio file extensions (mp3, wav) and video file extensions (mp4, mkv, avi) with corresponding _audio and _video suffixes - Document usage of audio/video recordings in beh directory for capturing vocalizations, speech, facial expressions, and body movements - Add metadata schema for audio/video device information and stream properties - Include privacy warnings about personally identifiable information in human subject recordings - Update behavioral experiments title to remove "with no neural recordings" restriction, clarifying data can be stored with or without neural recordings - Add examples for file organization including multi-angle recordings and split files - Define optional entities: task, acquisition, run, recording, split
…ee macros - Change section title from 'Behavioral experiments' to 'Behavioral recordings' - Convert file tree examples to use MACROS___make_filetree_example for consistent rendering - Address review comments from @yarikoptic in PR #2231
effigies
left a comment
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.
Overall this makes sense to me. It would be good to get some feedback from contributors to related BEPs, such as eye-tracking (20), motion (29), stimuli (44) and physio (45). Even if this PR doesn't propose adding this as an associated file to those data types, the potential is there and it's worth getting opinions and identifying potential conflicts.
cc @bids-standard/bep029 @bids-standard/bep044
cc @mszinte @julia-pfarr @oesteban (BEP020)
cc @m-miedema @smoia @SouravKulkarni (?) (BEP045)
| "sub-01_ses-01_task-freeplay_run-01_video": { | ||
| "split-001.mp4": "", | ||
| "split-002.mp4": "", | ||
| "split-003.mp4": "", | ||
| }, |
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.
This is a substantial change in how much extensions, entities and suffixes can be reordered. I talked with @yarikoptic yesterday and I think this is an interesting future direction, but something that does not require any changes at the moment would be:
| "sub-01_ses-01_task-freeplay_run-01_video": { | |
| "split-001.mp4": "", | |
| "split-002.mp4": "", | |
| "split-003.mp4": "", | |
| }, | |
| "sub-01_ses-01_task-freeplay_run-01_split-001_video.mp4": "", | |
| "sub-01_ses-01_task-freeplay_run-01_split-002_video.mp4": "", | |
| "sub-01_ses-01_task-freeplay_run-01_split-003_video.mp4": "", |
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.
@yarikoptic 's concern here was that if we have hundreds or even thousands of video files this would get cumbersome. @talmo also preferred the subdirectory approach.
From my perspective, my philosophy is to only propose changes to the standard for expressibility or performance and this a large proposed change that seems merely for convenience, not expressibility or performance (correct me if I am wrong), so I am fine with reverting to the standard split organization (i.e. no subdirectory).
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 see I missed this discussion, apologies for jumping in late. I unresolved this so others can see this conversation, but reading through the previous discussion I think there's an 80/20 split here where it's better to work within the current BIDS constraints and then separately push for a generalization to _<suffix>/split-<label>.<ext> if that becomes a practical problem for people.
I would say it's a BEP-level change that would require design for the schema and validator and would significantly slow down this proposal to include it here.
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.
Agreed
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2231 +/- ##
=======================================
Coverage 82.86% 82.86%
=======================================
Files 20 20
Lines 1669 1669
=======================================
Hits 1383 1383
Misses 286 286 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: Chris Markiewicz <[email protected]>
This reverts commit bf00bc6.
…us recordings and timing alignment
fix #1771