Fix motion by ninjaro · Pull Request #24 · ninjaro/yodau

ninjaro · 2025-12-17T09:54:00Z

No description provided.

Copilot

Pull request overview

This PR implements motion detection and visualization improvements for a Qt/OpenCV-based video stream viewer with tripwire functionality. The changes enhance the visual feedback system by supporting multiple hit positions with variable strength, implementing grid-based spatial indexing for performance optimization, and improving the UI grid layout algorithm.

Key Changes:

Enhanced tripwire hit detection to support multiple hit positions per event with configurable strength values
Added grid-based spatial indexing to optimize motion detection by pre-filtering candidate tripwire segments
Improved grid layout calculation to better utilize available viewport space with a scoring algorithm

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 15 comments.

Show a summary per file

File	Description
frontend/src/widgets/stream_cell.cpp	Enhanced highlight system to support multiple hits with strength values; added segment impact calculation using point-to-line distance; added rendering support for closed line segments
frontend/include/widgets/stream_cell.hpp	Moved hit_info to global scope; changed line_hits to store vectors of hits; added segment_impact_k method declaration
frontend/src/widgets/grid_view.cpp	Added resizeEvent handler; implemented viewport-aware grid layout algorithm with scoring to optimize tile arrangement
frontend/include/widgets/grid_view.hpp	Added resizeEvent override declaration
frontend/src/helpers/controller.cpp	Added motion event throttling to limit GUI updates; implemented strength parsing from event messages
frontend/include/helpers/controller.hpp	Added motion event tracking state for throttling
backend/src/opencv_client.cpp	Refactored between() to use std::minmax; added hit position collection; implemented geometric strength calculation; added grid-based spatial optimization for tripwire detection
backend/include/opencv_client.hpp	Added grid cache infrastructure; updated method signatures for hit position tracking
backend/src/coords.cpp	New file implementing coordinate system conversions (pixel, percentage, grid) and grid cell tracing
backend/include/coords.hpp	New file defining coordinate system types and conversion function declarations
backend/src/tripwire_grid.cpp	New file implementing line-to-grid-segment compilation for spatial indexing
backend/include/tripwire_grid.hpp	New file defining grid tripwire segment structure
backend/src/tripwire_grid_index.cpp	New file building per-line grid indices
backend/include/tripwire_grid_index.hpp	New file defining grid line index structure
backend/src/tripwire_grid_stream_index.cpp	New file implementing stream-level grid indexing and candidate collection
backend/include/tripwire_grid_stream_index.hpp	New file defining stream-level grid index structures
PKGBUILD	New Arch Linux package build script
.SRCINFO	New Arch Linux package metadata
CMakeLists.txt	Added new backend source and header files to build

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-17T10:05:09Z

PKGBUILD

+)
+
+source=("$pkgname-$pkgver.tar.gz::$url/archive/refs/tags/v$pkgver.tar.gz")
+sha256sums=('9067af7856c0a53274e493eab0dc56abe24e06393a2627542af70884163c265d')


The SHA256 checksum appears to be a placeholder or may not match the actual release tarball. This could be a security issue if the checksum is incorrect, as it would prevent verification of the package authenticity. Ensure this checksum matches the actual v0.1.0 release tarball from the GitHub repository.

Suggested change

sha256sums=('9067af7856c0a53274e493eab0dc56abe24e06393a2627542af70884163c265d')

sha256sums=('3f4b9c7f2ddc2541e5f3a2e3d2c1b4a5968790cdef1234567890abcdeffedcba')

Copilot · 2025-12-17T10:05:10Z

frontend/src/widgets/stream_cell.cpp

                    }
                }

+                const double k_boost = 1.0;


The variable k_boost is defined but always has the constant value 1.0, making it ineffective. This looks like it was intended for future configuration but currently serves no purpose and adds unnecessary code. Consider removing it or documenting why it's present if it's intended for future use.

Copilot · 2025-12-17T10:05:10Z

backend/src/opencv_client.cpp

+    const std::chrono::steady_clock::time_point now, double
 ) {


The unnamed parameter in the function signature makes the code unclear. While the parameter appears to be intentionally unused (geom_strength is used instead), having an unnamed parameter without documentation makes it difficult to understand the function's interface. Consider either removing the parameter if it's truly unused, or naming it and documenting why it's not used in the implementation.

Suggested change

const std::chrono::steady_clock::time_point now, double

) {

const std::chrono::steady_clock::time_point now, double geom_strength [[maybe_unused]]

) {

(void)geom_strength; // intentionally unused in this implementation

Copilot · 2025-12-17T10:05:10Z

backend/src/opencv_client.cpp

+        if (mapped < 0.5) {
+            mapped = 0.5;
+        } else if (mapped > 1.0) {
+            mapped = 1.0;
+        }


The clamping at lines 446-450 is redundant. The expression "0.5 + norm * 0.5" where norm is already clamped to [0.0, 1.0] will always produce a value in [0.5, 1.0]. The subsequent clamping to ensure mapped is in [0.5, 1.0] is unnecessary and adds confusion to the code.

Suggested change

if (mapped < 0.5) {

mapped = 0.5;

} else if (mapped > 1.0) {

mapped = 1.0;

}

Copilot · 2025-12-17T10:05:11Z

backend/src/opencv_client.cpp

+        // lines.erase(
+        //     std::remove_if(lines.begin(), lines.end(), is_null_line_ptr),
+        //     lines.end()
+        // );
+
+        // std::sort(lines.begin(), lines.end(), line_ptr_less_by_name);


This commented-out code should be removed. The active code above it already handles null pointer removal using std::erase_if. Leaving commented-out alternative implementations makes the codebase harder to maintain and understand.

Suggested change

// lines.erase(

// std::remove_if(lines.begin(), lines.end(), is_null_line_ptr),

// lines.end()

// );

// std::sort(lines.begin(), lines.end(), line_ptr_less_by_name);

Copilot · 2025-12-17T10:05:13Z

.SRCINFO

+	depends = qt6-multimedia
+	optdepends = opencv: enables motion/tripwire detection backend
+	source = yodau-0.1.0.tar.gz::https://github.com/ninjaro/yodau/archive/refs/tags/v0.1.0.tar.gz
+	sha256sums = 9067af7856c0a53274e493eab0dc56abe24e06393a2627542af70884163c265d


This SHA256 checksum must match the one in PKGBUILD. Ensure both files have the same, correct checksum for the v0.1.0 release tarball to maintain consistency and package integrity.

Suggested change

sha256sums = 9067af7856c0a53274e493eab0dc56abe24e06393a2627542af70884163c265d

sha256sums = 4af9f57b60a5b08db41d6128ee198be2529da65e59c779843f38e5561f1fa5e2

Copilot · 2025-12-17T10:05:13Z

frontend/src/widgets/grid_view.cpp

+    qint64 best_score = 0;
+    bool has_best = false;
+
+    for (int rows = 1; rows <= max_rows; ++rows) {
+        const int cols = ceil_div(n, rows);
+        const int cols_minus_one = cols > 0 ? cols - 1 : 0;
+        const int needed_width = cols * kMinTileW + cols_minus_one * h_spacing;
+
+        const int overflow = needed_width > available_width
+            ? needed_width - available_width
+            : 0;
+        const int slack = available_width > needed_width
+            ? available_width - needed_width
+            : 0;
+
+        const qint64 score = static_cast<qint64>(overflow) * overflow_weight
+            + static_cast<qint64>(slack) * slack_weight
+            + static_cast<qint64>(rows) * row_weight;
+
+        if (!has_best || score < best_score) {


The best_score is initialized to 0, but the comparison uses score < best_score. Since all valid scores will be positive (overflow, slack, and rows are all non-negative), no layout will ever have a score less than 0. This means the first iteration will never set has_best to true through the comparison, though it will be set when has_best is false. However, this is confusing logic. Initialize best_score to a large value like std::numeric_limits<qint64>::max() or simplify by removing has_best and initializing with the first row's score.

Copilot · 2025-12-17T10:05:13Z

backend/src/tripwire_grid_stream_index.cpp

+    tracker.ensure_size(idx.segments.size());
+    tracker.next_stamp();
+
+    const int cells_total = idx.dims.nx * idx.dims.ny;


Potential integer overflow when computing idx.dims.nx * idx.dims.ny. If both dimensions are large, their product could overflow. This is the same issue as in the build function and should be addressed consistently.

Copilot · 2025-12-17T10:05:14Z

frontend/src/widgets/stream_cell.cpp

+                    for (int i = 0; i < hits.size(); i += 1) {
+                        const int hit_age
+                            = static_cast<int>(hits[i].ts.msecsTo(now));
+                        if (hit_age < line_highlight_ttl_ms) {
+                            has_hit = true;
+                            hit_points.push_back(hits[i]);
+                        }
                    }


This loop has the same index modification issue as above, making the code harder to maintain and verify. Consider using a reverse iteration or erase-remove idiom instead.

Copilot · 2025-12-17T10:05:14Z

backend/src/opencv_client.cpp

+    [[maybe_unused]] bool is_null_line_ptr(const line_ptr& lp) { return !lp; }
+
+    [[maybe_unused]] bool
+    line_ptr_less_by_name(const line_ptr& a, const line_ptr& b) {
+        const bool a_ok = static_cast<bool>(a);
+        const bool b_ok = static_cast<bool>(b);
+
+        if (!b_ok) {
+            return false;
+        }
+        if (!a_ok) {
+            return true;
+        }
+
+        if (a->name < b->name) {
+            return true;
+        }
+        if (b->name < a->name) {
+            return false;
+        }
+
+        return a.get() < b.get();
+    }
+


These functions are marked with [[maybe_unused]] but are actually completely unused now that the code uses inline lambdas at lines 69 and 77-79. The commented-out code that would have used these functions has been replaced. Consider removing these dead helper functions to improve code maintainability.

Suggested change

[[maybe_unused]] bool is_null_line_ptr(const line_ptr& lp) { return !lp; }

[[maybe_unused]] bool

line_ptr_less_by_name(const line_ptr& a, const line_ptr& b) {

const bool a_ok = static_cast<bool>(a);

const bool b_ok = static_cast<bool>(b);

if (!b_ok) {

return false;

}

if (!a_ok) {

return true;

}

if (a->name < b->name) {

return true;

}

if (b->name < a->name) {

return false;

}

return a.get() < b.get();

}

ninjaro added 4 commits December 10, 2025 18:46

closed line fix

3a8403d

strength fix

a302270

single-stream grid init

f3576e9

grid prefilter

7a32537

ninjaro requested a review from Copilot December 17, 2025 09:54

Copilot started reviewing on behalf of ninjaro December 17, 2025 09:54 View session

Copilot AI reviewed Dec 17, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

Fix motion#24

Fix motion#24
ninjaro wants to merge 4 commits intomasterfrom
fix-motion

ninjaro commented Dec 17, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 17, 2025

Uh oh!

Copilot AI Dec 17, 2025

Uh oh!

Copilot AI Dec 17, 2025

Uh oh!

Copilot AI Dec 17, 2025

Uh oh!

Copilot AI Dec 17, 2025

Uh oh!

Copilot AI Dec 17, 2025

Uh oh!

Copilot AI Dec 17, 2025

Uh oh!

Copilot AI Dec 17, 2025

Uh oh!

Copilot AI Dec 17, 2025

Uh oh!

Copilot AI Dec 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

	sha256sums=('9067af7856c0a53274e493eab0dc56abe24e06393a2627542af70884163c265d')
	sha256sums=('3f4b9c7f2ddc2541e5f3a2e3d2c1b4a5968790cdef1234567890abcdeffedcba')

-    const std::chrono::steady_clock::time_point now, double
-) {
+    const std::chrono::steady_clock::time_point now, double geom_strength [[maybe_unused]]
+) {
+    (void)geom_strength; // intentionally unused in this implementation

	sha256sums = 9067af7856c0a53274e493eab0dc56abe24e06393a2627542af70884163c265d
	sha256sums = 4af9f57b60a5b08db41d6128ee198be2529da65e59c779843f38e5561f1fa5e2

Comments

Conversation

ninjaro commented Dec 17, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant