[jclaveau]

Fixes the vchiq_mmal_port_enable+0xd4 oops reported in #4920 (root cause and test details in #4920 (comment)).

When a client hits end of stream before its first frame, it sends V4L2_ENC_CMD_STOP before VIDIOC_STREAMON(OUTPUT). The driver submits the EOS buffer to the still-disabled MMAL input port, which queues it on port->buffers; bcm2835_codec_start_streaming() then memsets that buffer while it is still linked, and port_enable() oopses in list_del() on its NULL pointers. After that the codec reports Mutex fail until a reboot. Jellyfin hits this when it transcodes the last HLS segment of a file whose audio outlasts its video.

This holds the EOS back while the input port is disabled and submits it once start_streaming() has enabled the port, so a drain requested before the first frame still completes. vidioc_decoder_cmd() shares the path.

A V4L2_ENC_CMD_START/V4L2_DEC_CMD_START drops an EOS that is still held back, and the driver only logs a failure to submit the held EOS from start_streaming(), so STREAMON never fails with the port already enabled.

This starts the drain once streaming begins, where dev-encoder.rst says a STOP on a non-streaming queue must not start one. Following the spec strictly makes jellyfin-ffmpeg 7.1 hang on a zero-frame encode, and v4l2_m2m_encoder_cmd() makes the same choice; benchmarks and the other review points are in the comment below.

Reproducer:

ffmpeg -f lavfi -i testsrc=size=1280x720:rate=24 -t 0 -c:v h264_v4l2m2m -b:v 2M -f null -

Tested on a Pi 4 with 6.12.93+rpt-rpi-v8 (patch built out of tree from rpi-6.12.y; the file's EOS handling is identical in rpi-6.18.y and the patch applies cleanly there, but I have not booted a 6.18 kernel):

test	stock codec	patched codec
zero-frame encode, Pi OS ffmpeg 5.1.9 and jellyfin-ffmpeg 7.1.4	oops	no oops in 12 runs; ffmpeg exits 0
Jellyfin's last-segment command	oops 4/4	no oops 3/3; ffmpeg exits on its own
1-frame and 48-frame encodes	—	normal H.264 output

Sequence diagrams (before / after) and the patched decision flow

Before: stock driver

sequenceDiagram
    autonumber
    participant C as ffmpeg
    participant D as bcm2835-codec<br/>(V4L2 driver)
    participant P as MMAL input port<br/>(VCHIQ, kernel side)
    participant F as VPU firmware

    Note over P: port disabled
    C->>D: ENCODER_CMD(STOP)
    D->>P: vchiq_mmal_submit_buffer(eos_buffer)
    Note over P: port disabled → eos_buffer is only<br/>list_add_tail()'ed on port->buffers
    C->>D: STREAMON(OUTPUT)
    D->>D: start_streaming(): memset(&eos_buffer, 0)
    Note over P: eos_buffer still linked in port->buffers,<br/>its list pointers are now NULL
    D->>P: vchiq_mmal_port_enable()
    P->>P: list_del(eos_buffer) → write to NULL+8
    Note over P: OOPS — codec then answers<br/>"Mutex fail" until reboot

Loading

After: this PR

sequenceDiagram
    autonumber
    participant C as ffmpeg
    participant D as bcm2835-codec<br/>(V4L2 driver)
    participant P as MMAL input port<br/>(VCHIQ, kernel side)
    participant F as VPU firmware

    Note over P: port disabled
    C->>D: ENCODER_CMD(STOP)
    D->>D: submit_eos(): port disabled →<br/>eos_buffer_pending = true, return 0
    C->>D: STREAMON(OUTPUT)
    D->>D: start_streaming(): eos_pending = true,<br/>memset(&eos_buffer, 0) — safe, never queued
    D->>P: vchiq_mmal_port_enable()
    Note over P: port enabled, port->buffers clean
    D->>P: submit_eos() → vchiq_mmal_submit_buffer(eos_buffer)
    P->>F: EOS buffer
    C->>D: STREAMON(CAPTURE)
    F-->>D: output buffer with EOS flag
    D-->>C: CAPTURE buffer flagged V4L2_BUF_FLAG_LAST
    Note over C: drain complete, ffmpeg exits 0

Loading

Decision flow in the patched driver

flowchart TD
    STOP["ENCODER_CMD / DECODER_CMD STOP"] --> SUB{"submit_eos():<br/>input port enabled?"}
    SUB -- yes --> SEND["vchiq_mmal_submit_buffer(eos_buffer)<br/>(streaming already: unchanged behaviour)"]
    SUB -- no --> HOLD["eos_buffer_pending = true<br/>nothing queued on port->buffers"]

    HOLD --> NEXT{"next call on this queue"}
    NEXT -- "START command" --> CANCEL["cancel_pending_eos():<br/>pending = false, in_use = false<br/>(stream goes on, no early LAST)"]
    NEXT -- "STREAMON(OUTPUT)" --> SS["start_streaming():<br/>remember pending, memset eos_buffer,<br/>pending = false"]

    SS --> EN{"vchiq_mmal_port_enable()<br/>ok?"}
    EN -- no --> ERR1["log 'Failed enabling i/p port'<br/>(as before)"]
    EN -- yes --> WAS{"EOS was pending?"}
    WAS -- no --> DONE["normal streaming"]
    WAS -- yes --> SEND2["submit_eos() → port enabled →<br/>vchiq_mmal_submit_buffer(eos_buffer)"]
    SEND2 --> OK{"submit ok?"}
    OK -- yes --> LAST["firmware returns EOS →<br/>CAPTURE buffer gets V4L2_BUF_FLAG_LAST"]
    OK -- no --> ERR2["log 'EOS buffer submit failed'<br/>STREAMON still succeeds<br/>(port is already enabled)"]

Loading

Assisted-by: Claude:claude-opus-5-5

[jclaveau]

A self-review of this patch turned up four points. Two are fixed in a follow-up commit (d3e78c1) and two are left as they are; each is described below so you can judge them. All runs: Pi 4, 6.12.93+rpt-rpi-v8, codec module rebuilt out of tree with each variant; 0 oopses in all of them.

Test tool: a small V4L2 client (drain-test.c, at the end of this comment) that issues V4L2_ENC_CMD_STOP at different points and reports what comes back on CAPTURE (frames = buffers with data, last = a V4L2_BUF_FLAG_LAST buffer within 5 s). 3 runs per cell, identical results unless noted.

Variants:

• A: this PR as first pushed
• A2: A + fixes 1 and 3 below (now the PR head, d3e78c1)
• B: A2 + the EOS waits for frames queued before STREAMON (point 2, not pushed)
• S: A2, but the EOS is dropped while the port is disabled (strict spec, point 4, not pushed)

order of calls	A	A2	B	S
zero: STOP → STREAMON(OUT) → STREAMON(CAP) (ffmpeg, zero frames)	LAST, 0 frames	same	same	no LAST
queued: QBUF(OUT)×3 → STOP → STREAMON both	LAST, 0 of 3 frames	0 of 3	3 of 3	3 of 3, no LAST
restart: STOP → START → STREAMON both → QBUF×3 → STOP	LAST before the frames, 0 of 3	3 of 3	3 of 3	3 of 3
normal: STREAMON both → QBUF×3 → STOP	0–1 of 3	0–1 of 3	0–1 of 3	0–1 of 3
jellyfin-ffmpeg 7.1, zero frames	exits rc=0	rc=0	rc=0	hangs (SIGKILL after 20 s), 3/3
RPi OS ffmpeg 5.1, zero frames	rc=0	rc=0	rc=0	rc=0
ffmpeg 5.1 / 7.1, 1 and 48 frames	same bytes in every variant (5299 / 97437 / 95040)

1. STREAMON failed after the port was enabled (fixed). If submitting the held-back EOS failed, start_streaming() returned the error with the MMAL input port already enabled; vb2 then treats streaming as failed, and a retried STREAMON trips the WARN_ONs at the top of start_streaming(). The failure is now only logged, as the STOP ioctl does. I could not make the VCHIQ submit fail on the real hardware, so this one is untested.

2. Frames queued before STREAMON come after the EOS (left as is). In the "queued" order the EOS reaches the firmware ahead of those frames (A, A2: 0 of 3 encoded). Variant B fixes it by keeping the EOS held back in start_streaming() while OUTPUT buffers are queued, and sending it from device_run() once the last one has gone. B passes every row above. I left it out: no client I know queues frames, then STOPs, then streams on (ffmpeg doesn't), and B touches device_run(), which runs from the m2m worker outside dev_mutex, so it needs more thought than this fix. Happy to send it as a follow-up if you want it.

3. A START did not cancel a held-back STOP (fixed). Before: STOP → START → STREAMON sent the EOS anyway, ending the next stream before its first frame ("restart" row, A). V4L2_ENC_CMD_START / V4L2_DEC_CMD_START now drop it. A STREAMOFF without START still keeps it: vb2 does not call stop_streaming() on a queue that never started.

4. Not following dev-encoder.rst to the letter (deliberate). The spec says a STOP on a queue that isn't streaming "will not fail" but "will not initiate the Drain sequence". Variant S does exactly that, and jellyfin-ffmpeg 7.1 then hangs on a zero-frame encode, 3 out of 3 times: it sends STOP before STREAMON (v4l2_receive_packet() → ff_v4l2_context_enqueue_frame(NULL)) and then waits for the LAST buffer. Keeping the drain matches the shared helper too: v4l2_m2m_encoder_cmd() → v4l2_update_last_buf_state() starts a drain whatever the streaming state, marking last_src_buf or flagging the next CAPTURE buffer next_buf_last. So the deviation is the same one the m2m helpers make.

Not caused by this PR, for the record. "normal" order: frames already queued when STOP arrives on a streaming encoder are lost (0–1 of 3 in every variant, so it predates this patch). vidioc_encoder_cmd() submits the EOS to MMAL straight away while those frames still sit in the m2m queue waiting for device_run(). ffmpeg avoids it by dequeuing as it goes. Not touched here.

On A2 (the PR head) as a DKMS module, re-run on the real Jellyfin request from the issue: 3/3 no oops, and the minimal ffmpeg -t 0 reproducer 2/2 no oops; no codec errors in the kernel log.

drain-test.c

// SPDX-License-Identifier: GPL-2.0
/*
 * Drain-order test for the bcm2835-codec H.264 encoder: issues
 * V4L2_ENC_CMD_STOP at different points of the stream setup and reports
 * what comes back on the CAPTURE queue.
 *
 *   drain-test /dev/video11 zero      STOP, STREAMON(OUT), STREAMON(CAP)
 *                                     (ffmpeg's order for a zero-frame encode)
 *   drain-test /dev/video11 queued 3  QBUF(OUT)x3, STOP, STREAMON both
 *   drain-test /dev/video11 normal 3  STREAMON both, QBUF(OUT)x3, STOP
 *   drain-test /dev/video11 restart 3 STOP, START, STREAMON both, QBUF(OUT)x3,
 *                                     wait 1.5 s, STOP
 *
 * Prints one line:
 *   scenario=<s> frames=<buffers with data> bytes=<n> last=<0|1>
 *   early_last=<0|1> ms=<elapsed>
 * last=0 means no LAST buffer within 5 s, i.e. a client waiting for the
 * drain would hang. early_last=1 (restart only) means a LAST buffer came
 * before the second STOP.
 */
#include <errno.h>
#include <fcntl.h>
#include <poll.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/ioctl.h>
#include <sys/mman.h>
#include <time.h>
#include <unistd.h>
#include <linux/videodev2.h>

#define WIDTH 1280
#define HEIGHT 720
#define OUT_COUNT 4
#define CAP_COUNT 4

static int video_fd;
static void *out_map[OUT_COUNT];
static unsigned int out_size;
static unsigned int cap_length[CAP_COUNT];
static long total_frames, total_bytes;
static int last_seen;

static void die(const char *what)
{
	fprintf(stderr, "%s: %s\n", what, strerror(errno));
	exit(1);
}

static void xioctl(unsigned long request, void *arg, const char *what)
{
	if (ioctl(video_fd, request, arg) < 0)
		die(what);
}

static long now_ms(void)
{
	struct timespec ts;

	clock_gettime(CLOCK_MONOTONIC, &ts);
	return ts.tv_sec * 1000 + ts.tv_nsec / 1000000;
}

static void set_format(enum v4l2_buf_type type, unsigned int fourcc)
{
	struct v4l2_format fmt = { .type = type };

	fmt.fmt.pix_mp.width = WIDTH;
	fmt.fmt.pix_mp.height = HEIGHT;
	fmt.fmt.pix_mp.pixelformat = fourcc;
	fmt.fmt.pix_mp.num_planes = 1;
	xioctl(VIDIOC_S_FMT, &fmt, "S_FMT");
	if (type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
		out_size = fmt.fmt.pix_mp.plane_fmt[0].sizeimage;
}

static void request_buffers(enum v4l2_buf_type type, unsigned int count)
{
	struct v4l2_requestbuffers req = {
		.type = type, .memory = V4L2_MEMORY_MMAP, .count = count,
	};

	xioctl(VIDIOC_REQBUFS, &req, "REQBUFS");
	for (unsigned int i = 0; i < count; i++) {
		struct v4l2_plane plane = { 0 };
		struct v4l2_buffer buf = {
			.type = type, .memory = V4L2_MEMORY_MMAP, .index = i,
			.m.planes = &plane, .length = 1,
		};

		xioctl(VIDIOC_QUERYBUF, &buf, "QUERYBUF");
		void *map = mmap(NULL, plane.length, PROT_READ | PROT_WRITE,
				 MAP_SHARED, video_fd, plane.m.mem_offset);
		if (map == MAP_FAILED)
			die("mmap");
		if (type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
			out_map[i] = map;
			memset(map, 128, plane.length);
		} else {
			cap_length[i] = plane.length;
		}
	}
}

static void queue_buffer(enum v4l2_buf_type type, unsigned int index)
{
	struct v4l2_plane plane = { 0 };
	struct v4l2_buffer buf = {
		.type = type, .memory = V4L2_MEMORY_MMAP, .index = index,
		.m.planes = &plane, .length = 1,
	};

	if (type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
		plane.bytesused = out_size;
		buf.timestamp.tv_usec = index * 41666;
	} else {
		plane.length = cap_length[index];
	}
	xioctl(VIDIOC_QBUF, &buf, "QBUF");
}

static void stream_on(enum v4l2_buf_type type)
{
	xioctl(VIDIOC_STREAMON, &type, "STREAMON");
}

static void encoder_command(unsigned int cmd)
{
	struct v4l2_encoder_cmd ec = { .cmd = cmd };

	xioctl(VIDIOC_ENCODER_CMD, &ec, "ENCODER_CMD");
}

/* Collects CAPTURE buffers until LAST or the timeout; returns 1 on LAST. */
static int collect_capture(int timeout_ms)
{
	long deadline = now_ms() + timeout_ms;

	while (!last_seen && now_ms() < deadline) {
		struct pollfd pfd = { .fd = video_fd, .events = POLLIN };
		struct v4l2_plane plane = { 0 };
		struct v4l2_buffer buf = {
			.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
			.memory = V4L2_MEMORY_MMAP, .m.planes = &plane,
			.length = 1,
		};
		struct v4l2_buffer out_buf = buf;
		struct v4l2_plane out_plane = { 0 };

		poll(&pfd, 1, 100);
		out_buf.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
		out_buf.m.planes = &out_plane;
		ioctl(video_fd, VIDIOC_DQBUF, &out_buf);

		if (ioctl(video_fd, VIDIOC_DQBUF, &buf) < 0) {
			if (errno == EPIPE)
				last_seen = 1;
			continue;
		}
		if (plane.bytesused) {
			total_frames++;
			total_bytes += plane.bytesused;
		}
		if (buf.flags & V4L2_BUF_FLAG_LAST)
			last_seen = 1;
		else
			queue_buffer(V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
				     buf.index);
	}
	return last_seen;
}

int main(int argc, char **argv)
{
	const enum v4l2_buf_type out = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
	const enum v4l2_buf_type cap = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
	const char *scenario;
	unsigned int frame_count;
	int early_last = 0;
	long start;

	if (argc < 3) {
		fprintf(stderr, "usage: %s <device> zero|queued|normal|restart [frames]\n",
			argv[0]);
		return 2;
	}
	scenario = argv[2];
	frame_count = argc > 3 ? atoi(argv[3]) : 0;
	if (frame_count > OUT_COUNT) {
		fprintf(stderr, "frames must be <= %d\n", OUT_COUNT);
		return 2;
	}

	video_fd = open(argv[1], O_RDWR | O_NONBLOCK);
	if (video_fd < 0)
		die("open");
	set_format(out, V4L2_PIX_FMT_YUV420);
	set_format(cap, V4L2_PIX_FMT_H264);
	request_buffers(out, OUT_COUNT);
	request_buffers(cap, CAP_COUNT);
	for (unsigned int i = 0; i < CAP_COUNT; i++)
		queue_buffer(cap, i);

	start = now_ms();
	if (!strcmp(scenario, "zero")) {
		encoder_command(V4L2_ENC_CMD_STOP);
		stream_on(out);
		stream_on(cap);
	} else if (!strcmp(scenario, "queued")) {
		for (unsigned int i = 0; i < frame_count; i++)
			queue_buffer(out, i);
		encoder_command(V4L2_ENC_CMD_STOP);
		stream_on(out);
		stream_on(cap);
	} else if (!strcmp(scenario, "normal")) {
		stream_on(out);
		stream_on(cap);
		for (unsigned int i = 0; i < frame_count; i++)
			queue_buffer(out, i);
		encoder_command(V4L2_ENC_CMD_STOP);
	} else if (!strcmp(scenario, "restart")) {
		encoder_command(V4L2_ENC_CMD_STOP);
		encoder_command(V4L2_ENC_CMD_START);
		stream_on(out);
		stream_on(cap);
		for (unsigned int i = 0; i < frame_count; i++)
			queue_buffer(out, i);
		early_last = collect_capture(1500);
		encoder_command(V4L2_ENC_CMD_STOP);
	} else {
		fprintf(stderr, "unknown scenario \"%s\"\n", scenario);
		return 2;
	}
	collect_capture(5000);

	printf("scenario=%s frames=%ld bytes=%ld last=%d early_last=%d ms=%ld\n",
	       scenario, total_frames, total_bytes, last_seen, early_last,
	       now_ms() - start);
	return 0;
}

I kept the fixes as a separate commit so the change since the first review is easy to see; I'm happy to squash the two into one (with the spec note above in the message) if you prefer.

Assisted-by: Claude:claude-opus-5-5

[pelwell]

I, for one, would appreciate not being part of the debug loop while you and your AI work knock the PR into shape. Try and get your ducks in a row, and a nice concise explanation (the first post in this thread was good) before submitting.

[jclaveau]

Sorry, that was noise I should have kept on my side. I'll finish the remaining work on my fork and only come back here with a single clean commit and a short summary.

Assisted-by: Claude:claude-opus-5-5

[jclaveau]

I, for one, would appreciate not being part of the debug loop while you and your AI work knock the PR into shape. Try and get your ducks in a row, and a nice concise explanation (the first post in this thread was good) before submitting.

I achieved making AI produce sharp issues and fine prs but making it go straightforward is by far the hardest part. I guess it makes all of us tired of it.

Sorry again.

Written by a human