| Message ID | 20250429124806.138056-1-isaac.scott@ideasonboard.com |
|---|---|
| Headers | show |
| Series |
|
| Related | show |
Hi Isaac, On Tue, Apr 29, 2025 at 01:48:05PM +0100, Isaac Scott wrote: > In many use cases, including debugging, it would be useful to disable > individual algorithms to ensure the effects of one algorithm are not > affecting others. This patch adds this functionality by allowing users > to mark algorithms as "disabled" in their camera sensor's tuning file. Why do we need this, when we can simply comment them out ? > Isaac Scott (1): > libipa: Allow disabling algorithms via the tuning file > > src/ipa/libipa/module.h | 7 +++++++ > 1 file changed, 7 insertions(+)
Quoting Laurent Pinchart (2025-04-30 09:31:51) > Hi Isaac, > > On Tue, Apr 29, 2025 at 01:48:05PM +0100, Isaac Scott wrote: > > In many use cases, including debugging, it would be useful to disable > > individual algorithms to ensure the effects of one algorithm are not > > affecting others. This patch adds this functionality by allowing users > > to mark algorithms as "disabled" in their camera sensor's tuning file. > > Why do we need this, when we can simply comment them out ? There can be /many/ lines to comment out for a single algorithm. https://git.libcamera.org/libcamera/libcamera.git/tree/src/ipa/rkisp1/data/ov8858.yaml#n8 for instance ... Or for developing/testing - add a single 'disable:' statement - and you can toggle it quickly. You can even comment out the single disable line to re-enable it without modifying other lines... The upstream tuning files we have for libipa are quite simple - but they can grow substantially once we start adding noise reduction, DPC, or dewarp parameters for instance. Not libipa, but trying to comment out AWB from this tuning file for instance would be quite cumbersome: https://git.libcamera.org/libcamera/libcamera.git/tree/src/ipa/rpi/vc4/data/imx219.json#n42 > > Isaac Scott (1): > > libipa: Allow disabling algorithms via the tuning file > > > > src/ipa/libipa/module.h | 7 +++++++ > > 1 file changed, 7 insertions(+) > > -- > Regards, > > Laurent Pinchart
On Wed, Apr 30, 2025 at 09:45:28AM +0100, Kieran Bingham wrote: > Quoting Laurent Pinchart (2025-04-30 09:31:51) > > On Tue, Apr 29, 2025 at 01:48:05PM +0100, Isaac Scott wrote: > > > In many use cases, including debugging, it would be useful to disable > > > individual algorithms to ensure the effects of one algorithm are not > > > affecting others. This patch adds this functionality by allowing users > > > to mark algorithms as "disabled" in their camera sensor's tuning file. > > > > Why do we need this, when we can simply comment them out ? > > There can be /many/ lines to comment out for a single algorithm. > > https://git.libcamera.org/libcamera/libcamera.git/tree/src/ipa/rkisp1/data/ov8858.yaml#n8 > for instance ... > > Or for developing/testing - add a single 'disable:' statement - and you > can toggle it quickly. You can even comment out the single disable line > to re-enable it without modifying other lines... > > The upstream tuning files we have for libipa are quite simple - but they > can grow substantially once we start adding noise reduction, DPC, or > dewarp parameters for instance. > > Not libipa, but trying to comment out AWB from this tuning file for > instance would be quite cumbersome: > > https://git.libcamera.org/libcamera/libcamera.git/tree/src/ipa/rpi/vc4/data/imx219.json#n42 This just means you don't have implemented the right macro in your text editor ;-) Jokes aside, I'm OK with this patch if people find it useful, even if I don't find commenting blocks from tuning files painful myself. As Stefan mentioned, an "enabled" property would be better. Please expand the patch in v2 to also document the property somewhere. Note that this means the "enabled" property will become reserved by libipa, no algorithms will be able to use it for custom purposes. I don't think that's much of a problem. > > > Isaac Scott (1): > > > libipa: Allow disabling algorithms via the tuning file > > > > > > src/ipa/libipa/module.h | 7 +++++++ > > > 1 file changed, 7 insertions(+)