Repository navigation
video: the reader type is defined by the file format, not MATLAB's VideoReader - #143
Merged
Merged
Conversation
…deoReader
The 'video' type reads mp4/avi/mov containers and each frame's stored
presentation time, so a Python implementation (e.g. PyAV) can read the
same files and give the same times; VideoReader is how MATLAB implements
it. The class help, constructor help and frametimes help now say so, the
frame time is defined as the presentation time (in MATLAB, VideoReader's
CurrentTime before each readFrame), and the MATLAB-specific 'videoreader'
alias is removed from ndr_reader_types.json ('video', 'mp4', 'avi' and
'mov' remain). No change to how frames are read. Asked by Steve Van
Hooser in review.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FxKLA8Tbf9cpvDe7fuVeKf
2 of 3 tasks
Contributor
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #143 +/- ##
=======================================
Coverage 54.88% 54.88%
=======================================
Files 164 164
Lines 9598 9598
=======================================
Hits 5268 5268
Misses 4330 4330 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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
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.
Summary
The follow-up to #140 that Steve asked for in its review. #140 was merged at 794a0e8, a minute before this commit was pushed, so the change was not in it.
The
videoreader type is defined by the file format: it reads mp4/avi/mov containers and the time stored for each frame. MATLAB implements it with VideoReader, and a Python reader can use any library that reads the same files (e.g. PyAV). The code already worked that way; this PR fixes what said otherwise.+ndr/+reader/video.m): the class, constructor andframetimeshelp say the type is format-defined and that VideoReader is the MATLAB implementation, rather than "using ONLY MATLAB's built-in VideoReader".CurrentTimeread just before each frame is decoded, as before.videoreaderalias is removed fromresource/ndr_reader_types.json. The type stays registered asvideo,mp4,aviandmov. Nothing in NDI uses the alias (grepof NDI-matlab'ssrc,testsand workflows for'videoreader'/"videoreader": no matches).No code change to how frames are read;
TestVideochecksknown_readerslistsvideo, which is unchanged.Test plan
Test codegreen on d2ebe98: 412 tests, 409 passed, 0 failed (the 3 skipped are the CED sonpipe tests, as onmain).Test NDR SymmetryandTest .smrx(ubuntu, windows) green on d2ebe98; codecov patch and project pass.🤖 Generated with Claude Code
https://claude.ai/code/session_01FxKLA8Tbf9cpvDe7fuVeKf