Merge BEP047 audio/video (#2231) onto common media file definitions (#2367)#2
Closed
bendichter wants to merge 60 commits into
Closed
Merge BEP047 audio/video (#2231) onto common media file definitions (#2367)#2bendichter wants to merge 60 commits into
bendichter wants to merge 60 commits into
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
for more information, see https://pre-commit.ci
…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 bids-standard#2231
Co-authored-by: Chris Markiewicz <markiewicz@stanford.edu>
This reverts commit bf00bc6.
…us recordings and timing alignment
- Add new `_audiovideo` suffix for files containing both audio and video streams - Update documentation to distinguish between audio-only, video-only, and combined recordings - Split AudioVideoStreams sidecar table into separate AudioStreams and VideoStreams tables - Add example files and JSON sidecars for audiovideo recordings - Update schema suffixes to include audiovideo definition
…iments Add `_image` suffix for storing still images captured during behavioral experiments in the `beh` directory. Changes include: - Add `.jpg` and `.png` as supported image file extensions - Document use cases: pose estimation training frames, behavioral setup snapshots, and extracted video frames - Update privacy/PII warnings to include images alongside audio/video - Add ImageProperties sidecar table and example files - Update AudioVideoDevice macro to AudioVideoImageDevice
- Add License field to AudioVideoImageDevice sidecar schema - Update documentation to include images in audio/video section headings - Add note explaining licensing considerations for recordings containing identifiable participant data
…decar rules) Introduce shared media file infrastructure for BEP044 (stimuli) and BEP047 (behavioral A/V). Both BEPs need overlapping audio/video/image support, so this extracts the common foundation: - Suffixes: audio, video, audiovideo, image - Extensions: .wav, .mp3, .aac, .ogg, .mp4, .avi, .mkv, .webm, .svg, .webp, .tiff - Metadata: Duration, FrameRate, Width, Height, AudioChannelCount, AudioSampleRate, VideoCodec, AudioCodec, VideoCodecRFC6381, AudioCodecRFC6381 - Sidecar rules (media.yaml): suffix-based rules that auto-apply to any datatype - Appendix (media-files.md): formats, codec identification, privacy, examples Co-Authored-By: Claude Code 2.1.63 / Claude Opus 4.6 <noreply@anthropic.com>
Add spaces between pipes and dashes in all separator rows (e.g., `| --- |` instead of `|---|`) to satisfy the remark-lint table-cell-padding rule. Co-Authored-By: Claude Code 2.1.63 / Claude Opus 4.6 <noreply@anthropic.com>
Replace the newly added `Duration` metadata field with the existing
`RecordingDuration` field, which already has the same semantics
("length of the recording in seconds") and unit. This avoids
introducing a near-duplicate field for media files.
Co-Authored-By: Claude Code 2.1.63 / Claude Opus 4.6 <noreply@anthropic.com>
Add a note in the appendix explaining why AudioSampleRate is used instead of the existing SamplingFrequency: audio-video containers need to distinguish the audio sampling rate from the video frame rate, so the Audio prefix is necessary for multi-stream files. Co-Authored-By: Claude Code 2.1.63 / Claude Opus 4.6 <noreply@anthropic.com>
The existing photo suffix rules use .tif, so document both .tif and .tiff as valid TIFF extensions for image contexts. This ensures consistency when BEPs define file rules for the image suffix. Co-Authored-By: Claude Code 2.1.63 / Claude Opus 4.6 <noreply@anthropic.com>
Add a section explaining that the media file definitions generalize all media in BIDS. The existing photo suffix covers a narrower use case (still images in electrophysiology/microscopy) and predates this framework. A "photo" could equally be a video with narration, an audio description, or a drawing. The media suffixes should be adopted for new datatypes, and a future proposal may deprecate photo in favor of the broader image suffix with migration tooling. Co-Authored-By: Claude Code 2.1.63 / Claude Opus 4.6 <noreply@anthropic.com>
Replace hand-written metadata tables with MACROS___make_sidecar_table() calls that pull field names, requirement levels, types, and descriptions directly from the schema (rules/sidecars/media.yaml + objects/metadata.yaml). This eliminates duplication between the appendix prose and the schema, addressing review feedback from @neuromechanist and @effigies. The suffix applicability is noted in prose above each table since the existing macro does not render a "Suffix" column. Format/extension tables remain as manual markdown since no macro exists for that layout. Co-Authored-By: Claude Code 2.1.63 / Claude Opus 4.6 <noreply@anthropic.com>
Add MACROS___make_suffix_table() call in the introduction to render the audio, video, audiovideo, and image suffix definitions directly from the schema, keeping the appendix in sync with suffixes.yaml. Co-Authored-By: Claude Code 2.1.63 / Claude Opus 4.6 <noreply@anthropic.com>
Add MACROS___make_extension_table() that renders a table of file extensions from the schema (objects/extensions.yaml), with columns for format name, extension (linked to glossary), and description. Replace the 3 hand-written format tables in media-files.md (audio, video, image) with macro calls, eliminating duplication between the appendix prose and extensions.yaml. Other spec files with similar hand-written extension tables (EEG, iEEG, EMG, MEG appendix) can adopt this macro in follow-up PRs. Co-Authored-By: Claude Code 2.1.63 / Claude Opus 4.6 <noreply@anthropic.com>
Test that the macro correctly renders extension information from the schema, including display names, extension values, glossary links, and proper table structure. Follows the same pattern as existing render table tests. Co-Authored-By: Claude Code 2.1.63 / Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Ben Dichter <ben.dichter@gmail.com>
Per PR review discussion: - Width and Height descriptions now explicitly state they correspond to the number of columns and rows in the stored pixel grid as captured, without applying any orientation correction (for example, EXIF Orientation tag). Addresses thread r2989971312 and yarikoptic's follow-up r3349597959. - Add PixelFormat (OPTIONAL) under MediaVideoProperties, using FFmpeg's pix_fmt string. A single value encodes color model, channel count, chroma subsampling, and bit depth, and is auto-extractable via ffprobe. Addresses thread r2989988246 (proposed by @h-mayorquin, refined by @bendichter in r3348715711; OPTIONAL per project convention rather than RECOMMENDED). Co-authored-by: Heberto Mayorquin <h.mayorquin@gmail.com> Co-authored-by: Ben Dichter <ben.dichter@gmail.com> Co-Authored-By: Claude Code 2.1.161 / Claude Opus 4.7 <noreply@anthropic.com>
Following PR review discussion on naming consistency (yarikoptic's analysis in r3349532158 and r3349547943): - Rename FrameRate -> VideoFrameRate to match the existing Video* / Audio* prefix convention in MediaVideoProperties / MediaAudioProperties. - Add VideoFrameCount (RECOMMENDED) under MediaVideoProperties. Required for variable frame rate video where the count cannot be derived from VideoFrameRate * RecordingDuration; useful as an integrity check otherwise. Addresses thread r2989982185. - Rename MediaVisualProperties -> MediaImageProperties and rename Width/Height to ImageWidth/ImageHeight. The Image prefix disambiguates these pixel-grid dimensions from other notions of width/height (for example, physical object sizes in microscopy, field-of-view extents) and aligns with the schema-wide convention of family prefixes for generic terms. Example JSON in the appendix updated accordingly. Co-authored-by: Heberto Mayorquin <h.mayorquin@gmail.com> Co-authored-by: Cody Baker <CodyCBakerPhD@gmail.com> Co-Authored-By: Claude Code 2.1.161 / Claude Opus 4.7 <noreply@anthropic.com>
The pixel format (FFmpeg's pix_fmt) applies equally to single images and video frames: ffprobe reports it for both, and the encoded information (color model, channel count, chroma subsampling, bit depth) is the same concept in either case. Move the field out of MediaVideoProperties into MediaImageProperties (which already covers image, video, and audiovideo), and rename with the Image prefix to match the rest of the group (ImageWidth, ImageHeight). Description broadened from "video stream" to "video frame or image". Co-Authored-By: Claude Code 2.1.161 / Claude Opus 4.7 <noreply@anthropic.com>
Co-authored-by: Yaroslav Halchenko <debian@onerussian.com>
Per PR review discussion on thread r2989972681 (h-mayorquin proposed, @CodyCBakerPhD raised the image-vs-video tension on r3349547943): ImagePixelFormat (FFmpeg pix_fmt) deterministically encodes bit depth for any FFmpeg-readable file, so ImageBitDepth is redundant for video. However: - Common PIL modes (L, RGB, RGBA, P, ...) are implicitly 8-bit-per- channel and do not encode bit depth in the mode name. Image-domain tooling (Pillow, libtiff, PNG library) surfaces bit depth as a first-class integer rather than as part of a pix_fmt string. - For image-only sidecars whose producing tools do not naturally go through FFmpeg, ImagePixelFormat may be absent and bit depth is the only color-precision field available. - An integer is more directly discoverable for the typical researcher than the FFmpeg pix_fmt naming convention. Added as OPTIONAL with an explicit "redundant with ImagePixelFormat when both present; the two MUST agree" note in the description, so the redundancy is acknowledged rather than hidden. Co-authored-by: Heberto Mayorquin <h.mayorquin@gmail.com> Co-authored-by: Cody Baker <CodyCBakerPhD@gmail.com> Co-Authored-By: Claude Code 2.1.161 / Claude Opus 4.7 <noreply@anthropic.com>
…d#2367 (common media file definitions) This branch builds off PR bids-standard#2367 ("Add common media file definitions for BEP044/BEP047") and layers PR bids-standard#2231 ("BEP047 - Add audio/video recordings to behavioral experiments") on top. Conflict resolutions (preferred bids-standard#2367's canonical media definitions for all shared objects, kept bids-standard#2231's unique behavioral additions): - extensions.yaml: kept bids-standard#2367 generic descriptions for mkv/mp3/mp4/ogg/wav/avi; dropped bids-standard#2231 duplicate avi/wav entries. - suffixes.yaml: kept bids-standard#2367 generic audio/audiovideo/image/video suffixes; dropped bids-standard#2231 duplicate behavioral-specific versions. NOTE: bids-standard#2367 defines 'video' as video-only (no audio); bids-standard#2231 had treated 'video' as possibly containing audio. Resolved in favor of bids-standard#2367. - metadata.yaml: dropped bids-standard#2231 duplicate AudioChannelCount/AudioSampleRate (kept bids-standard#2367's richer versions); retained bids-standard#2231-only fields (AudioDuration, AudioBitDepth, CameraPosition, Duration, FrameRate, Height, Width). - behavioral-experiments.md: kept bids-standard#2367's updated examples-page URL. NOTE: bids-standard#2231 uses generic metadata names (FrameRate/Width/Height/Duration) while bids-standard#2367 uses prefixed names (VideoFrameRate/VideoWidth/...). Both sets coexist after this merge and should be reconciled by maintainers. Schema loads cleanly; no duplicate keys remain. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
Owner
Author
|
Superseded: retargeting onto PR bids-standard#2367's mediafiles branch instead. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
Draft combining two upstream PRs into a single branch that builds off PR bids-standard#2367 and layers PR bids-standard#2231 on top:
Conflict resolution
Preferred bids-standard#2367's canonical media definitions for all shared schema objects; kept bids-standard#2231's unique behavioral additions.
mkv/mp3/mp4/ogg/wav/avi; dropped [ENH] BEP047 - Add audio/video recordings to behavioral experiments bids-standard/bids-specification#2231 duplicateavi/wav.audio/audiovideo/image/video; dropped [ENH] BEP047 - Add audio/video recordings to behavioral experiments bids-standard/bids-specification#2231 behavioral-specific duplicates.AudioChannelCount/AudioSampleRate(kept Add common media file definitions for BEP044/BEP047 bids-standard/bids-specification#2367's richer versions); retained [ENH] BEP047 - Add audio/video recordings to behavioral experiments bids-standard/bids-specification#2231-only fields (AudioDuration,AudioBitDepth,CameraPosition,Duration,FrameRate,Height,Width).videosuffix semantics — Add common media file definitions for BEP044/BEP047 bids-standard/bids-specification#2367 definesvideoas video-only (no audio); [ENH] BEP047 - Add audio/video recordings to behavioral experiments bids-standard/bids-specification#2231 had treated it as possibly containing audio. Resolved in favor of Add common media file definitions for BEP044/BEP047 bids-standard/bids-specification#2367.FrameRate/Width/Height/Duration); Add common media file definitions for BEP044/BEP047 bids-standard/bids-specification#2367 uses prefixed names (VideoFrameRate/VideoWidth/…). Both currently coexist and should be reconciled.Validation
bidsschematools(v1.3.0-dev)🤖 Generated with Claude Code