Repository navigation
Adding Ambarella S3L support - #71
DeathCamel58 wants to merge 11 commits into
Conversation
PR Summary by QodoAdd an ioctl-only hardware backend for Ambarella S3L
AI Description
Diagram
High-Level Assessment
Files changed (14)
|
Code Review by Qodo
1. Larger JPEG streams use the wrong size
|
|
Thanks for the review. I've force-pushed the series with fixes folded into the commits they belong to; every commit still builds. Retested on the IP4M-1026E with the SDK microcode:
|
|
I've force-pushed one change to the base commit, made for the review of OpenIPC/firmware#2793: the ISP configuration file is now relocatable (S3LISP03). The file's table pointers used to carry the IMG buffer's absolute DSP address. If a
The older S3LISP01/02 formats are dropped; nothing shipped them. Tested on the IP4M-1026E: the relocated OV4689 file loads, and the picture and 3A come up as before. Every commit still builds. #2793 and this PR need to go in together, because the generator's output and this loader now use the same new format. |
|
Tried this branch (HEAD TL;DR: with the right toolchain and one change to the preview bring‑up, divinus runs on S3L and serves a decodable H.264 + JPEG stream. Two things tripped it up: 1.
|
|
Thanks for running it under 1. Toolchain. 2. The preview restart.
3. Also in this push:
On your config notes: right, the real DSP does H.265 as well. |
The HAL_* macros pick their colours with isatty(STDERR_FILENO) before a message's arguments are evaluated. When stderr is not a terminal (a log file, a pipe, a service manager), isatty() sets errno to ENOTTY, so every message that reports strerror(errno) printed "Not a tty" (musl) or "Inappropriate ioctl for device" (glibc) instead of the failure it was about. The colour test now keeps errno. Found while running the Ambarella S3L backend (#71) with its log redirected: a refused ioctl that returned EPERM was reported as "Not a tty". Tested with a HAL_DANGER(..., strerror(errno)) after errno = EPERM: stderr to a file printed "Inappropriate ioctl for device" before and "Operation not permitted" after; on a terminal both print the latter.
|
Thanks — the rebase and the fallback both look right, and the Two things on the back of that: I took the PREVIEW→IDLE refusal back to the emulated DSP, and here's the boot recipe you asked for. The re-idle is now accepted on the emulated DSP tooYour datapoint — that the restart is accepted on the IP4M-1026E with both microcodes — was the clue that this was an emulation gap, not a HAL bug. With Reading the shipped So on the emulated S3L divinus now runs your unpatched bring-up — the normal restart path, no fallback — straight through to (One unrelated thing I see on the emulated DSP, non-fatal: QEMU boot recipeEverything is OpenIPC public artefacts; no real hardware. 1. Build the emulator (gives git clone https://github.com/OpenIPC/qemu-bench && cd qemu-bench
bash qemu/setup.sh # clones QEMU 10.2.0, applies the SoC models, builds
./qemu-src/build/qemu-system-arm -M help | grep '^s3l'2. Kernel + rootfs. 3. Boot as an initramfs with a small cat > root/init <<'INIT'
#!/bin/sh
mount -t proc proc /proc; mount -t sysfs sysfs /sys
mount -t devtmpfs dev /dev; mount -t tmpfs tmpfs /tmp; mount -t tmpfs tmpfs /var
mkdir -p /var/run /var/log /etc/sensors
ifconfig eth0 10.0.2.15 netmask 255.255.255.0 up; route add default gw 10.0.2.2
load_ambarella -i # insmods iav.ko/dsp.ko, creates /dev/iav
divinus
INIT
chmod +x root/init
(cd root && find . | cpio -o -H newc | gzip -9) > rd.gz
./qemu-src/build/qemu-system-arm \
-M s3l,dsp=on,sensor=ov4689 -m 256M -nographic -monitor none \
-kernel zImage -initrd rd.gz \
-global amb-dsp.dump-dir=/tmp/frames \
-nic user,model=xm-dwmac,hostfwd=tcp::5600-:554 \
-append "console=ttyS0,115200 rdinit=/init loglevel=4 s3l.env_sensor"
Config notes for S3L under QEMU: the emulated DSP does H.264 + MJPEG only (no H.265), so Happy to help wire this into a CI check if that's useful on your side. |
A HAL for the Ambarella S3L (S3LM) over its IAV driver (/dev/iav), for ARM with either float ABI: it drives the driver through ioctls and links no vendor library. It is picked when the IAV driver reports an S3L (AMBARELLA_CHIP in /proc/ambarella/iav). - VIN mode and frame rate, mirror/flip through VIN SET_MIRROR, the source buffers, preview, and one H.264 stream read from the bitstream buffer (RTSP, MP4). Encoder settings are the driver's defaults for now. MJPEG is skipped on S3L. - ISP configuration: system.sensor_config names an S3LISP03 file (made from options, with no vendor data, by the generator OpenIPC's ambarella-osdrv-s3l package carries in src/isp), loaded into the IMG buffer before the first ENABLE_PREVIEW and applied with the IMG 'p' set ioctls. Without it, the one for the sensor the VIN reports is loaded, /etc/sensors/<sensor>.bin, as OpenIPC's S3L package generates them. Its table pointers are IMG-buffer offsets with a relocation list, and the HAL adds the buffer's address as the driver reports it, so the file does not depend on where the buffer is. The file's sizes and offsets are checked against the IMG buffer, and against the blocks the HAL keeps below the command rings, before anything is copied or applied. The IMG command rings are zeroed first: after a cold boot DRAM holds garbage there that the DSP would otherwise read as commands. - 3A in Divinus: DSP statistics (CONFIG_STATISTICS / GET_STATISTICS, two slices per frame stitched), auto exposure through the VIN AGC/shutter ioctls every Ambarella sensor driver implements, with anti-flicker, auto white balance from the per-tile raw G/R, G/B median (handed to the AE thread under a lock, so both gains come from one frame), and bad-pixel thresholds that follow the gain. New isp options auto_exposure, exposure_target and auto_white_balance. - The DSP takes the 3A statistics setup into effect only when preview starts, and the setup can only be sent once preview runs. In the first preview after the microcode is loaded, the SDK's 2018/10/28 284712 (which OpenIPC ships) sends no statistics, and this camera's own 267837 sends stale ones: the same three buffers over and over. So the setup is sent, left in place for a frame, and preview started again before any stream encodes, on every start (about 0.15 s). A driver that refuses to go back to idle (an emulated DSP does) leaves preview running, and the HAL carries on with a warning. - Grayscale for night mode, by swapping the chroma-scale curve. - The sensor name for /api/status, from the VIN device info. - build.sh s3l (and CI): OpenIPC's toolchain.ambarella-s3l. The S3L is a Cortex-A9 with VFPv3; the armhf-musl toolchain emits VFPv4 fused multiply-adds, which fault here. - No audio yet: with audio enabled, Divinus warns and runs without it. Saving the configuration keeps audio.enable as it was set, so the choice is not lost (app_config.audio_requested). - The main source buffer is sized for the H.264/H.265 stream, and the VIN runs at the fastest enabled stream's frame rate. - IAV state changes (idle, preview, encode start/stop) run with signals blocked: the driver waits for the DSP interruptibly, and a signal in that window (a service stop repeating SIGTERM) left IAV stuck, refusing every later start until a reboot. Tested on an Amcrest IP4M-1026E (Dahua IPC-HFW1430SN, S3L + OV4689): H.264 2688x1512 over RTSP from a cold boot, AE/AWB, night-mode grayscale, stop/start/restart cycles, with both the SDK microcode (284712) and the camera's own (267837). In the first preview after boot, the 3A statistics now change from frame to frame with both (with 267837 they had cycled through three fixed buffers). Builds for all build.yml targets, s3l among them (VFPv3/NEONv1).
MJPEG was skipped on S3L. It now runs as its own IAV stream (MJPEG stream type) from the second source buffer, sized to mjpeg width/height (s3l_sub_size(), called before the pipeline is created), with the quality from mjpeg.qfactor (which media_mjpeg_enable passes on for every platform since OpenIPC#68). When MJPEG is the only stream, the main buffer already has its size and a sub-buffer cannot match it, so MJPEG encodes from the main buffer instead. The frame thread routes each frame by the stream id in its descriptor and only splits NAL units for H.264. Tested on the IP4M-1026E: H.264 2688x1512 over RTSP plus 640x360 MJPEG at 20 fps, qfactor 30 and 90 giving the matching JPEG quantisation tables (8.8 KB and 27 KB frames); MJPEG-only at 1280x720.
The mp4 bitrate, mode, GOP and profile were ignored: the encoder ran on the driver defaults (8 Mbit/s smart CBR, GOP 30, Main). Before encoding starts the H.264 config now takes the GOP length (an IDR every GOP) and the profile, and the bitrate stream config the average bitrate in bit/s with CBR or VBR, read-modify-write so the driver's QP limits stay. Other modes keep the driver's smart CBR, with a warning. request_idr() now forces an IDR through the stream config, so a new RTSP client no longer waits for the next GOP. The S3L's IAV encodes Main or High only: Baseline is encoded as Main, with a warning. Tested on the IP4M-1026E: 4096 kbit/s CBR, GOP 40, High measures about 4.2 Mbit/s with keyframes exactly every 40 frames; 2048 kbit/s VBR measures 2.2 Mbit/s; a second RTSP client connecting mid-stream puts an IDR off the GOP grid in the first client's stream. Profiles 1 and 2 come out as Main and High; profile 0 comes out as Main.
The second source buffer is limited to 720x720 (IAV: "cannot be larger than 720x720"), so an MJPEG stream of 1280x720 or more could not start. Sizes above that now use the third source buffer, which takes up to the sensor size. Source buffers that are not used are switched off rather than left as the previous run configured them; IAV keeps that setup, and a stale third buffer larger than the main one made SET_SRCBUF_SETUP fail. Tested on the IP4M-1026E with H.264 2688x1512: MJPEG 640x360 (second buffer), 1280x720 and 1920x1080 (third buffer) at 20 fps, and MJPEG-only at 1280x720 right after a 1920x1080 run.
/image.jpg (and the ONVIF snapshot URI) did not work on S3L. Snapshots now follow T31: with MJPEG on they reuse the MJPEG stream at its resolution; otherwise jpeg_init() opens its own encoder stream, an MJPEG-type IAV stream at jpeg width/height and qfactor running at about 1 fps. A snapshot waits for that stream's next frame, which the frame thread copies only while a snapshot is pending; frames of the snapshot stream are not passed on as video. Every stream now also sets its frame-rate divider explicitly, since IAV keeps per-stream settings across runs and a stream left at the snapshot rate would otherwise stay at 1 fps. Tested on the IP4M-1026E with H.264 2688x1512 alongside: own snapshot stream at 1280x720 / q85 and 1920x1080 (5 of 5 snapshots, about 0.8 s each); with MJPEG 640x360 on, snapshots from it (0.1 s each) and MJPEG at 20 fps; MJPEG back at 20 fps after a snapshot-stream run.
mp4.codec H265 was refused on S3L. The encoder supports it: it now runs as an IAV H.265 stream, configured like H.264 through the H.265 config ioctl (GOP length, an IDR every GOP; the driver's Main profile) and the H.265 stream configs (bitrate with CBR/VBR, forced IDR), which mirror the H.264 ones at +0x2000. IAV hands an H.265 frame over one slice at a time (three slices at 2688x1512, each with its own descriptor carrying the slice index and count). The frame thread collects a frame's slices, starting only at slice 0, and passes the whole frame on with one timestamp and its H.265 NAL types. Tested on the IP4M-1026E: 2688x1512 H.265 Main over RTSP at 20 fps, 4096 kbit/s CBR measuring about 4.1 Mbit/s, keyframes exactly every 40 frames, a forced IDR when a second client connects, recordings that decode without errors, /video.mp4, alongside MJPEG, and restarts. H.264 unchanged.
Frames were stamped with the time the frame thread read their
descriptor. After a large keyframe the next frames wait in IAV's queue
while the keyframe is sent, so they were stamped late and then in a
burst (steps of 9963, 2033, 1466 instead of 4500 at 20 fps); recordings
logged "non monotonically increasing dts" around every keyframe and
probing guessed 60 fps. Frames now carry the 90 kHz PTS from their
descriptor, which the RTP layer expects ("the encoder's capture time");
the read time stays as the fallback when a descriptor has none.
Tested on the IP4M-1026E: H.264 and H.265 RTSP recordings step exactly
4500 per frame at 20 fps with no ffmpeg warnings (5-8 per 10 s
before), and /video.mp4 reports 20 fps; MJPEG and snapshots unchanged.
Regions were not drawn on S3L. They now use IAV's encode overlay: up to 10 areas per stream (one per OSD region), 8-bit indexed pixels with a 256-entry CLUT of V, U, Y and alpha, in the overlay buffer (QUERY_BUF 4). The S3L driver's overlay insert is 248 bytes, with 10 areas against the S2L SDK's 4. Each ARGB1555 bitmap becomes indexed colour: index 0 is transparent, opaque colours get BT.601 limited-range entries at the region's opacity, and an image with more than 255 colours falls back to 3-3-2. Every region has two pages, so an update writes the idle one and the DSP never reads a half-written bitmap. Areas follow the driver's rules (even x and width, y and height multiples of 4) and are clipped to the stream. The regions are drawn on the H.264/H.265 stream and put back whenever it starts. IAV keeps overlays across runs, and an area that does not fit a stream stops it from starting, so every stream's overlay is cleared when preview starts. Tested on the IP4M-1026E at 2688x1512: a timestamp updating every second, text with an outline at half opacity, a PNG with transparent parts and over 255 colours, regions clipped at the right and bottom edges, an off-frame one skipped, odd positions snapped, a region removed, and the overlay kept across an encoder restart from /api/mp4. Builds for all 12 targets in build.yml.
An H.264/H.265 stream could only be the main one, from the main source buffer. Now the HAL allows three encode streams, and one smaller than the main picture is encoded from the source buffer of its size: the second one, or the third when MJPEG put its own size there. The second buffer is limited to 720x720 and its size is fixed when preview starts, so it is planned then. MJPEG or the snapshots keep it at their size when they are at most 720x720; otherwise, or when neither is on, it is 640 wide at the main aspect (640x360 for 2688x1512), so a 640x360 sub-stream, the size the RTSP sub stream of OpenIPC#62 asks for, finds it. A size no buffer has is refused with the sizes there are (media.c then stops Divinus starting, as for any stream it cannot create). The on-screen display stays on the stream from the main buffer, and the frame thread now watches all three streams. Divinus itself creates no second H.264/H.265 stream yet: that comes with the sub-stream plumbing of OpenIPC#62. Until then, this path is taken only when a streamer creates one. Tested on the IP4M-1026E, with a local hook in media.c creating a 640x360 H.264 sub-stream next to the 2688x1512 main stream (main at 20 fps throughout): alone; with MJPEG at 1280x720 (three streams) and at 640x360 (sharing the buffer); with 1 fps JPEG snapshots at 1920x1080; and with MJPEG at 640x480, where the sub-stream is refused and, with the hook, the rest runs. Without a sub-stream, the board's default (main stream, OSD) runs as before. Builds for all 12 targets in build.yml.
IAV refuses to start a stream when all streams together exceed the DSP's encoder load, in macroblocks per second (570600 on S3L, "1080p60 + 480p60"). Each stream's frame rate was set only after it started, at the VIN's, so the check counted every stream at the full rate: MJPEG at the main size (or the yaml template's 3840x2160, which falls back to it) beside the 2688x1512 main stream never started. The frame rate is now set before the start, as the configured fraction of the VIN's (snapshots 1 fps as before). The HAL adds up the streams' load first: MJPEG and the snapshots that would not fit run at the highest frame rate that does, with a warning (and are refused if none does); an H.264/H.265 stream that would not fit is refused with the numbers, which stops Divinus starting. A stream asking for more than the rate the pipeline started with runs at that rate, with a warning. A stream is enabled before its encoder starts, so its first frame (the IDR) is not dropped. Tested on the IP4M-1026E: MJPEG at 3840x2160 and 20 fps runs at 2688x1512 and 15 fps next to the main stream at 20 fps (it did not start before); MJPEG at 1920x1080 and 15 fps gives exactly 15 fps; with a 640x360 sub-stream and full-size MJPEG, MJPEG drops to 14 fps; the board's default is unchanged. The refusal of an H.264/H.265 stream was not hit, as no case here adds one after the budget is used. Builds for all 12 targets in build.yml.
The main source buffer scales the VIN down by less than 4, and buffers and encode streams are a multiple of 16 wide and of 8 high. A main stream outside that, such as 640x360 or 960x540 on a 2688x1512 sensor, failed SET_SRCBUF_SETUP and Divinus exited without video. A size four or more times below the VIN is now encoded as a centred window of the smallest main buffer there can be (688x384 here), so only the edges are cropped, and sizes off the 16x8 grid are rounded up with a warning (960x540 is encoded as 960x544); the requested size is matched against the buffers the same way. The second source buffer, at least 240x240 in this mode, is left off when it would be smaller, and MJPEG or the snapshots use the main buffer. Its type is now set on every start, as for the third and fourth: IAV keeps it across runs, so a run with it off left the next one unable to encode from it. An MJPEG or JPEG size too small for a sub-buffer (at least 240x240, and scaled down from the main buffer by less than 6, IAV's check_zoom_limit) is scaled up at its aspect with a warning, and one not below the main buffer warns that it is encoded at the main buffer's size. The README notes that a stream's size, or a higher frame rate, takes a restart of Divinus to change. Tested on the IP4M-1026E: main streams of 640x360 (exited before), 960x540 (encoded 960x544), 656x370 (a 656x376 window of a 688x384 buffer), 1280x720 and 2688x1512; MJPEG-only 960x540; MJPEG at 640x360 on the second buffer before and after a run with it off; MJPEG and JPEG at 160x120 (SET_SRCBUF_SETUP failed before; now 464x344 with a warning), and MJPEG at 1920x1080 beside a 1280x720 main stream (warns, encodes 1280x720).
|
Thanks for chasing the re-idle down in the DSP model, and for the recipe. I've saved it, and I'll use it to check HAL changes before they reach a camera. A CI check built on it would be welcome; let's look at that once this PR is in. Your note that a missing I've force-pushed the branch, still on the same base. A missing or rejected ISP configuration is now an error ("No ISP configuration loaded (…); the DSP produces no image without one"), raised before preview starts, so the DSP stays idle. Tested on the camera without a reboot in between: no file and an old-format file each give that error and leave IAV idle, and a valid file straight after starts normally and streams. Under qemu-bench this means the image needs an ISP file. Generating it is one command, with the generator and preset from OpenIPC/firmware ( python3 s3l_isp_gen.py root/etc/sensors/ov4689.bin --preset ov4689.yamlBuilds with |
Adds a hardware backend for the Ambarella S3L (S3LM), tested on an Amcrest IP4M-1026E (Dahua IPC-HFW1430SN, S3L + OV4689). It drives the IAV driver (
/dev/iav) through ioctls only and links no vendor library, so it builds for ARM with either float ABI. It's picked when the driver reports an S3L (AMBARELLA_CHIPin/proc/ambarella/iav).What works
system.sensor_config, or else/etc/sensors/<sensor>.binfor the detected sensor. OpenIPC's S3L package generates it at build time from options, with no vendor data: see ambarella-osdrv-s3l: generate the OV4689 ISP configuration at build time firmware#2793 for the generator and the OV4689 preset. With that in the image, Divinus needs no ISP configuration of its own.ispoptions:auto_exposure,exposure_targetandauto_white_balance(seedoc/config.md).Majestic on S3L
Majestic also ships for S3L (OpenIPC/firmware#2785). On this board, both streamers now start preview on a fresh boot and keep a steady timestamp OSD: Majestic since its
master+f2fa260build, after the report on OpenIPC/firmware#2787. Divinus resets IAV's source-buffer types itself and double-buffers its OSD regions, so it has needed neither fix.Notes for review
mjpeg.qfactor, whichmedia_mjpeg_enablepasses on for every platform since Passing the MJPEG qfactor to the encoder #68../build.sh s3l(also in CI) uses OpenIPC'stoolchain.ambarella-s3l. The S3L's Cortex-A9 has VFPv3, while thearmhf-musltoolchain emits VFPv4 fused multiply-adds, which fault on it with SIGILL.HAL_PLATFORM_S3Lis inserted before T31 in the platform enum. Nothing stores or indexes the numeric values.Testing
On the IP4M-1026E, netbooted, with each microcode:
/image.jpgsnapshots;mp41280×720 with MJPEG at 1920×1080;qfactor30 and 90 (matching JPEG quantisation tables);/etc/sensors/ov4689.binwith nosensor_configset.Every commit builds with the S3L toolchain, and CI builds every target in build.yml,
s3lincluded. The HAL was also read over in several review rounds (concurrency, memory safety against driver-reported sizes and the ISP file, integration with the other HALs) until a round came back with nothing left to fix (ten rounds).Related OpenIPC work for this board: OpenIPC/firmware#2782 (SDK microcode), OpenIPC/linux#177 (overlayfs) and OpenIPC/linux#178 (board DTB).