Skip to content

fix: report video file initialisation failures - #153

Draft
slophisticated wants to merge 1 commit into
petercorke:mainfrom
slophisticated:fix/videofile-open-errors
Draft

slophisticated wants to merge 1 commit into
petercorke:mainfrom
slophisticated:fix/videofile-open-errors

Conversation

@slophisticated

Copy link
Copy Markdown

Summary

Raise a ValueError naming the video when OpenCV cannot open it or cannot decode its first frame, instead of accessing None.shape. Release the initial metadata-probe capture in finally, including on failure.

Related issue

Addresses only the VideoFile initialisation-error item in #150. The other source cleanup tasks remain separate.

Draft: #150 asks for #149 to land first. Please keep this behind that PR.

Checklist

  • PR title follows Conventional Commits: fix: report video file initialisation failures
  • Full test suite passes locally — not verified; see validation below.
  • Added tests for this change.
  • New test function signatures use modern type hints; production signature unchanged.
  • Added the ValueError case to the reST docstring.

Validation

Windows, Python 3.12.14, OpenCV contrib 5.0.0.93:

  • New regression file: 6 passed. The unmodified backend fails all 5 negative cases and passes the valid-video control.
  • Existing tests/test_image_sources.py -k videofile: 2 passed.
  • Negative cases cover real empty/corrupt files and capture adapters for open failure, failed first read, and a missing first frame. The adapters also check release and that metadata is not consulted after failure.
  • The control writes a tiny MJPG video, checks metadata, and verifies both frames are still iterated in order.
  • Black accepts both changed files; Ruff accepts the new tests; git diff --check passes.

The full suite and OpenCV 4 were not run. Disk-full errors interrupted the development-environment install; the packages required for these focused tests were repaired and the tests rerun successfully. This does not claim a fully provisioned development environment.

AI-assisted implementation and regression tests using OpenAI Codex.

Raise a clear ValueError for an unavailable video or unreadable first frame,
and release the metadata probe on all paths. Add native and adapter
regressions for the VideoFile initialisation-error item in petercorke#150.
@codacy-production

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 8 complexity · 0 duplication

Metric Results
Complexity 8
Duplication 0

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@petercorke

Copy link
Copy Markdown
Owner

Welcome to MVTB, and thank you for this contribution! This is a nice, careful fix: raising a clear ValueError that names the file (instead of the None.shape crash), releasing the probe capture in finally, and the tests covering real empty/corrupt files plus the mocked open/read failures and a valid-video control. Thanks too for the clear validation notes and for flagging that the full suite and OpenCV 4 weren't run; our CI matrix covers those.

You don't need to hold this behind #149: that PR touches EarthView, far from VideoFile, so there's no overlap. When you're ready, please mark this as ready for review so it can be merged once CI is green.

Thanks for picking up an item from #150 — much appreciated.

@petercorke petercorke left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI is green; looks good to me.

This branch has not been deployed

No deployments
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