From 4212517b51d03f1020dbb02b78a8d6db7a0f59f2 Mon Sep 17 00:00:00 2001 From: Alan Liddell Date: Thu, 14 Dec 2023 14:49:10 -0500 Subject: [PATCH 01/19] Create inner `AvailableData` struct whether we have frames or no. --- src/runtime.rs | 45 +++++++++++++++++++++------------------------ 1 file changed, 21 insertions(+), 24 deletions(-) diff --git a/src/runtime.rs b/src/runtime.rs index b0a0717..4b3d20c 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -329,31 +329,28 @@ impl AvailableDataContext { let stream_id = *stream_id; let (beg, end) = inner.map_read(stream_id)?; let nbytes = unsafe { byte_offset_from(beg, end) }; - if nbytes > 0 { - log::trace!( - "[stream {}] ACQUIRED {:p}-{:p}:{} bytes", - stream_id, - beg, - end, - nbytes + + log::trace!( + "[stream {}] ACQUIRED {:p}-{:p}:{} bytes", + stream_id, + beg, + end, + nbytes + ); + *available_data = Some(Python::with_gil(|py| { + Py::new( + py, + AvailableData { + inner: Some(Arc::new(RawAvailableData { + runtime: self.inner.clone(), + beg: NonNull::new(beg).ok_or(anyhow!("Expected non-null buffer"))?, + end: NonNull::new(end).ok_or(anyhow!("Expected non-null buffer"))?, + stream_id, + consumed_bytes: None, + })), + }, ) - }; - if nbytes > 0 { - *available_data = Some(Python::with_gil(|py| { - Py::new( - py, - AvailableData { - inner: Some(Arc::new(RawAvailableData { - runtime: self.inner.clone(), - beg: NonNull::new(beg).ok_or(anyhow!("Expected non-null buffer"))?, - end: NonNull::new(end).ok_or(anyhow!("Expected non-null buffer"))?, - stream_id, - consumed_bytes: None, - })), - }, - ) - })?); - } + })?); return Ok(self.available_data.clone()); } From 00b41fdb20a570ba325c9892482d13a5bc30190f Mon Sep 17 00:00:00 2001 From: Alan Liddell Date: Thu, 14 Dec 2023 14:56:57 -0500 Subject: [PATCH 02/19] Update tests to reflect new AvailableData contract. --- tests/test_basic.py | 76 ++++++++++++++++++++------------------------- tests/test_zarr.py | 3 +- 2 files changed, 35 insertions(+), 44 deletions(-) diff --git a/tests/test_basic.py b/tests/test_basic.py index 09701d8..ce375bf 100644 --- a/tests/test_basic.py +++ b/tests/test_basic.py @@ -96,9 +96,8 @@ def test_repeat_acq(runtime: Runtime): runtime.start() while True: with runtime.get_available_data(0) as a: - if a: - logging.info(f"Got {a.get_frame_count()}") - break + logging.info(f"Got {a.get_frame_count()}") + break if a: assert a.get_frame_count() == 0 assert next(a.frames()) is None @@ -107,12 +106,11 @@ def test_repeat_acq(runtime: Runtime): runtime.start() while True: with runtime.get_available_data(0) as a: - if a: - logging.info(f"Got {a.get_frame_count()}") - break - if a: - assert a.get_frame_count() == 0 - assert next(a.frames()) is None + logging.info(f"Got {a.get_frame_count()}") + break + + assert a.get_frame_count() == 0 + assert next(a.frames()) is None runtime.stop() # TODO: (nclack) assert 1 more acquired frame. stop cancels and waits. @@ -131,7 +129,7 @@ def test_repeat_with_no_stop(runtime: Runtime): # wait for 1 frame while True: with runtime.get_available_data(0) as a: - if a: + if a.get_frame_count() > 0: logging.info(f"Got {a.get_frame_count()} frame") break # acq is still on going here @@ -181,17 +179,16 @@ def took_too_long(): while nframes < p.video[0].max_frame_count and not took_too_long(): clock = time.time() with runtime.get_available_data(0) as a: - if a: - packet = a.get_frame_count() - for f in a.frames(): - logging.info( - f"{f.data().shape} {f.data()[0][0][0][0]} " - + f"{f.metadata()}" - ) - nframes += packet + packet = a.get_frame_count() + for f in a.frames(): logging.info( - f"frame count: {nframes} - frames in packet: {packet}" + f"{f.data().shape} {f.data()[0][0][0][0]} " + + f"{f.metadata()}" ) + nframes += packet + logging.info( + f"frame count: {nframes} - frames in packet: {packet}" + ) elapsed = time.time() - clock sleep(max(0, 0.1 - elapsed)) @@ -231,8 +228,7 @@ def test_change_filename(runtime: Runtime): runtime.start() while nframes < p.video[0].max_frame_count: with runtime.get_available_data(0) as packet: - if packet: - nframes += packet.get_frame_count() + nframes += packet.get_frame_count() logging.info("Stopping") runtime.stop() @@ -257,8 +253,7 @@ def test_write_external_metadata_to_tiff( runtime.start() while nframes < p.video[0].max_frame_count: with runtime.get_available_data(0) as packet: - if packet: - nframes += packet.get_frame_count() + nframes += packet.get_frame_count() runtime.stop() # Check that the written tif has the expected structure @@ -321,20 +316,17 @@ def is_not_done() -> bool: while is_not_done(): if nframes[stream_id] < p.video[stream_id].max_frame_count: with runtime.get_available_data(stream_id) as packet: - if packet: - n = packet.get_frame_count() - for i, frame in enumerate(packet.frames()): - expected_frame_id = nframes[stream_id] + i - assert ( - frame.metadata().frame_id == expected_frame_id - ), ( - "frame id's didn't match " - + f"({frame.metadata().frame_id}" - + f"!={expected_frame_id})" - + f" [stream {stream_id} nframes {nframes}]" - ) - nframes[stream_id] += n - logging.debug(f"NFRAMES {nframes}") + n = packet.get_frame_count() + for i, frame in enumerate(packet.frames()): + expected_frame_id = nframes[stream_id] + i + assert frame.metadata().frame_id == expected_frame_id, ( + "frame id's didn't match " + + f"({frame.metadata().frame_id}" + + f"!={expected_frame_id})" + + f" [stream {stream_id} nframes {nframes}]" + ) + nframes[stream_id] += n + logging.debug(f"NFRAMES {nframes}") stream_id = (stream_id + 1) % 2 logging.info("Stopping") @@ -362,9 +354,9 @@ def test_abort(runtime: Runtime): while True: with runtime.get_available_data(0) as packet: - if packet: - nframes += packet.get_frame_count() - else: + frame_count = packet.get_frame_count() + nframes += frame_count + if frame_count == 0: break logging.debug( @@ -383,7 +375,7 @@ def wait_for_data( elapsed = timedelta() while elapsed < timeout: with runtime.get_available_data(stream_id) as packet: - if packet: + if packet.get_frame_count() > 0: frames = list(packet.frames()) return (len(frames), frames[0].metadata().frame_id) sleep(sleep_duration.total_seconds()) @@ -419,7 +411,7 @@ def test_execute_trigger(runtime: Runtime): # No triggers yet, so no data. with runtime.get_available_data(0) as packet: - assert packet is None + assert packet.get_frame_count() == 0 # Snap a few individual frames for i in range(p.video[0].max_frame_count): diff --git a/tests/test_zarr.py b/tests/test_zarr.py index a17b9b6..b14638b 100644 --- a/tests/test_zarr.py +++ b/tests/test_zarr.py @@ -47,8 +47,7 @@ def test_write_external_metadata_to_zarr( runtime.start() while nframes < p.video[0].max_frame_count: with runtime.get_available_data(0) as packet: - if packet: - nframes += packet.get_frame_count() + nframes += packet.get_frame_count() runtime.stop() assert p.video[0].storage.settings.filename From ffb5bae35eae0338d551805ff7bd36bdf44dcab6 Mon Sep 17 00:00:00 2001 From: Alan Liddell Date: Thu, 14 Dec 2023 15:15:04 -0500 Subject: [PATCH 03/19] Remove Optional<> tags from AvailableDataContext.available_data and AvailableDataContext.__enter__. --- python/acquire/acquire.pyi | 2 +- src/runtime.rs | 14 ++++++-------- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/python/acquire/acquire.pyi b/python/acquire/acquire.pyi index 0311752..8eabd13 100644 --- a/python/acquire/acquire.pyi +++ b/python/acquire/acquire.pyi @@ -14,7 +14,7 @@ from numpy.typing import NDArray @final class AvailableDataContext: - def __enter__(self) -> Optional[AvailableData]: ... + def __enter__(self) -> AvailableData: ... def __exit__( self, exc_type: Any, exc_value: Any, traceback: Any ) -> None: ... diff --git a/src/runtime.rs b/src/runtime.rs index 4b3d20c..23cc459 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -196,7 +196,7 @@ impl Runtime { Ok(AvailableDataContext { inner: self.inner.clone(), stream_id, - available_data: None, + available_data: Python::with_gil(|py| Py::new(py, AvailableData { inner: None }))?, }) } } @@ -315,12 +315,12 @@ impl AvailableData { pub(crate) struct AvailableDataContext { inner: Arc, stream_id: u32, - available_data: Option>, + available_data: Py, } #[pymethods] impl AvailableDataContext { - fn __enter__(&mut self) -> PyResult>> { + fn __enter__(&mut self) -> PyResult> { let AvailableDataContext { inner, stream_id, @@ -337,7 +337,7 @@ impl AvailableDataContext { end, nbytes ); - *available_data = Some(Python::with_gil(|py| { + *available_data = Python::with_gil(|py| { Py::new( py, AvailableData { @@ -350,15 +350,13 @@ impl AvailableDataContext { })), }, ) - })?); + })?; return Ok(self.available_data.clone()); } fn __exit__(&mut self, _exc_type: &PyAny, _exc_value: &PyAny, _traceback: &PyAny) { Python::with_gil(|py| { - if let Some(a) = &self.available_data { - a.as_ref(py).borrow_mut().invalidate() - }; + (&self.available_data).as_ref(py).borrow_mut().invalidate(); }); } } From c7ec8fffa821a407ec346cef6438bef2a3bcd226 Mon Sep 17 00:00:00 2001 From: Alan Liddell Date: Tue, 19 Dec 2023 15:00:29 -0500 Subject: [PATCH 04/19] Update version to 0.3.0-rc1. --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 44c228a..3580714 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "acquire-imaging" authors = ["Nathan Clack "] -version = "0.3.0" +version = "0.3.0-rc1" edition = "2021" [lib] From 3d92722f530c6bbd765433b42d80e96aaa12bdb4 Mon Sep 17 00:00:00 2001 From: Alan Liddell Date: Tue, 19 Dec 2023 15:00:52 -0500 Subject: [PATCH 05/19] Update Zarr driver to v0.1.8. --- drivers.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers.json b/drivers.json index 64c033c..49feaae 100644 --- a/drivers.json +++ b/drivers.json @@ -1,6 +1,6 @@ { "acquire-driver-common": "0.1.6", - "acquire-driver-zarr": "0.1.7", + "acquire-driver-zarr": "0.1.8", "acquire-driver-egrabber": "0.1.5", "acquire-driver-hdcam": "0.1.7", "acquire-driver-spinnaker": "0.1.1" From 773a7cf9aa8434738dede10f2c2452f47485e8e4 Mon Sep 17 00:00:00 2001 From: Alan Liddell Date: Tue, 19 Dec 2023 15:04:51 -0500 Subject: [PATCH 06/19] Update acquire-core-libs and acquire-video-runtime. --- acquire-libs/acquire-core-libs | 2 +- acquire-libs/acquire-video-runtime | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/acquire-libs/acquire-core-libs b/acquire-libs/acquire-core-libs index 60cb52c..f2dd747 160000 --- a/acquire-libs/acquire-core-libs +++ b/acquire-libs/acquire-core-libs @@ -1 +1 @@ -Subproject commit 60cb52cd3e42a557ade1bd3d57f32308392ab8ec +Subproject commit f2dd7475b1e2667e0682baf4034b1a4c07196ac9 diff --git a/acquire-libs/acquire-video-runtime b/acquire-libs/acquire-video-runtime index 8ff1105..ba0fdc4 160000 --- a/acquire-libs/acquire-video-runtime +++ b/acquire-libs/acquire-video-runtime @@ -1 +1 @@ -Subproject commit 8ff1105935ba1f952bdfd436543e0e1b3ea885f6 +Subproject commit ba0fdc4bbb1474ea4cec48016b3440cc46708c4d From e1000aef1102a4c902f225e33b03096edc57882b Mon Sep 17 00:00:00 2001 From: Alan Liddell Date: Tue, 19 Dec 2023 16:26:30 -0500 Subject: [PATCH 07/19] Explicitly set SampleType in test_setup. --- tests/test_basic.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_basic.py b/tests/test_basic.py index ce375bf..abf4310 100644 --- a/tests/test_basic.py +++ b/tests/test_basic.py @@ -162,6 +162,7 @@ def test_setup(runtime: Runtime): assert p.video[0].storage.identifier is not None assert p.video[0].storage.settings.filename == "out.tif" assert p.video[0].max_frame_count == 100 + p.video[0].camera.settings.pixel_type = acquire.SampleType.U8 p.video[0].camera.settings.shape = (192, 108) p = runtime.set_configuration(p) From 031a34796cb52da784c6e97df5fb14a488cc4420 Mon Sep 17 00:00:00 2001 From: Alan Liddell Date: Tue, 19 Dec 2023 16:42:57 -0500 Subject: [PATCH 08/19] Set exposure time and sleep a little bit longer in test_setup. --- tests/test_basic.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_basic.py b/tests/test_basic.py index abf4310..0dd19fc 100644 --- a/tests/test_basic.py +++ b/tests/test_basic.py @@ -163,6 +163,7 @@ def test_setup(runtime: Runtime): assert p.video[0].storage.settings.filename == "out.tif" assert p.video[0].max_frame_count == 100 p.video[0].camera.settings.pixel_type = acquire.SampleType.U8 + p.video[0].camera.settings.exposure_time_us = 2e3 p.video[0].camera.settings.shape = (192, 108) p = runtime.set_configuration(p) @@ -192,7 +193,7 @@ def took_too_long(): ) elapsed = time.time() - clock - sleep(max(0, 0.1 - elapsed)) + sleep(max(0, 0.4 - elapsed)) logging.info("stopping") runtime.stop() From 5beb9722217c54b0ea4904bbf398a848efc739cb Mon Sep 17 00:00:00 2001 From: Alan Liddell Date: Wed, 20 Dec 2023 09:37:45 -0500 Subject: [PATCH 09/19] Don't set exposure time in test_setup. --- tests/test_basic.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_basic.py b/tests/test_basic.py index 0dd19fc..1fe11e7 100644 --- a/tests/test_basic.py +++ b/tests/test_basic.py @@ -163,7 +163,6 @@ def test_setup(runtime: Runtime): assert p.video[0].storage.settings.filename == "out.tif" assert p.video[0].max_frame_count == 100 p.video[0].camera.settings.pixel_type = acquire.SampleType.U8 - p.video[0].camera.settings.exposure_time_us = 2e3 p.video[0].camera.settings.shape = (192, 108) p = runtime.set_configuration(p) From 39fc618553eb5579f77fc2f8086009da8ab8989f Mon Sep 17 00:00:00 2001 From: Alan Liddell Date: Wed, 20 Dec 2023 12:58:43 -0500 Subject: [PATCH 10/19] Remove unused pointers to RawAvailableData. --- src/runtime.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/runtime.rs b/src/runtime.rs index 23cc459..735edee 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -290,7 +290,6 @@ impl AvailableData { VideoFrameIterator { inner: if let Some(frames) = &self.inner { Some(VideoFrameIteratorInner { - store: frames.clone(), cur: Mutex::new(frames.beg), end: frames.end, }) @@ -362,7 +361,6 @@ impl AvailableDataContext { } struct VideoFrameIteratorInner { - store: Arc, cur: Mutex>, end: NonNull, } @@ -375,10 +373,7 @@ impl Iterator for VideoFrameIteratorInner { fn next(&mut self) -> Option { let mut cur = self.cur.lock(); if *cur < self.end { - let out = VideoFrame { - _store: self.store.clone(), - cur: *cur, - }; + let out = VideoFrame { cur: *cur }; let c = cur.as_ptr(); let o = unsafe { (c as *const u8).offset((*c).bytes_of_frame as _) } @@ -498,7 +493,6 @@ impl IntoDimension for capi::ImageShape { #[pyclass] pub(crate) struct VideoFrame { - _store: Arc, cur: NonNull, } From 4f37e92f487a45bdf44eb5f83a8377df57a11ccd Mon Sep 17 00:00:00 2001 From: Alan Liddell Date: Wed, 20 Dec 2023 13:01:22 -0500 Subject: [PATCH 11/19] Make AvailableData.inner an Option. --- src/runtime.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/runtime.rs b/src/runtime.rs index 735edee..5048b41 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -273,7 +273,7 @@ impl Drop for RawAvailableData { #[pyclass] pub(crate) struct AvailableData { - inner: Option>, + inner: Option, } #[pymethods] @@ -340,13 +340,13 @@ impl AvailableDataContext { Py::new( py, AvailableData { - inner: Some(Arc::new(RawAvailableData { + inner: Some(RawAvailableData { runtime: self.inner.clone(), beg: NonNull::new(beg).ok_or(anyhow!("Expected non-null buffer"))?, end: NonNull::new(end).ok_or(anyhow!("Expected non-null buffer"))?, stream_id, consumed_bytes: None, - })), + }), }, ) })?; From cb328e5264f5b9fb21f05f7b855dd246a66054e6 Mon Sep 17 00:00:00 2001 From: Alan Liddell Date: Wed, 20 Dec 2023 13:01:50 -0500 Subject: [PATCH 12/19] Restore `test_basic` settings and sleep time. --- tests/test_basic.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/test_basic.py b/tests/test_basic.py index 1fe11e7..ce375bf 100644 --- a/tests/test_basic.py +++ b/tests/test_basic.py @@ -162,7 +162,6 @@ def test_setup(runtime: Runtime): assert p.video[0].storage.identifier is not None assert p.video[0].storage.settings.filename == "out.tif" assert p.video[0].max_frame_count == 100 - p.video[0].camera.settings.pixel_type = acquire.SampleType.U8 p.video[0].camera.settings.shape = (192, 108) p = runtime.set_configuration(p) @@ -192,7 +191,7 @@ def took_too_long(): ) elapsed = time.time() - clock - sleep(max(0, 0.4 - elapsed)) + sleep(max(0, 0.1 - elapsed)) logging.info("stopping") runtime.stop() From ac086c0cd3f8e0e231df516689cd56da57b748b7 Mon Sep 17 00:00:00 2001 From: Alan Liddell Date: Wed, 3 Jan 2024 12:20:25 -0500 Subject: [PATCH 13/19] Update test_pr.yml --- .github/workflows/test_pr.yml | 94 +++++++++++++++++------------------ 1 file changed, 47 insertions(+), 47 deletions(-) diff --git a/.github/workflows/test_pr.yml b/.github/workflows/test_pr.yml index 7d14f0a..e34c515 100644 --- a/.github/workflows/test_pr.yml +++ b/.github/workflows/test_pr.yml @@ -103,53 +103,53 @@ jobs: - name: Test run: | python -m pytest -k test_dcam - - egrabber: - name: Python ${{ matrix.python }} (eGrabber) - runs-on: - - self-hosted - - egrabber - - VC-151MX-M6H00 - timeout-minutes: 20 - strategy: - fail-fast: false - matrix: - python: [ "3.8", "3.9", "3.10" ] - - permissions: - actions: write - env: - GH_TOKEN: ${{ github.token }} - steps: - - name: Cancel Previous Runs - uses: styfle/cancel-workflow-action@0.11.0 - with: - access_token: ${{ github.token }} - - - uses: actions/checkout@v3 - with: - submodules: true - ref: ${{ github.event.pull_request.head.sha }} - - - name: Get CMake 3.24 - uses: lukka/get-cmake@latest - with: - cmakeVersion: 3.24.3 - - - name: Set up Python ${{ matrix.python }} - uses: actions/setup-python@v4 - with: - python-version: ${{ matrix.python }} - - - name: Install - run: | - pip install --upgrade pip - pip install -e .[testing] - - - name: Test - run: | - python -m pytest -k test_egrabber - + + # TODO (aliddell): uncomment when we get an eGrabber runner up again + # egrabber: + # name: Python ${{ matrix.python }} (eGrabber) + # runs-on: + # - self-hosted + # - egrabber + # - VC-151MX-M6H00 + # timeout-minutes: 20 + # strategy: + # fail-fast: false + # matrix: + # python: [ "3.8", "3.9", "3.10" ] + + # permissions: + # actions: write + # env: + # GH_TOKEN: ${{ github.token }} + # steps: + # - name: Cancel Previous Runs + # uses: styfle/cancel-workflow-action@0.11.0 + # with: + # access_token: ${{ github.token }} + + # - uses: actions/checkout@v3 + # with: + # submodules: true + # ref: ${{ github.event.pull_request.head.sha }} + + # - name: Get CMake 3.24 + # uses: lukka/get-cmake@latest + # with: + # cmakeVersion: 3.24.3 + + # - name: Set up Python ${{ matrix.python }} + # uses: actions/setup-python@v4 + # with: + # python-version: ${{ matrix.python }} + + # - name: Install + # run: | + # pip install --upgrade pip + # pip install -e .[testing] + + # - name: Test + # run: | + # python -m pytest -k test_egrabber spinnaker: name: Python ${{ matrix.python }} (Spinnaker) From 69d6a403eb55fb9aa7b0a501e1b34b329d92cfdd Mon Sep 17 00:00:00 2001 From: Alan Liddell Date: Wed, 3 Jan 2024 14:04:56 -0500 Subject: [PATCH 14/19] Make AvailableData::invalidate private. --- python/acquire/acquire.pyi | 1 - src/runtime.rs | 2 ++ 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/python/acquire/acquire.pyi b/python/acquire/acquire.pyi index 8eabd13..74b1b79 100644 --- a/python/acquire/acquire.pyi +++ b/python/acquire/acquire.pyi @@ -23,7 +23,6 @@ class AvailableDataContext: class AvailableData: def frames(self) -> Iterator[VideoFrame]: ... def get_frame_count(self) -> int: ... - def invalidate(self) -> None: ... def __iter__(self) -> Iterator[VideoFrame]: ... @final diff --git a/src/runtime.rs b/src/runtime.rs index 5048b41..ef29433 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -302,7 +302,9 @@ impl AvailableData { fn __iter__(slf: PyRef<'_, Self>) -> PyResult> { Py::new(slf.py(), slf.frames()) } +} +impl AvailableData { fn invalidate(&mut self) { // Will drop the RawAvailableData and cause Available data to act like // an empty iterator. From 6db217bc5146ef46b43a434b680d2be1a43719e8 Mon Sep 17 00:00:00 2001 From: Alan Liddell Date: Wed, 3 Jan 2024 14:38:57 -0500 Subject: [PATCH 15/19] Update acquire.gui to reflect new AvailableData contract. --- python/acquire/__init__.py | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/python/acquire/__init__.py b/python/acquire/__init__.py index adf6627..9d95d2a 100644 --- a/python/acquire/__init__.py +++ b/python/acquire/__init__.py @@ -190,17 +190,16 @@ def next_frame() -> Optional[npt.NDArray[Any]]: """Get the next frame from the current stream.""" if nframes[stream_id] < p.video[stream_id].max_frame_count: with runtime.get_available_data(stream_id) as packet: - if packet: - n = packet.get_frame_count() - nframes[stream_id] += n - logging.info( - f"[stream {stream_id}] frame count: {nframes}" - ) - f = next(packet.frames()) - logging.debug( - f"stream {stream_id} frame {f.metadata().frame_id}" - ) - return f.data().squeeze().copy() + n = packet.get_frame_count() + nframes[stream_id] += n + logging.info( + f"[stream {stream_id}] frame count: {nframes}" + ) + f = next(packet.frames()) + logging.debug( + f"stream {stream_id} frame {f.metadata().frame_id}" + ) + return f.data().squeeze().copy() return None while is_not_done(): # runtime.get_state()==DeviceState.Running: From ef87a39f7d123f80dcd3b572fc63d05e0fe42001 Mon Sep 17 00:00:00 2001 From: Alan Liddell Date: Wed, 3 Jan 2024 17:11:59 -0500 Subject: [PATCH 16/19] Add a variable indicating whether the store underlying a VideoFrame is valid. --- src/runtime.rs | 36 ++++++++++++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/src/runtime.rs b/src/runtime.rs index ef29433..e21705a 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -5,6 +5,7 @@ use numpy::{ Ix4, ToPyArray, }; use parking_lot::Mutex; +use pyo3::exceptions::PyRuntimeError; use pyo3::prelude::*; use serde::{Deserialize, Serialize}; use std::{ @@ -196,7 +197,15 @@ impl Runtime { Ok(AvailableDataContext { inner: self.inner.clone(), stream_id, - available_data: Python::with_gil(|py| Py::new(py, AvailableData { inner: None }))?, + available_data: Python::with_gil(|py| { + Py::new( + py, + AvailableData { + inner: None, + valid: Arc::new(Mutex::new(false)), + }, + ) + })?, }) } } @@ -274,6 +283,7 @@ impl Drop for RawAvailableData { #[pyclass] pub(crate) struct AvailableData { inner: Option, + valid: Arc>, } #[pymethods] @@ -290,6 +300,7 @@ impl AvailableData { VideoFrameIterator { inner: if let Some(frames) = &self.inner { Some(VideoFrameIteratorInner { + store_is_valid: self.valid.clone(), cur: Mutex::new(frames.beg), end: frames.end, }) @@ -309,6 +320,7 @@ impl AvailableData { // Will drop the RawAvailableData and cause Available data to act like // an empty iterator. self.inner = None; + *self.valid.lock() = false; } } @@ -349,6 +361,7 @@ impl AvailableDataContext { stream_id, consumed_bytes: None, }), + valid: Arc::new(Mutex::new(true)), }, ) })?; @@ -363,6 +376,7 @@ impl AvailableDataContext { } struct VideoFrameIteratorInner { + store_is_valid: Arc>, cur: Mutex>, end: NonNull, } @@ -375,7 +389,10 @@ impl Iterator for VideoFrameIteratorInner { fn next(&mut self) -> Option { let mut cur = self.cur.lock(); if *cur < self.end { - let out = VideoFrame { cur: *cur }; + let out = VideoFrame { + store_is_valid: self.store_is_valid.clone(), + cur: *cur, + }; let c = cur.as_ptr(); let o = unsafe { (c as *const u8).offset((*c).bytes_of_frame as _) } @@ -495,6 +512,7 @@ impl IntoDimension for capi::ImageShape { #[pyclass] pub(crate) struct VideoFrame { + store_is_valid: Arc>, cur: NonNull, } @@ -503,6 +521,11 @@ unsafe impl Send for VideoFrame {} #[pymethods] impl VideoFrame { fn metadata(slf: PyRef<'_, Self>) -> PyResult { + if !*slf.store_is_valid.lock() { + return Err(PyRuntimeError::new_err( + "VideoFrame is not valid outside of context", + )); + } let cur = slf.cur.as_ptr(); let meta = unsafe { VideoFrameMetadata { @@ -513,7 +536,12 @@ impl VideoFrame { Ok(meta) } - fn data<'py>(&self, py: Python<'py>) -> Py { + fn data<'py>(&self, py: Python<'py>) -> PyResult> { + if !*self.store_is_valid.lock() { + return Err(PyRuntimeError::new_err( + "VideoFrame is not valid outside of context", + )); + } let cur = self.cur.as_ptr(); macro_rules! gen_match { @@ -547,7 +575,7 @@ impl VideoFrame { } .unwrap(); - array.to_pyobject(py) + Ok(array.to_pyobject(py)) } } From 6c819728625bc0c3317ab4ed6f993ee5218d4969 Mon Sep 17 00:00:00 2001 From: Alan Liddell Date: Wed, 3 Jan 2024 17:12:26 -0500 Subject: [PATCH 17/19] Test invalidation behavior. --- tests/test_basic.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/test_basic.py b/tests/test_basic.py index ce375bf..8dbe4b8 100644 --- a/tests/test_basic.py +++ b/tests/test_basic.py @@ -593,6 +593,28 @@ def test_storage_capabilities( assert storage.multiscale.is_supported == multiscale +def test_invalidated_frame(runtime: Runtime): + dm = runtime.device_manager() + p = runtime.get_configuration() + p.video[0].camera.identifier = dm.select(DeviceKind.Camera, ".*empty") + p.video[0].storage.identifier = dm.select(DeviceKind.Storage, "trash") + p.video[0].max_frame_count = 1 + runtime.set_configuration(p) + + frame = None + runtime.start() + while frame is None: + with runtime.get_available_data(0) as packet: + if packet.get_frame_count() > 0: + frame = next(packet.frames()) + frame.data() + with pytest.raises(RuntimeError): + frame.metadata() + with pytest.raises(RuntimeError): + frame.data() + + runtime.stop() + # FIXME: (nclack) awkwardness around references (available frames, f) # NOTES: From 8c010266849a14f4ac4272d18b2edd6e1512d216 Mon Sep 17 00:00:00 2001 From: Alan Liddell Date: Wed, 3 Jan 2024 17:30:22 -0500 Subject: [PATCH 18/19] Make black happy / precommit pass. --- tests/test_basic.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_basic.py b/tests/test_basic.py index 8dbe4b8..7e08271 100644 --- a/tests/test_basic.py +++ b/tests/test_basic.py @@ -615,6 +615,7 @@ def test_invalidated_frame(runtime: Runtime): runtime.stop() + # FIXME: (nclack) awkwardness around references (available frames, f) # NOTES: From 8b290763505df3622fa5a3ec297cc2bd027e3b15 Mon Sep 17 00:00:00 2001 From: Alan Liddell Date: Fri, 5 Jan 2024 13:45:38 -0500 Subject: [PATCH 19/19] Goodbye to `store_is_valid` --- src/runtime.rs | 32 ++++++++++++++------------------ 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/src/runtime.rs b/src/runtime.rs index e21705a..664fe4d 100644 --- a/src/runtime.rs +++ b/src/runtime.rs @@ -201,8 +201,7 @@ impl Runtime { Py::new( py, AvailableData { - inner: None, - valid: Arc::new(Mutex::new(false)), + inner: Arc::new(Mutex::new(None)), }, ) })?, @@ -282,14 +281,13 @@ impl Drop for RawAvailableData { #[pyclass] pub(crate) struct AvailableData { - inner: Option, - valid: Arc>, + inner: Arc>>, } #[pymethods] impl AvailableData { fn get_frame_count(&self) -> usize { - if let Some(inner) = &self.inner { + if let Some(inner) = &*self.inner.lock() { inner.get_frame_count() } else { 0 @@ -298,9 +296,9 @@ impl AvailableData { fn frames(&self) -> VideoFrameIterator { VideoFrameIterator { - inner: if let Some(frames) = &self.inner { + inner: if let Some(frames) = &*self.inner.lock() { Some(VideoFrameIteratorInner { - store_is_valid: self.valid.clone(), + store: self.inner.clone(), cur: Mutex::new(frames.beg), end: frames.end, }) @@ -319,8 +317,7 @@ impl AvailableData { fn invalidate(&mut self) { // Will drop the RawAvailableData and cause Available data to act like // an empty iterator. - self.inner = None; - *self.valid.lock() = false; + *self.inner.lock() = None; } } @@ -354,14 +351,13 @@ impl AvailableDataContext { Py::new( py, AvailableData { - inner: Some(RawAvailableData { + inner: Arc::new(Mutex::new(Some(RawAvailableData { runtime: self.inner.clone(), beg: NonNull::new(beg).ok_or(anyhow!("Expected non-null buffer"))?, end: NonNull::new(end).ok_or(anyhow!("Expected non-null buffer"))?, stream_id, consumed_bytes: None, - }), - valid: Arc::new(Mutex::new(true)), + }))), }, ) })?; @@ -376,7 +372,7 @@ impl AvailableDataContext { } struct VideoFrameIteratorInner { - store_is_valid: Arc>, + store: Arc>>, cur: Mutex>, end: NonNull, } @@ -388,9 +384,9 @@ impl Iterator for VideoFrameIteratorInner { fn next(&mut self) -> Option { let mut cur = self.cur.lock(); - if *cur < self.end { + if (*self.store.lock()).is_some() && *cur < self.end { let out = VideoFrame { - store_is_valid: self.store_is_valid.clone(), + _store: self.store.clone(), cur: *cur, }; @@ -512,7 +508,7 @@ impl IntoDimension for capi::ImageShape { #[pyclass] pub(crate) struct VideoFrame { - store_is_valid: Arc>, + _store: Arc>>, cur: NonNull, } @@ -521,7 +517,7 @@ unsafe impl Send for VideoFrame {} #[pymethods] impl VideoFrame { fn metadata(slf: PyRef<'_, Self>) -> PyResult { - if !*slf.store_is_valid.lock() { + if (*slf._store.lock()).is_none() { return Err(PyRuntimeError::new_err( "VideoFrame is not valid outside of context", )); @@ -537,7 +533,7 @@ impl VideoFrame { } fn data<'py>(&self, py: Python<'py>) -> PyResult> { - if !*self.store_is_valid.lock() { + if (*self._store.lock()).is_none() { return Err(PyRuntimeError::new_err( "VideoFrame is not valid outside of context", ));