Skip to content

fix(video): isolate concurrent remux temp files - #531

Open
harshitethic wants to merge 1 commit into
Hebbian-Robotics:mainfrom
harshitethic:fix/video-cache-unique-remux-temp
Open

fix(video): isolate concurrent remux temp files#531
harshitethic wants to merge 1 commit into
Hebbian-Robotics:mainfrom
harshitethic:fix/video-cache-unique-remux-temp

Conversation

@harshitethic

Copy link
Copy Markdown
Contributor

Summary

  • replace the deterministic <output>.tmp remux path with a unique sibling temporary file per call
  • keep the existing atomic replace() behavior and failure cleanup
  • prevent concurrent remuxes targeting the same cache path from truncating or unlinking each other's ffmpeg output
  • add a regression test that runs two remuxes concurrently, synchronizes them at the subprocess boundary, and asserts they use distinct temp files while producing one valid final cache file

Verification

  • git diff --check
  • Python compile check for the modified video module and concurrency regression test
  • helper workflow removed and branch squashed to one focused commit

Fixes #530

@kstonekuan kstonekuan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ruff check fails on tests/test_video_concurrency.py (missing annotations on tmp_path, monkeypatch, fake_run), so this would turn main red.

The fix is right. The test is weaker than #530 asks for: with subprocess.run mocked it pins that two calls get distinct temp names, which catches a revert to the fixed name but cannot catch what the issue measured. #530 wants the cached final to match a single-process reference by sha256 and the full frame count to extract.

The barrier and the not path.exists() check are the right instincts. Worth saying whether the sha256 version is practical here or not.

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.

video cache: concurrent remuxes share one fixed-name .tmp file, silently poisoning the frame cache with torn MP4s

2 participants