| Message ID | 20260216203034.27558-1-mzamazal@redhat.com |
|---|---|
| Headers | show |
| Series |
|
| Related | show |
Quoting Milan Zamazal (2026-02-16 20:30:19) > This patch series implements parameters and statistics buffer sharing in > software ISP. This is a rebase of an old patch series, adjusted for the > current master and GPU ISP. It implements the following software ISP > TODOs: > > - 2. Reconsider stats sharing > - 5. Store ISP parameters in per-frame buffers > > The first part of the patches deals with parameters buffers, the next > part with statistics buffers. > > It’s a way to make params correspond to the respective frames, which is > currently not the case with software ISP. But with the plans to compute > stats on a GPU and make everything synchronous, the only useful patches > from this series may eventually be those dropping the software ISP > TODOs. Which means I don’t plan to work on the patches further unless > we decide they are needed. I know it's only RFC, but just reporting the CI pinged me on some compile failures in here. Just trivials by the look of it, an extra semi-colon causing a warning. https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1606180 > > Changes in v2: > - Rebase on master and make it working with GPU ISP. > > Milan Zamazal (14): > libcamera: software_isp: Remove initializer_list include > libcamera: software_isp: Introduce arguments for parameters buffers > libcamera: software_isp: Separate allocation of the parameters buffer > libcamera: software_isp: Track unused parameters buffers > libcamera: software_isp: Allocation of multiple params buffers > libcamera: software_isp: Allocate multiple parameters buffers > libcamera: software_isp: Use multiple parameters buffers in IPA > libcamera: software_isp: Share parameters buffers with debayering > libcamera: software_isp: Remove per-frame params buffers TODO item > libcamera: software_isp: Introduce arguments for statistics buffers > libcamera: software_isp: Allocate statistics buffers > libcamera: software_isp: Track statistics buffers > libcamera: software_isp: Share statistics buffers with IPA > libcamera: software_isp: Remove stats-sharing TODO item > > .../internal/software_isp/software_isp.h | 21 ++- > .../internal/software_isp/swstats_cpu.h | 21 +-- > include/libcamera/ipa/soft.mojom | 11 +- > src/ipa/simple/soft_simple.cpp | 94 +++++++------ > src/libcamera/pipeline/simple/simple.cpp | 12 +- > src/libcamera/software_isp/TODO | 45 ------- > src/libcamera/software_isp/debayer.cpp | 42 ++++-- > src/libcamera/software_isp/debayer.h | 12 +- > src/libcamera/software_isp/debayer_cpu.cpp | 79 ++++++----- > src/libcamera/software_isp/debayer_cpu.h | 16 ++- > src/libcamera/software_isp/debayer_egl.cpp | 18 ++- > src/libcamera/software_isp/debayer_egl.h | 11 +- > src/libcamera/software_isp/software_isp.cpp | 126 +++++++++++++++--- > src/libcamera/software_isp/swstats_cpu.cpp | 66 ++++----- > 14 files changed, 336 insertions(+), 238 deletions(-) > > -- > 2.53.0 >
2026. 02. 16. 21:30 keltezéssel, Milan Zamazal írta: > This patch series implements parameters and statistics buffer sharing in > software ISP. This is a rebase of an old patch series, adjusted for the > current master and GPU ISP. It implements the following software ISP > TODOs: > > - 2. Reconsider stats sharing > - 5. Store ISP parameters in per-frame buffers > > The first part of the patches deals with parameters buffers, the next > part with statistics buffers. > > It’s a way to make params correspond to the respective frames, which is > currently not the case with software ISP. But with the plans to compute > stats on a GPU and make everything synchronous, the only useful patches I think I'm missing something because as far as I can see splitting the statistics collection would just introduce more async: 1. raw buffer arrives -> queue statistics calculation (async) 2. statistics arrive -> queue parameter computations (asnyc) 3. parameters arrive -> queue frame processing with parameters (async) 4. frame arrives -> signal completion at least assuming you want to use the statistics of the corresponding frame. > from this series may eventually be those dropping the software ISP > TODOs. Which means I don’t plan to work on the patches further unless > we decide they are needed. > > Changes in v2: > - Rebase on master and make it working with GPU ISP. > > Milan Zamazal (14): > libcamera: software_isp: Remove initializer_list include > libcamera: software_isp: Introduce arguments for parameters buffers > libcamera: software_isp: Separate allocation of the parameters buffer > libcamera: software_isp: Track unused parameters buffers > libcamera: software_isp: Allocation of multiple params buffers > libcamera: software_isp: Allocate multiple parameters buffers > libcamera: software_isp: Use multiple parameters buffers in IPA > libcamera: software_isp: Share parameters buffers with debayering > libcamera: software_isp: Remove per-frame params buffers TODO item > libcamera: software_isp: Introduce arguments for statistics buffers > libcamera: software_isp: Allocate statistics buffers > libcamera: software_isp: Track statistics buffers > libcamera: software_isp: Share statistics buffers with IPA > libcamera: software_isp: Remove stats-sharing TODO item > > .../internal/software_isp/software_isp.h | 21 ++- > .../internal/software_isp/swstats_cpu.h | 21 +-- > include/libcamera/ipa/soft.mojom | 11 +- > src/ipa/simple/soft_simple.cpp | 94 +++++++------ > src/libcamera/pipeline/simple/simple.cpp | 12 +- > src/libcamera/software_isp/TODO | 45 ------- > src/libcamera/software_isp/debayer.cpp | 42 ++++-- > src/libcamera/software_isp/debayer.h | 12 +- > src/libcamera/software_isp/debayer_cpu.cpp | 79 ++++++----- > src/libcamera/software_isp/debayer_cpu.h | 16 ++- > src/libcamera/software_isp/debayer_egl.cpp | 18 ++- > src/libcamera/software_isp/debayer_egl.h | 11 +- > src/libcamera/software_isp/software_isp.cpp | 126 +++++++++++++++--- > src/libcamera/software_isp/swstats_cpu.cpp | 66 ++++----- > 14 files changed, 336 insertions(+), 238 deletions(-) >
Kieran Bingham <kieran.bingham@ideasonboard.com> writes: > Quoting Milan Zamazal (2026-02-16 20:30:19) >> This patch series implements parameters and statistics buffer sharing in >> software ISP. This is a rebase of an old patch series, adjusted for the > >> current master and GPU ISP. It implements the following software ISP >> TODOs: >> >> - 2. Reconsider stats sharing >> - 5. Store ISP parameters in per-frame buffers >> >> The first part of the patches deals with parameters buffers, the next >> part with statistics buffers. >> >> It’s a way to make params correspond to the respective frames, which is >> currently not the case with software ISP. But with the plans to compute >> stats on a GPU and make everything synchronous, the only useful patches >> from this series may eventually be those dropping the software ISP >> TODOs. Which means I don’t plan to work on the patches further unless >> we decide they are needed. > > I know it's only RFC, but just reporting the CI pinged me on some > compile failures in here. Just trivials by the look of it, an extra > semi-colon causing a warning. > > https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1606180 Ah, right. Apparently clang only, I should check it next time. >> Changes in v2: >> - Rebase on master and make it working with GPU ISP. >> >> Milan Zamazal (14): >> libcamera: software_isp: Remove initializer_list include >> libcamera: software_isp: Introduce arguments for parameters buffers >> libcamera: software_isp: Separate allocation of the parameters buffer >> libcamera: software_isp: Track unused parameters buffers >> libcamera: software_isp: Allocation of multiple params buffers >> libcamera: software_isp: Allocate multiple parameters buffers >> libcamera: software_isp: Use multiple parameters buffers in IPA >> libcamera: software_isp: Share parameters buffers with debayering >> libcamera: software_isp: Remove per-frame params buffers TODO item >> libcamera: software_isp: Introduce arguments for statistics buffers >> libcamera: software_isp: Allocate statistics buffers >> libcamera: software_isp: Track statistics buffers >> libcamera: software_isp: Share statistics buffers with IPA >> libcamera: software_isp: Remove stats-sharing TODO item >> >> .../internal/software_isp/software_isp.h | 21 ++- >> .../internal/software_isp/swstats_cpu.h | 21 +-- >> include/libcamera/ipa/soft.mojom | 11 +- >> src/ipa/simple/soft_simple.cpp | 94 +++++++------ >> src/libcamera/pipeline/simple/simple.cpp | 12 +- >> src/libcamera/software_isp/TODO | 45 ------- >> src/libcamera/software_isp/debayer.cpp | 42 ++++-- >> src/libcamera/software_isp/debayer.h | 12 +- >> src/libcamera/software_isp/debayer_cpu.cpp | 79 ++++++----- >> src/libcamera/software_isp/debayer_cpu.h | 16 ++- >> src/libcamera/software_isp/debayer_egl.cpp | 18 ++- >> src/libcamera/software_isp/debayer_egl.h | 11 +- >> src/libcamera/software_isp/software_isp.cpp | 126 +++++++++++++++--- >> src/libcamera/software_isp/swstats_cpu.cpp | 66 ++++----- >> 14 files changed, 336 insertions(+), 238 deletions(-) >> >> -- >> 2.53.0 >>
Barnabás Pőcze <barnabas.pocze@ideasonboard.com> writes: > 2026. 02. 16. 21:30 keltezéssel, Milan Zamazal írta: >> This patch series implements parameters and statistics buffer sharing in >> software ISP. This is a rebase of an old patch series, adjusted for the >> current master and GPU ISP. It implements the following software ISP >> TODOs: >> - 2. Reconsider stats sharing >> - 5. Store ISP parameters in per-frame buffers >> The first part of the patches deals with parameters buffers, the next >> part with statistics buffers. >> It’s a way to make params correspond to the respective frames, which is >> currently not the case with software ISP. But with the plans to compute >> stats on a GPU and make everything synchronous, the only useful patches > > I think I'm missing something because as far as I can see splitting the statistics > collection would just introduce more async: > > 1. raw buffer arrives -> queue statistics calculation (async) > 2. statistics arrive -> queue parameter computations (asnyc) > 3. parameters arrive -> queue frame processing with parameters (async) > 4. frame arrives -> signal completion > > at least assuming you want to use the statistics of the corresponding frame. Async is fine (or even useful) as long as we use the right data together. The idea is to replace the current mechanism of "use whatever is available at the moment" with buffer/frame id tracking. (I'm sure this doesn't work correctly at the moment as I just rebased the very old series, adjusted it for GPU ISP some way, made it to compile (with gcc), and checked it runs. For example, I can see there is still, unused, debayerParams_ around. I believe there are other bugs. Doing more ATM would be waste of time unless we decide we need this. Hence RFC.) >> from this series may eventually be those dropping the software ISP >> TODOs. Which means I don’t plan to work on the patches further unless >> we decide they are needed. >> Changes in v2: >> - Rebase on master and make it working with GPU ISP. >> Milan Zamazal (14): >> libcamera: software_isp: Remove initializer_list include >> libcamera: software_isp: Introduce arguments for parameters buffers >> libcamera: software_isp: Separate allocation of the parameters buffer >> libcamera: software_isp: Track unused parameters buffers >> libcamera: software_isp: Allocation of multiple params buffers >> libcamera: software_isp: Allocate multiple parameters buffers >> libcamera: software_isp: Use multiple parameters buffers in IPA >> libcamera: software_isp: Share parameters buffers with debayering >> libcamera: software_isp: Remove per-frame params buffers TODO item >> libcamera: software_isp: Introduce arguments for statistics buffers >> libcamera: software_isp: Allocate statistics buffers >> libcamera: software_isp: Track statistics buffers >> libcamera: software_isp: Share statistics buffers with IPA >> libcamera: software_isp: Remove stats-sharing TODO item >> .../internal/software_isp/software_isp.h | 21 ++- >> .../internal/software_isp/swstats_cpu.h | 21 +-- >> include/libcamera/ipa/soft.mojom | 11 +- >> src/ipa/simple/soft_simple.cpp | 94 +++++++------ >> src/libcamera/pipeline/simple/simple.cpp | 12 +- >> src/libcamera/software_isp/TODO | 45 ------- >> src/libcamera/software_isp/debayer.cpp | 42 ++++-- >> src/libcamera/software_isp/debayer.h | 12 +- >> src/libcamera/software_isp/debayer_cpu.cpp | 79 ++++++----- >> src/libcamera/software_isp/debayer_cpu.h | 16 ++- >> src/libcamera/software_isp/debayer_egl.cpp | 18 ++- >> src/libcamera/software_isp/debayer_egl.h | 11 +- >> src/libcamera/software_isp/software_isp.cpp | 126 +++++++++++++++--- >> src/libcamera/software_isp/swstats_cpu.cpp | 66 ++++----- >> 14 files changed, 336 insertions(+), 238 deletions(-) >>