reauktion/daedalus-v4l2
2
0
Fork 0
Code Issues 3 Pull Requests Actions Packages Projects Releases Wiki Activity

kernel + daemon: H.264 B-frame display reorder fix (closes #6) #7

Merged
marfrit merged 1 commits from noether/kernel-daemon-h264-reorder-fix into main 2026-05-21 10:36:53 +00:00
Conversation 0 Commits 1 Files Changed 6
+725 -288
marfrit commented 2026-05-21 10:33:31 +00:00
Owner
Copy Link
Copy Source

Implements the design from #6. Bumps DAEDALUS_PROTO_VERSION 0 → 1; kernel + daemon + tools must be rebuilt in lock-step.

Wire protocol changes

struct daedalus_req_decode {
    ...existing fields...
    __u32 reserved0;      /* pad to 8-byte align */
    __u64 src_pts;        /* V4L2 OUTPUT buf timestamp */
};

#define DAEDALUS_RESP_FLAG_HAS_PIXELS   0x00000001u
#define DAEDALUS_RESP_FLAG_SRC_CONSUMED 0x00000002u

struct daedalus_resp_frame {
    ...existing fields...
    __u32 flags;          /* was 'reserved'; now DAEDALUS_RESP_FLAG_* */
    __u32 reserved0;
    __u64 output_src_pts; /* = frame->pts on drain */
};

Kernel

  • device_run mirrors src_buf->vb2_buf.timestamp into req->src_pts and inf->src_pts.
  • daedalus_inflight carries src_pts so daedalus_complete_resp_frame can stamp dst_buf.timestamp explicitly (V4L2_BUF_FLAG_TIMESTAMP_COPY's auto-pairing no longer applies once src/dst lifecycles decouple).
  • Completion path branches on fr->flags: HAS_PIXELS packs pixels + v4l2_m2m_buf_done(dst, DONE); SRC_CONSUMED releases media_request + v4l2_m2m_buf_done(src) + v4l2_m2m_job_finish.
  • Inflight entry is freed only when both src_buf and dst_buf have been cleared.
  • Combined HAS_PIXELS | SRC_CONSUMED RESPs (the steady-state VP9/AV1 case with no reorder lag) collapse to the prior 1:1 behaviour for free.

Daemon

  • daedalus_decoder_run_request split into submit / drain_one / pack_current.
  • submit sets pkt->pts = req->src_pts before avcodec_send_packet. libavcodec carries pkt->pts → frame->pts even after H.264 DPB display-order reorder.
  • drain_one returns the next display-ordered frame and populates output_src_pts = frame->pts.
  • chardev_client keeps a small (src_pts → cookie, cached_req) lookup table (linear array, ≤64 entries, bounded by V4L2 client buffer-pool depth).
  • handle_req_decode now drains libavcodec in a loop; each frame is routed to its OWNING cookie via the lookup, GET_DMABUF for that cookie, pack, emit RESP_FRAME(HAS_PIXELS, output_src_pts=frame->pts).
  • If the current REQ's cookie isn't drained inside the loop (libavcodec held the frame for display-order release), a standalone SRC_CONSUMED RESP unblocks the kernel scheduler; the parked dst_buf gets pixels in a future drain.

Behaviour

  • VP9 / AV1 (no reorder): exactly one frame per REQ, RESPs collapse to a single combined message, no behaviour change.
  • H.264 with B-frames: pairs of B/P no longer swap; pixels and timestamps stay paired through the display-order routing.

Caveats / out of scope for this PR

  • End-of-stream avcodec_send_packet(NULL) flush isn't wired; on stream stop, parked dst_bufs get VB2_BUF_STATE_ERROR via the existing stop_streaming loop. Acceptable for now (no visible regression — userspace doesn't expect pixels past stream stop).
  • The pending-table lookup is O(N) linear over ≤64 entries; cache-friendly for the bounded N but worth hashing if N ever grows.
  • libva-v4l2-request-fourier#13 (slice-buffer overflow on resolution change) is a parallel issue surfaced during the same diagnostic — separate PR.

Verified

  • Builds clean cross-compiled on higgs against 6.18.29+rpt-rpi-2712 (Pi CM5). Kernel + daemon + tools all link without errors (warnings unchanged from before this PR).
  • Field test on Pi CM5 pending; needs daemon-bump rebuild downstream in marfrit/marfrit-packages (daedalus-v4l2 + daedalus-v4l2-dkms) so apt can swap both binaries atomically per the proto-version lock-step.
Implements the design from #6. Bumps `DAEDALUS_PROTO_VERSION` 0 → 1; kernel + daemon + tools must be rebuilt in lock-step. ## Wire protocol changes ```c struct daedalus_req_decode { ...existing fields... __u32 reserved0; /* pad to 8-byte align */ __u64 src_pts; /* V4L2 OUTPUT buf timestamp */ }; #define DAEDALUS_RESP_FLAG_HAS_PIXELS 0x00000001u #define DAEDALUS_RESP_FLAG_SRC_CONSUMED 0x00000002u struct daedalus_resp_frame { ...existing fields... __u32 flags; /* was 'reserved'; now DAEDALUS_RESP_FLAG_* */ __u32 reserved0; __u64 output_src_pts; /* = frame->pts on drain */ }; ``` ## Kernel - `device_run` mirrors `src_buf->vb2_buf.timestamp` into `req->src_pts` and `inf->src_pts`. - `daedalus_inflight` carries `src_pts` so `daedalus_complete_resp_frame` can stamp `dst_buf.timestamp` explicitly (`V4L2_BUF_FLAG_TIMESTAMP_COPY`'s auto-pairing no longer applies once src/dst lifecycles decouple). - Completion path branches on `fr->flags`: `HAS_PIXELS` packs pixels + `v4l2_m2m_buf_done(dst, DONE)`; `SRC_CONSUMED` releases media_request + `v4l2_m2m_buf_done(src) + v4l2_m2m_job_finish`. - Inflight entry is freed only when both `src_buf` and `dst_buf` have been cleared. - Combined `HAS_PIXELS | SRC_CONSUMED` RESPs (the steady-state VP9/AV1 case with no reorder lag) collapse to the prior 1:1 behaviour for free. ## Daemon - `daedalus_decoder_run_request` split into `submit` / `drain_one` / `pack_current`. - `submit` sets `pkt->pts = req->src_pts` before `avcodec_send_packet`. libavcodec carries `pkt->pts → frame->pts` even after H.264 DPB display-order reorder. - `drain_one` returns the next display-ordered frame and populates `output_src_pts = frame->pts`. - `chardev_client` keeps a small `(src_pts → cookie, cached_req)` lookup table (linear array, ≤64 entries, bounded by V4L2 client buffer-pool depth). - `handle_req_decode` now drains libavcodec in a loop; each frame is routed to its OWNING cookie via the lookup, `GET_DMABUF` for that cookie, pack, emit `RESP_FRAME(HAS_PIXELS, output_src_pts=frame->pts)`. - If the current REQ's cookie isn't drained inside the loop (libavcodec held the frame for display-order release), a standalone `SRC_CONSUMED` RESP unblocks the kernel scheduler; the parked dst_buf gets pixels in a future drain. ## Behaviour - VP9 / AV1 (no reorder): exactly one frame per REQ, RESPs collapse to a single combined message, no behaviour change. - H.264 with B-frames: pairs of `B`/`P` no longer swap; pixels and timestamps stay paired through the display-order routing. ## Caveats / out of scope for this PR - End-of-stream `avcodec_send_packet(NULL)` flush isn't wired; on stream stop, parked dst_bufs get `VB2_BUF_STATE_ERROR` via the existing `stop_streaming` loop. Acceptable for now (no visible regression — userspace doesn't expect pixels past stream stop). - The pending-table lookup is O(N) linear over ≤64 entries; cache-friendly for the bounded N but worth hashing if N ever grows. - libva-v4l2-request-fourier#13 (slice-buffer overflow on resolution change) is a parallel issue surfaced during the same diagnostic — separate PR. ## Verified - Builds clean cross-compiled on higgs against `6.18.29+rpt-rpi-2712` (Pi CM5). Kernel + daemon + tools all link without errors (warnings unchanged from before this PR). - Field test on Pi CM5 pending; needs daemon-bump rebuild downstream in `marfrit/marfrit-packages` (`daedalus-v4l2` + `daedalus-v4l2-dkms`) so apt can swap both binaries atomically per the proto-version lock-step.
marfrit added 1 commit 2026-05-21 10:33:32 +00:00
kernel + daemon: H.264 B-frame display reorder fix (issue #6) 15fc2aba14 
H.264 streams with B-frames showed visibly pair-swapped output in
mpv / Firefox playback through the libva → daedalus_v4l2 path —
"frames went 2 1 4 3 6 5 instead of 1 2 3 4 5 6".  Reproduced in mpv
with --hwdec=vaapi-copy at 720p (bypassing Firefox's compositor),
confirming the bug was in this daemon pipeline, not downstream.

Root cause
----------
libavcodec's H.264 decoder internally reorders output to DISPLAY
order before returning from avcodec_receive_frame.  The daemon
previously called send_packet → receive_frame ONCE per REQ_DECODE
and shipped the resulting pixels in a RESP_FRAME tagged with the
SAME cookie.  For B-frames this is wrong: the frame returned from
receive_frame may belong to an EARLIER bitstream (libavcodec held
it for display-order release).  Cookie N's CAPTURE buffer therefore
got cookie N-2's pixels, while cookie N-2's CAPTURE buffer got
silently marked VB2_BUF_STATE_ERROR (the daemon returned
DAEDALUS_DECODE_NO_FRAME for the cookie whose pixels were held).

Fix shape
---------
Decouple kernel cookie identity (decode-order routing) from
libavcodec's display-ordered output.  Wire-protocol changes:

  REQ_DECODE  + __u64 src_pts        (= src_buf->vb2_buf.timestamp)
  RESP_FRAME  + __u32 flags          (HAS_PIXELS | SRC_CONSUMED)
              + __u64 output_src_pts (= frame->pts on drain)

PROTO_VERSION bumped 0 → 1.  Lock-step rebuild required.

Kernel
------
device_run now mirrors src_buf->vb2_buf.timestamp into req->src_pts
before sending REQ_DECODE, and stores it on the inflight item so
the completion path can stamp dst_buf.timestamp explicitly when
src/dst lifecycles decouple (V4L2_BUF_FLAG_TIMESTAMP_COPY's auto-
pairing no longer applies).

daedalus_complete_resp_frame splits into:

  HAS_PIXELS:    pack pixels into THIS cookie's CAPTURE buffer,
                 stamp dst timestamp from inflight->src_pts,
                 v4l2_m2m_buf_done(dst, DONE/ERROR).
                 No job_finish here.

  SRC_CONSUMED:  release the bound media_request, run
                 v4l2_m2m_buf_done(src) + v4l2_m2m_job_finish so
                 the scheduler can dispatch the next REQ.  dst_buf
                 may still be parked at this point.

Inflight entry is removed and freed only when BOTH src_buf and
dst_buf have been cleared.  Combined HAS_PIXELS|SRC_CONSUMED RESPs
(steady-state VP9/AV1 with no reorder lag) collapse to the prior
1:1 behaviour for free.

Daemon
------
daedalus_decoder_run_request split into three primitives:

  daedalus_decoder_submit       — set pkt->pts = req->src_pts,
                                  avcodec_send_packet.
  daedalus_decoder_drain_one    — avcodec_receive_frame, populate
                                  resp meta + output_src_pts (= the
                                  frame's pts, carried back from
                                  the bitstream that produced it).
  daedalus_decoder_pack_current — pack current AVFrame into the
                                  caller-mapped CAPTURE planes.

chardev_client maintains a small (src_pts → cookie, cached_req)
table indexed linearly (≤64 entries; bounded by V4L2 client buffer
pool depth).  On each REQ_DECODE:

  1. Register (src_pts → cookie) in the table.
  2. submit().
  3. Drain loop: for each frame returned, look up its owner cookie
     via pending_lookup(frame->pts), GET_DMABUF for THAT cookie,
     pack pixels, emit RESP_FRAME(owner_cookie, HAS_PIXELS,
     output_src_pts=frame->pts).  Combine with SRC_CONSUMED when
     owner_cookie equals the current REQ's cookie.
  4. If the current REQ's cookie wasn't drained inside the loop
     (libavcodec held the frame), emit a standalone SRC_CONSUMED
     RESP so the kernel runs job_finish + dispatches the next REQ;
     dst_buf for this cookie stays parked until a future drain
     produces its pixels.

VP9 / AV1 paths are unchanged in behaviour: one frame per REQ,
HAS_PIXELS|SRC_CONSUMED in one combined RESP.

Verified
--------
Builds clean cross-compiled on higgs against 6.18.29+rpt-rpi-2712
(Pi CM5).  Frame-size warning in device_run is pre-existing
(unchanged by this commit).
marfrit merged commit 79256dc7ef into main 2026-05-21 10:36:53 +00:00
marfrit deleted branch noether/kernel-daemon-h264-reorder-fix 2026-05-21 10:36:54 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
No items
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
claude-noether (Claude (noether)) marfrit (Markus Fritsche)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: reauktion/daedalus-v4l2#7
Delete Branch "noether/kernel-daemon-h264-reorder-fix"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?