public inbox for git-commits@fedoraproject.org
help / color / mirror / Atom feed
From: Jonathan Steffan <jsteffan@fedoraproject.org>
To: git-commits@fedoraproject.org
Subject: [rpms/libsurvive] f44: Enable ix86 builds.
Date: Sun, 02 Aug 2026 01:16:32 GMT	[thread overview]
Message-ID: <178563339224.1.10715379569750312205.rpms-libsurvive-daa06491f5fe@fedoraproject.org> (raw)

A new commit has been pushed.

Repo   : rpms/libsurvive
Branch : f44
Commit : daa06491f5fe872b76a70997a61a36bc0985ef78
Author : Jonathan Steffan <jsteffan@fedoraproject.org>
Date   : 2026-08-01T16:04:42-06:00
Stats  : +138/-3 in 3 file(s)
URL    : https://src.fedoraproject.org/rpms/libsurvive/c/daa06491f5fe872b76a70997a61a36bc0985ef78?branch=f44

Log:
Enable ix86 builds.

---
diff --git a/libsurvive-fopencookie-off64.patch b/libsurvive-fopencookie-off64.patch
new file mode 100644
index 0000000..bf5cd77
--- /dev/null
+++ b/libsurvive-fopencookie-off64.patch
@@ -0,0 +1,50 @@
+From: Jonathan Steffan <jonathansteffan@gmail.com>
+Subject: [PATCH] usbmon: fix fopencookie seek callback signature and semantics
+
+glibc's cookie_seek_function_t is always
+
+    int (*)(void *cookie, __off64_t *pos, int whence)
+
+regardless of _FILE_OFFSET_BITS.  gzip_cookie_seek() declared its position
+argument as z_off_t *, which zlib defines as off_t.  On LP64 targets off_t and
+__off64_t are both long, so the mismatch is invisible; on 32-bit targets such as
+i686, off_t is a 32-bit long while __off64_t is long long, and initialising
+cookie_io_functions_t::seek fails to compile:
+
+  src/driver_usbmon.c:796:99: error: initialization of
+    'int (*)(void *, __off64_t *, int)' {aka 'int (*)(void *, long long int *, int)'}
+    from incompatible pointer type 'int (*)(void *, off_t *, int)'
+    {aka 'int (*)(void *, long int *, int)'} [-Wincompatible-pointer-types]
+
+Use off64_t and gzseek64() so the type matches on every architecture.  The file
+already defines _GNU_SOURCE before any include, which implies
+_LARGEFILE64_SOURCE, so both are available.
+
+While here, follow the fopencookie(3) contract: the seek callback must return 0
+on success and -1 on error, storing the resulting offset in *pos.  The old code
+returned the new offset directly and never wrote back to *pos, so fseek()/
+ftell() on a compressed playback stream misbehaved on all architectures.
+
+gzip_cookie_seek() is also made static to match its siblings; its only user is
+the initialiser immediately below it.
+
+diff --git a/src/driver_usbmon.c b/src/driver_usbmon.c
+--- a/src/driver_usbmon.c
++++ b/src/driver_usbmon.c
+@@ -790,7 +790,14 @@
+ 
+ static ssize_t gzip_cookie_read(void *cookie, char *buf, size_t nbytes) { return gzread((gzFile)cookie, buf, nbytes); }
+ 
+-int gzip_cookie_seek(void *cookie, z_off_t *pos, int __w) { return gzseek((gzFile)cookie, *pos, __w); }
++static int gzip_cookie_seek(void *cookie, off64_t *pos, int whence) {
++	z_off64_t r = gzseek64((gzFile)cookie, (z_off64_t)*pos, whence);
++	if (r < 0)
++		return -1;
++
++	*pos = (off64_t)r;
++	return 0;
++}
+ 
+ cookie_io_functions_t gzip_cookie = {
+ 	.close = gzip_cookie_close, .write = gzip_cookie_write, .read = gzip_cookie_read, .seek = gzip_cookie_seek};
+ 

diff --git a/libsurvive-i686-imu-timecode.patch b/libsurvive-i686-imu-timecode.patch
new file mode 100644
index 0000000..dab3f41
--- /dev/null
+++ b/libsurvive-i686-imu-timecode.patch
@@ -0,0 +1,77 @@
+From: Jonathan Steffan <jonathansteffan@gmail.com>
+Subject: [PATCH] driver_playback: parse IMU timecode as unsigned
+
+parse_and_run_imu() reads the IMU sample's timecode into a plain `int` via
+`sscanf(..., "%d", &timecode, ...)`. Every other timecode field in the
+.rec.gz replay format (light, sweep, sync) is correctly typed as
+survive_timecode (uint32_t) and parsed with %u; only the IMU path uses a
+signed int and %d.
+
+The recorded IMU timecode is a free-running 32-bit hardware counter, so it
+routinely exceeds INT32_MAX (2147483647) for the back half of any recording
+longer than about 45 seconds (2^31 ticks / 48MHz). On x86_64, glibc's %d
+conversion happens to round-trip these large values correctly because its
+internal accumulator is a 64-bit long; the out-of-range value is narrowed
+into the destination int via a bit-preserving truncation that gets
+reinterpreted correctly once passed on as a uint32_t. On i686, `long` is
+32 bits (same width as `int`), so the strtol-style overflow handling inside
+%d parsing saturates at LONG_MAX instead: every IMU timecode above
+INT32_MAX gets clamped to exactly 2147483647.
+
+That freezes SurviveSensorActivations::last_imu at 2147483647 for the rest
+of playback. SurviveSensorActivations_long_timecode_light() uses last_imu
+as the reference clock for a workaround that corrects for a known
+lightcap/IMU FPGA clock desync in multiples of 2^28 ticks
+(src/survive_sensor_activations.c). With last_imu stuck, that correction
+miscomputes and yanks the light timecode backward by 5 * 2^28 ticks
+(~5.58s), which trips
+`assert(tracker->model.t - t < 1)` in survive_kalman_tracker_report_state()
+and aborts the process. This is the cause of the compare-test-index,
+drone, haagh-tracking, stool-test2, tracker-bad-tracking and
+tracker-throw-compare test failures that are otherwise specific to the
+i686 build.
+
+Fix the parse to match every other timecode field: use survive_timecode
+(uint32_t) and %u.
+
+While auditing the surrounding desync-correction code, also switch
+`labs(time_sync_error)` to `llabs()`: time_sync_error is an int64_t, and
+labs() takes a `long`, which truncates it to 32 bits on i686. This did not
+reproduce any test failure on its own (DIV_ROUND_CLOSEST() already
+operates on the untruncated 64-bit value), but it is the same class of
+ILP32-vs-LP64 bug and is worth closing since it's right next to the
+timecode fix.
+
+diff --git a/src/driver_playback.c b/src/driver_playback.c
+--- a/src/driver_playback.c
++++ b/src/driver_playback.c
+@@ -222,7 +222,7 @@ static int parse_and_run_imu(const char *line, SurvivePlaybackData *driver, bo
+ 		return 0;
+
+ 	char dev[10];
+-	int timecode = 0;
++	survive_timecode timecode = 0;
+ 	FLT accelgyro[9] = { 0 };
+ 	int mask;
+ 	int id;
+@@ -231,7 +231,7 @@ static int parse_and_run_imu(const char *line, SurvivePlaybackData *driver, bo
+ 	char i_char = 0;
+
+ 	int rr = sscanf(line,
+-					"%s %c %d %d " FLT_sformat " " FLT_sformat " " FLT_sformat " " FLT_sformat " " FLT_sformat
++					"%s %c %d %u " FLT_sformat " " FLT_sformat " " FLT_sformat " " FLT_sformat " " FLT_sformat
+ 					" " FLT_sformat " " FLT_sformat " " FLT_sformat " " FLT_sformat "%d",
+ 					dev, &i_char, &mask, &timecode, &accelgyro[0], &accelgyro[1], &accelgyro[2], &accelgyro[3],
+ 					&accelgyro[4], &accelgyro[5], &accelgyro[6], &accelgyro[7], &accelgyro[8], &id);
+diff --git a/src/survive_sensor_activations.c b/src/survive_sensor_activations.c
+--- a/src/survive_sensor_activations.c
++++ b/src/survive_sensor_activations.c
+@@ -454,7 +454,7 @@ SURVIVE_EXPORT survive_long_timecode SurviveSensorActivations_long_timecode_li
+ 	use that as a basis. It's worth noting that I've never seen a system develop this while running; it would
+ 	likely cause some chaos if it did since it'd kick the kalman out of sorts.
+ 	***/
+-	if (self->last_imu != 0 && labs(time_sync_error) > 48000000) {
++	if (self->last_imu != 0 && llabs(time_sync_error) > 48000000) {
+ 		int64_t offset = 0x10000000;
+ 		int scale = DIV_ROUND_CLOSEST(time_sync_error, offset);
+ 		initial_time -= offset * scale;

diff --git a/libsurvive.spec b/libsurvive.spec
index 61182be..b894cb2 100644
--- a/libsurvive.spec
+++ b/libsurvive.spec
@@ -53,9 +53,17 @@ Source1:        %{extras_data_url}/archive/%{extras_data_commit}/%{name}-extras-
 Patch:          %{name}-no-external-project.patch
 # Adapt eigen version detection for eigen3-5.0.0
 Patch:          %{name}-eigen3.patch
-
-# Build fails on i686 due to incompatible pointer types
-ExcludeArch:    %{ix86}
+# fopencookie's seek callback is int(void *, __off64_t *, int) on every arch;
+# using z_off_t breaks the i686 build
+# https://github.com/collabora/libsurvive/pull/368
+Patch:          %{name}-fopencookie-off64.patch
+# Replay's IMU timecode is parsed as a signed int with %%d; on i686 (32-bit
+# long) sscanf saturates at INT32_MAX for the back half of any recording,
+# freezing the lightcap/IMU desync correction and failing 6 ctest cases
+# https://github.com/collabora/libsurvive/pull/368
+Patch:          %{name}-i686-imu-timecode.patch
+
+ExclusiveArch:  aarch64 %{ix86} x86_64
 
 BuildRequires:  cmake
 BuildRequires:  blas-devel

                 reply	other threads:[~2026-08-02  1:16 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=178563339224.1.10715379569750312205.rpms-libsurvive-daa06491f5fe@fedoraproject.org \
    --to=jsteffan@fedoraproject.org \
    --cc=git-commits@fedoraproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox