replay: C3/C3X hardware decoder #35821

jakethesnake420 · 2025-07-27T20:22:54Z

On the comma device, kill openpilot managed procs in tmux with ctrl+c, then start the ui again with ./selfdrive/ui/ui.
then run:
COMMA_CACHE=/data/cache ./tools/replay/replay --demo --ecam --dcam

adeebshihadeh

Very cool! Did a quick pass and left some comments to start.

Do we really need the rotator? Decoder won't output NV12?

(To clarify, I'd like to merge the rotator support anyway. It's nice to have support for all the HW blocks in openpilot anyway.)

adeebshihadeh · 2025-08-01T21:54:12Z

tools/replay/qcom/decoder.h

+		(V4L2_CID_MPEG_MSM_VIDC_BASE + 22)
+
+
+enum v4l2_mpeg_vidc_video_decoder_multi_stream {


looks like this is copy/pasted from some kernel header?

and a bunch of other stuff in this file

adeebshihadeh · 2025-08-01T21:55:13Z

msgq_repo

what's this for?

adeebshihadeh · 2025-08-01T21:56:46Z

tools/replay/qcom/sde_rotator.h

@@ -0,0 +1,37 @@
+#pragma once
+
+#include <fcntl.h>


does this file really use all these includes? I imagine half are unused

adeebshihadeh · 2025-08-01T21:57:32Z

tools/replay/qcom/sde_rotator.cc

+#define V4L2_PIX_FMT_NV12_UBWC v4l2_fourcc('Q', '1', '2', '8')
+#endif
+
+static void checked_ioctl(int fd, unsigned long request, void *argp) {


let's put this in common/ instead of copy/pasting everywhere

or better yet make check an argument to util::safe_ioctl?

What if safe_ioctl accepts a message and an error callback? It is used in spi.cc so it would be like this:

auto fail = [serial, this] {cleanup(); throw std::runtime_error("Error connecting to panda with serial: " + serial);}; // SPI settings util::safe_ioctl(spi_fd, SPI_IOC_WR_MODE, &spi_mode, "failed setting SPI mode", fail); util::safe_ioctl(spi_fd, SPI_IOC_WR_MAX_SPEED_HZ, &spi_speed, "failed setting SPI speed", fail); util::safe_ioctl(spi_fd, SPI_IOC_WR_BITS_PER_WORD, &spi_bits_per_word, "failed setting SPI bits per word", fail);

template<typename ErrorCallback> int safe_ioctl(int fd, unsigned long request, void *argp, const char* error_msg, ErrorCallback error_callback) { int ret; do { ret = ioctl(fd, request, argp); } while ((ret == -1) && (errno == EINTR)); if (ret == -1 && error_msg) { LOGE("ioctl failed: %s", error_msg); error_callback(); } return ret; }

tools/replay/qcom/sde_rotator.cc

adeebshihadeh · 2025-08-01T21:59:04Z

tools/replay/qcom/sde_rotator.cc

+  if (!queued)                       // nothing in flight
+    return nullptr;


use braces even for one liners

adeebshihadeh · 2025-08-01T21:59:19Z

tools/replay/qcom/sde_rotator.cc

+  return &cap_buf;
+}
+
+void SdeRotator::convertStride(VisionBuf *rotated_buf, VisionBuf *user_buf) {


rotate can't handle this?

jakethesnake420 · 2025-08-02T20:34:11Z

I am basing this off a working example code https://github.com/jakethesnake420/v4l2-decode. With that said, I don't have a complete understanding of why certain things are done the way they are.

Do we really need the rotator? Decoder won't output NV12?

The venus driver throws an error when trying to set the capture buffer type to NV12.

msm_vidc:   fw: 
               <VFW_E:HostDr:265d:92a139e6:0> vDec_LoadResources(1228): Linear format not supported for DPB!

comma@comma-fa9d355d:/$ strings firmware/image/venus.* | grep Linear
%s(%d): Linear format not supported for DPB!
%s(%d): Linear format not supported for DPB!

Which tells me that the decoded picture buffer needs to be UBWC. The DPB format seems to get overridden when I set the capture plane format. I have tried a few things to get it working but the only way I could see is to use the rotator.

The stuff in decoder.h is copied from kernel headers to work around build issues. I think this can be cleaned up I just have not spent the time yet to solve the issue.

The change to msgq_repo is to add a modified version of the VisionBuf::allocate that maps the memory differently.

new function allocate_no_cache:

void VisionBuf::allocate_no_cache(size_t length) {
ion_alloc.len = length;
ION_IOC_ALLOC
ION_IOC_MAP
ION_IOC_FREE
mmap

versus original allocate:

void VisionBuf::allocate(size_t length) {
ion_alloc.len = length + PADDING_CL + sizeof(uint64_t);
ION_IOC_ALLOC
ION_IOC_SHARE
mmap

calls ION_IOC_FREE later

rotate can't handle this?

Another pain point. I couldn't get the driver to accept a custom fmt_cap.fmt.pix.bytesperline as its calculated by the driver and its based on the resolution and alignment. I tried to format the output to a size that the driver would accept and then scale it up/down but I faced additional issues. I am not an expert on troubleshooting v4l2 or video formats so its difficult for me to tell what is wrong when an image is deformed a certain way.

adeebshihadeh · 2025-08-04T20:26:53Z

@jakethesnake420 can you rebase after the ioctl improvements and do a bit of cleanup? then I can see if I see can simplify this a bit before merging

adeebshihadeh · 2025-08-04T21:08:32Z

trigger-jenkins

jakethesnake420 · 2025-08-05T02:37:12Z

can you rebase after the ioctl improvements and do a bit of cleanup? then I can see if I see can simplify this a bit before merging

Rebased, cleaned and further simplified.

jakethesnake420 · 2025-08-05T11:59:38Z

The last major thing to simplify is the visionbuf stuff. Maybe allocate() can be refactored slightly to work instead of adding the new allocate_no_cache function. I originally did it this way to match the example from samsung with little understanding.

github-actions · 2025-08-15T02:14:12Z

This PR has had no activity for 9 days. It will be automatically closed in 2 days if there is no activity.

…-replay

adeebshihadeh · 2025-08-24T00:47:32Z

I started working on simplifying this but there's too many things that are a mess:

there's multiple errors
the msgq_repo change doesn't even seem necessary. if you replace allocate_no_cache with allocate, everything still seems to work
the rotator shouldn't be necessary. according to the kernel driver, non-UBWC should work fine
we shouldn't have to handle the alignment. the alignment at runtime is already driven by the encoder requirements

I can understand if one or two of these are left, but this feels like a PR that just started working.

jakethesnake420 · 2025-08-31T21:16:02Z

Ok I will take another swing at these issues.

…rotator

jakethesnake420 · 2025-09-02T02:49:44Z

I was able to find solutions.

Can use existing visionbuf allocate function.
The decoder can convert to NV12 directly.
The decoded frames have the correct stride.
Error messages were not real errors.

jakethesnake420 · 2025-09-02T02:52:48Z

tools/replay/qcom/decoder.cc

@@ -259,7 +262,7 @@ bool MsmVidc::setDBP() {
  struct v4l2_ext_control control[2] = {0};
  struct v4l2_ext_controls controls = {0};
  control[0].id           = V4L2_CID_MPEG_VIDC_VIDEO_STREAM_OUTPUT_MODE;
-  control[0].value        = 0; // V4L2_CID_MPEG_VIDC_VIDEO_STREAM_OUTPUT_PRIMARY
+  control[0].value        = 1; // V4L2_CID_MPEG_VIDC_VIDEO_STREAM_OUTPUT_SECONDARY


This was the key difference to get the decoder to decode directly to linear NV12 (non-UBWC)

…-replay

adeebshihadeh · 2025-09-02T20:48:55Z

Nice, this is a lot simpler but now I'm getting this immediately when running ./replay --demo:

common/util.cc: safe_ioctl error: VIDIOC_STREAMON CAPTURE failed Invalid argument(22) (fd: 10 request: 4004612 argp: 0x7f5fffde14)

jakethesnake420 · 2025-09-02T22:29:16Z

Unfortunately I can't reproduce this. There will likely be information in dmesg. You can change the driver debug verbosity
echo "0xFFFF" > /sys/kernel/debug/msm_vidc/debug_level

I just did a clean install as a test and it works for me.

…-replay

jakethesnake420 · 2025-09-05T21:27:24Z

It should be possible to use the ion buffers created by vipc, eliminating the copy buffer and buffer creation code. But it may not be simple.

adeebshihadeh · 2025-09-05T23:05:43Z

Thanks for the PR!

This reverts commit 1f1efec.

jakethesnake420 · 2025-09-07T14:33:37Z

Your welcome!

jakethesnake420 marked this pull request as draft July 27, 2025 20:26

jakethesnake420 force-pushed the qcom-replay branch from 61cae04 to 24f0095 Compare July 28, 2025 02:10

jakethesnake420 marked this pull request as ready for review August 1, 2025 20:20

jakethesnake420 mentioned this pull request Aug 1, 2025

replay: convert UBWC VisionBuf to nv12 with sde rotator #35575

Closed

adeebshihadeh requested changes Aug 1, 2025

View reviewed changes

jakethesnake420 force-pushed the qcom-replay branch from 06bd2ec to 66121d1 Compare August 3, 2025 17:02

Test User added 3 commits August 4, 2025 21:13

bump msgq

6d2c04d

add third_party/linux/include/media/msm_vidc.h

552ea50

add sde_rotator hw interface

57425a1

jakethesnake420 force-pushed the qcom-replay branch 2 times, most recently from 7aad2e2 to 561efd8 Compare August 5, 2025 02:29

Test User added 3 commits August 4, 2025 21:32

add msm_vidc hw decoder interface

61be81c

update SConscript to build qcom decoder and rotator

d4693d6

use qcom decoder in replay framereader

0d6aa7a

jakethesnake420 force-pushed the qcom-replay branch from 561efd8 to 0d6aa7a Compare August 5, 2025 02:32

jakethesnake420 mentioned this pull request Aug 6, 2025

Make replay tool run in realtime on a comma 3/3X #33007

Closed

github-actions bot added the stale label Aug 15, 2025

adeebshihadeh added the ignore stale label Aug 15, 2025

Merge branch 'master' of ssh://github.com/commaai/openpilot into qcom…

7455dde

…-replay

adeebshihadeh marked this pull request as draft August 24, 2025 00:47

decode directly to NV12 with the correct stride without using the hw …

276bc77

…rotator

jakethesnake420 force-pushed the qcom-replay branch from 21ddb9d to 276bc77 Compare September 2, 2025 02:38

bump msgq back to master

4b60876

jakethesnake420 marked this pull request as ready for review September 2, 2025 02:43

jakethesnake420 commented Sep 2, 2025

View reviewed changes

Test User and others added 3 commits September 1, 2025 21:57

don't compile rotator

498af0c

Merge branch 'master' of ssh://github.com/commaai/openpilot into qcom…

95a5ddd

…-replay

cleanup

d39c6a2

adeebshihadeh added 2 commits September 5, 2025 14:09

Merge branch 'master' of ssh://github.com/commaai/openpilot into qcom…

a4a7150

…-replay

works now but much to simplify

57a5239

adeebshihadeh added 2 commits September 5, 2025 15:30

rm signals

737f1c0

rm header

8aef290

github-actions bot added the tools label Sep 5, 2025

adeebshihadeh merged commit 1f1efec into commaai:master Sep 5, 2025
10 checks passed

Edison-CBS added a commit to Edison-CBS/openpilot that referenced this pull request Sep 6, 2025

Revert "replay: C3/C3X hardware decoder (commaai#35821)"

d6ccd3e

This reverts commit 1f1efec.

Edison-CBS added a commit to Edison-CBS/openpilot that referenced this pull request Sep 6, 2025

Revert "replay: C3/C3X hardware decoder (commaai#35821)"

c6d4b27

This reverts commit 1f1efec.

royjr mentioned this pull request Sep 8, 2025

Cannot build openpilot: master on macOS #36119

Open

		(V4L2_CID_MPEG_MSM_VIDC_BASE + 22)


		enum v4l2_mpeg_vidc_video_decoder_multi_stream {

replay: C3/C3X hardware decoder #35821

replay: C3/C3X hardware decoder #35821

Conversation

jakethesnake420 commented Jul 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adeebshihadeh left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jakethesnake420 Aug 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jakethesnake420 commented Aug 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adeebshihadeh commented Aug 4, 2025

Uh oh!

adeebshihadeh commented Aug 4, 2025

Uh oh!

jakethesnake420 commented Aug 5, 2025

Uh oh!

jakethesnake420 commented Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Aug 15, 2025

Uh oh!

adeebshihadeh commented Aug 24, 2025

Uh oh!

jakethesnake420 commented Aug 31, 2025

Uh oh!

jakethesnake420 commented Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adeebshihadeh commented Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jakethesnake420 commented Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jakethesnake420 commented Sep 5, 2025

Uh oh!

Uh oh!

adeebshihadeh commented Sep 5, 2025

Uh oh!

jakethesnake420 commented Sep 7, 2025

Uh oh!

Uh oh!

jakethesnake420 commented Jul 27, 2025 •

edited

Loading

adeebshihadeh left a comment •

edited

Loading

jakethesnake420 Aug 2, 2025 •

edited

Loading

jakethesnake420 commented Aug 2, 2025 •

edited

Loading

jakethesnake420 commented Aug 5, 2025 •

edited

Loading

jakethesnake420 commented Sep 2, 2025 •

edited

Loading

adeebshihadeh commented Sep 2, 2025 •

edited

Loading

jakethesnake420 commented Sep 2, 2025 •

edited

Loading