Merge pull request #41 from GyeongHoKim/fix/rpi

fix/rpi

Author: GyeongHoKim

.goreleaser.yml

@@ -41,7 +41,7 @@ builds:
       - "7"
     hooks:
       pre:
-        - cmd: ./scripts/fetch-mtxrpicam.sh {{ if eq .Arch "arm" }}32{{ else }}64{{ end }} v2.4.3 internal/rpicamera/mtxrpicam_{{ if eq .Arch "arm" }}32{{ else }}64{{ end }}
+        - cmd: ./scripts/fetch-mtxrpicam.sh {{ if eq .Arch "arm" }}32{{ else }}64{{ end }} v2.5.6 internal/rpicamera/mtxrpicam_{{ if eq .Arch "arm" }}32{{ else }}64{{ end }}
     ldflags:
       - -s -w
       - -X github.com/GyeongHoKim/onvif-simulator/internal/version.Version={{.Version}}

Makefile

@@ -20,11 +20,11 @@ FRONTEND_DIST := internal/gui/frontend/dist
 
 # Pinned mediamtx-rpicamera release tag. mediamtx-rpicamera lives in its own
 # repo (bluenviron/mediamtx-rpicamera) with versioning independent from
-# mediamtx itself; mediamtx v1.13.1 internally pins this same v2.4.3 (see
+# mediamtx itself; mediamtx v1.18.1 internally pins this same v2.5.6 (see
 # its internal/staticsources/rpicamera/mtxrpicamdownloader/VERSION). Bump
 # together with scripts/mtxrpicam.sha256 so the Pi build channel ships a
 # reviewable, reproducible binary blob.
-MTXRPICAM_VERSION ?= v2.4.3
+MTXRPICAM_VERSION ?= v2.5.6
 RPICAM_DIR_32     := internal/rpicamera/mtxrpicam_32
 RPICAM_DIR_64     := internal/rpicamera/mtxrpicam_64
 

internal/rpicamera/camera_rpicam.go

@@ -3,7 +3,9 @@
 package rpicamera
 
 import (
+	"bufio"
 	"debug/elf"
+	"errors"
 	"fmt"
 	"io"
 	"log/slog"
@@ -23,6 +25,17 @@ import (
 	"github.com/GyeongHoKim/onvif-simulator/internal/obs"
 )
 
+// stderrTailLines bounds the post-mortem snapshot we keep of helper stderr.
+// libcamera prints a few dozen lines of init context plus any error detail;
+// 64 lines is enough to capture the tail that explains a STREAMON / IPA /
+// pipeline failure without unbounded memory if the helper goes haywire.
+const stderrTailLines = 64
+
+// errTerminated is the sentinel runInner returns on a graceful Close(). It
+// lets run() suppress the post-mortem stderr WARN on normal shutdown without
+// muddling string comparison.
+var errTerminated = errors.New("rpicamera: terminated")
+
 const (
 	libraryToCheckArchitecture = "libc.so.6"
 	dumpPrefix                 = "/dev/shm/onvif-simulator-rpicamera-"
@@ -51,10 +64,13 @@ type Camera struct {
 	logger *slog.Logger
 	onData OnDataFunc
 
-	cmd      *exec.Cmd
-	pipeOut  *pipe
-	pipeIn   *pipe
-	finalErr error
+	cmd        *exec.Cmd
+	pipeOut    *pipe
+	pipeIn     *pipe
+	stderrPipe io.ReadCloser
+	stderrDone chan struct{}
+	tail       *stderrTail
+	finalErr   error
 
 	terminate chan struct{}
 	done      chan struct{}
@@ -72,7 +88,7 @@ func Open(p Params, logger *slog.Logger, onData OnDataFunc) (*Camera, error) {
 		logger = obs.Discard()
 	}
 
-	c := &Camera{logger: logger, onData: onData}
+	c := &Camera{logger: logger, onData: onData, tail: newStderrTail(stderrTailLines)}
 
 	if err := dumpComponent(); err != nil {
 		return nil, err
@@ -99,11 +115,20 @@ func Open(p Params, logger *slog.Logger, onData OnDataFunc) (*Camera, error) {
 	}
 
 	c.cmd = exec.Command(filepath.Join(dumpPath, executableName)) //nolint:gosec
-	// Discard helper output so it does not leak into the simulator's stdout
-	// (reserved for user-facing CLI output) or stderr. Helper-side errors
-	// reach us via the 'e' control byte on the read pipe.
+	// Discard helper stdout so it never leaks into the simulator's user-facing
+	// stdout. Stderr is captured into a ring buffer (and forwarded to the
+	// simulator logger at DEBUG); on terminal failure we flush the tail at
+	// WARN so the operator can see libcamera/V4L2 detail behind the 'e'
+	// control byte (e.g. why VIDIOC_STREAMON failed).
 	c.cmd.Stdout = io.Discard
-	c.cmd.Stderr = io.Discard
+	stderrPipe, err := c.cmd.StderrPipe()
+	if err != nil {
+		c.pipeOut.close()
+		c.pipeIn.close()
+		freeComponent()
+		return nil, fmt.Errorf("rpicamera: stderr pipe: %w", err)
+	}
+	c.stderrPipe = stderrPipe
 	c.cmd.Env = env
 	c.cmd.Dir = dumpPath
 	// Detach the subprocess from the parent's process group so SIGINT/SIGTERM
@@ -112,12 +137,16 @@ func Open(p Params, logger *slog.Logger, onData OnDataFunc) (*Camera, error) {
 	c.cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
 
 	if err := c.cmd.Start(); err != nil {
+		_ = c.stderrPipe.Close()
 		c.pipeOut.close()
 		c.pipeIn.close()
 		freeComponent()
 		return nil, err
 	}
 
+	c.stderrDone = make(chan struct{})
+	go c.drainStderr()
+
 	c.terminate = make(chan struct{})
 	c.done = make(chan struct{})
 
@@ -159,6 +188,9 @@ func (c *Camera) ReloadParams(p Params) error {
 func (c *Camera) run() {
 	defer close(c.done)
 	c.finalErr = c.runInner()
+	if c.finalErr != nil && !errors.Is(c.finalErr, errTerminated) {
+		c.flushStderrTail(c.finalErr)
+	}
 }
 
 func (c *Camera) runInner() error {
@@ -178,13 +210,15 @@ func (c *Camera) runInner() error {
 			c.pipeIn.close()
 			c.pipeOut.close()
 			<-readDone
+			<-c.stderrDone
 			return err
 
 		case err := <-readDone:
 			_ = c.pipeOut.write([]byte{'e'})
 			<-cmdDone
 			c.pipeIn.close()
 			c.pipeOut.close()
+			<-c.stderrDone
 			return err
 
 		case <-c.terminate:
@@ -193,11 +227,54 @@ func (c *Camera) runInner() error {
 			c.pipeOut.close()
 			c.pipeIn.close()
 			<-readDone
-			return fmt.Errorf("rpicamera: terminated")
+			<-c.stderrDone
+			return errTerminated
 		}
 	}
 }
 
+// drainStderr reads helper stderr line-by-line, mirrors each into the
+// simulator log at DEBUG, and keeps the most recent lines in tail so a
+// terminal error can attach the relevant libcamera/V4L2 detail at WARN.
+//
+// A scanner failure (bufio.ErrTooLong on a >256 KiB line, or a read error
+// from the pipe) is itself diagnostic context — record it into the tail
+// and log it at WARN so the post-mortem flush carries the reason the
+// stderr stream stopped early.
+func (c *Camera) drainStderr() {
+	defer close(c.stderrDone)
+	scanner := bufio.NewScanner(c.stderrPipe)
+	// libcamera lines occasionally exceed the default 64 KiB token cap on
+	// verbose IPA banners. 256 KiB is overkill but cheap.
+	scanner.Buffer(make([]byte, 64*1024), 256*1024)
+	for scanner.Scan() {
+		line := scanner.Text()
+		c.tail.push(line)
+		c.logger.Debug("rpicamera: helper stderr", "line", line)
+	}
+	if err := scanner.Err(); err != nil {
+		c.tail.push(fmt.Sprintf("<stderr scanner error: %s>", err))
+		c.logger.Warn("rpicamera: helper stderr scanner stopped", "err", err)
+	}
+}
+
+// flushStderrTail emits the captured stderr ring at WARN when the camera is
+// torn down with a non-nil error. Operator-visible: this is the load-bearing
+// diagnostic for "encoder_create()", "pipeline_handler" and similar helper
+// failures whose root cause libcamera prints to stderr but mtxrpicam relays
+// only as a short string over the 'e' control byte.
+func (c *Camera) flushStderrTail(cause error) {
+	lines := c.tail.snapshot()
+	if len(lines) == 0 {
+		return
+	}
+	c.logger.Warn(
+		"rpicamera: helper stderr tail",
+		"err", cause.Error(),
+		"lines", lines,
+	)
+}
+
 func (c *Camera) runReader() error {
 	// Wait for the helper's "ready" sentinel before forwarding access units.
 	for {

internal/rpicamera/stderrtail.go

@@ -0,0 +1,44 @@
+package rpicamera
+
+import "sync"
+
+// stderrTail keeps the most recent N lines emitted by the mtxrpicam helper on
+// its stderr stream. It is a ring buffer so a noisy helper cannot grow the
+// simulator's memory footprint, and Snapshot returns lines in arrival order so
+// a post-mortem WARN reads top-down like a tail of the helper log.
+type stderrTail struct {
+	mu    sync.Mutex
+	cap   int
+	buf   []string
+	start int
+	size  int
+}
+
+func newStderrTail(capacity int) *stderrTail {
+	if capacity <= 0 {
+		capacity = 1
+	}
+	return &stderrTail{cap: capacity, buf: make([]string, capacity)}
+}
+
+func (t *stderrTail) push(line string) {
+	t.mu.Lock()
+	defer t.mu.Unlock()
+	if t.size < t.cap {
+		t.buf[(t.start+t.size)%t.cap] = line
+		t.size++
+		return
+	}
+	t.buf[t.start] = line
+	t.start = (t.start + 1) % t.cap
+}
+
+func (t *stderrTail) snapshot() []string {
+	t.mu.Lock()
+	defer t.mu.Unlock()
+	out := make([]string, t.size)
+	for i := range t.size {
+		out[i] = t.buf[(t.start+i)%t.cap]
+	}
+	return out
+}

internal/rpicamera/stderrtail_test.go

@@ -0,0 +1,49 @@
+package rpicamera
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestStderrTailBelowCapacity(t *testing.T) {
+	t.Parallel()
+	tail := newStderrTail(4)
+	tail.push("a")
+	tail.push("b")
+	if got, want := tail.snapshot(), []string{"a", "b"}; !reflect.DeepEqual(got, want) {
+		t.Fatalf("snapshot=%v want=%v", got, want)
+	}
+}
+
+func TestStderrTailWrapsAroundCapacity(t *testing.T) {
+	t.Parallel()
+	tail := newStderrTail(3)
+	for _, line := range []string{"a", "b", "c", "d", "e"} {
+		tail.push(line)
+	}
+	if got, want := tail.snapshot(), []string{"c", "d", "e"}; !reflect.DeepEqual(got, want) {
+		t.Fatalf("snapshot=%v want=%v", got, want)
+	}
+}
+
+func TestStderrTailZeroCapacityPromotesToOne(t *testing.T) {
+	t.Parallel()
+	tail := newStderrTail(0)
+	tail.push("x")
+	tail.push("y")
+	if got, want := tail.snapshot(), []string{"y"}; !reflect.DeepEqual(got, want) {
+		t.Fatalf("snapshot=%v want=%v", got, want)
+	}
+}
+
+func TestStderrTailSnapshotIsCopy(t *testing.T) {
+	t.Parallel()
+	tail := newStderrTail(2)
+	tail.push("a")
+	snap := tail.snapshot()
+	tail.push("b")
+	tail.push("c")
+	if got, want := snap, []string{"a"}; !reflect.DeepEqual(got, want) {
+		t.Fatalf("snapshot mutated under writer: %v want %v", got, want)
+	}
+}

scripts/mtxrpicam.sha256

@@ -11,6 +11,9 @@
 #      so the two independent sources agree before recording them here
 #   3. commit Makefile + .goreleaser.yml + this file together
 #
-# Pinned to mediamtx-rpicamera v2.4.3 (matches mediamtx v1.13.1).
-db9310fe114fa09d8427e93331fe6aec9ff3ba4aa486c7e7edbcbab9eae043cf  mtxrpicam_32.tar.gz
-f6073cad9fd6c8570141f08aed72237ec062d28883b8ab62a37dbce190409f0d  mtxrpicam_64.tar.gz
+# Pinned to mediamtx-rpicamera v2.5.6 (matches mediamtx v1.18.1). v2.5.x adds
+# libcamera 0.7 support (#75/#77/#87) which v2.4.3 lacked, so older pins
+# crash on Pi OS images shipping libcamera v0.7.0+rpt2026* (encoder STREAMON
+# fails with "unable to activate output stream"); keep this in lockstep.
+306380abdba970296055e2cbca32ea968778157605d537892ae469bc639fafe9  mtxrpicam_32.tar.gz
+d12475fd4cb9b3f6c959a54e26f4d06b9752ab9c488e839b41fb1a895ae1f325  mtxrpicam_64.tar.gz