| Message ID | 20241122201305.1668098-1-mzamazal@redhat.com |
|---|---|
| Headers | show |
| Series |
|
| Related | show |
Hi, thanks for the patch! I gave this a quick try with Millipixels (https://source.puri.sm/Librem5/millipixels) which uses raw streams. It looks like the app will need manual adjustments to work with this patch - but that's kinda expected IIUC (Millipixels apparently doesn't request raw streams explicitly yet). It looks like other API changes on master / in 0.4 will require manual changes to the app anyway, so that shouldn't be a big deal. Do I understand correctly that exposure and gain were never supported for raw streams yet? Because in that case the patch here *is* already useful for Millipixels at it would allow it to co-exist with apps requiring swISP without requiring hacks like this: https://gitlab.postmarketos.org/postmarketOS/pmaports/-/blob/master/temp/libcamera/0002-libcamera-simple-Force-disable-softwareISP-for-milli.patch In other words: even without gain/exposure I'd be happy to have this in 0.4, especially if that allows us to enable the swISP on all platforms by default. On 22.11.24 21:13, Milan Zamazal wrote: > This makes raw streams working again in ‘simple’ pipeline when software > ISP is enabled for the given device. For now, only a single raw stream > is supported and there are no immediate plans to change that unless > there is a request for more. > > An example ‘cam’ invocation requesting a raw stream rather than a debayered stream: > > cam -c1 -C8 -s role=raw,width=1920,height=1080 -Ffile#.raw > > See the commit message for more information about the patch. > > The patch is RFC because I’m not sure where it is placed on the scale > between a hack/PoC and a proper solution and because it’s not that > useful until a control for exposure/gain adjustments is implemented. > > In the meantime, fixed exposure+gain can be hardwired in > src/ipa/simple/soft_simple.cpp by changing the lines > > ctrls.set(V4L2_CID_EXPOSURE, …); > ctrls.set(V4L2_CID_ANALOGUE_GAIN, …); > > to set the desired values (values used under the given conditions can be > obtained from normal software ISP debayered output debug log). > > Milan Zamazal (1): > libcamera: simple: Fix raw output > > src/libcamera/pipeline/simple/simple.cpp | 66 +++++++++++++++++++----- > 1 file changed, 54 insertions(+), 12 deletions(-) >
Hi Laurent, Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes: > On Fri, Nov 22, 2024 at 09:13:03PM +0100, Milan Zamazal wrote: >> This makes raw streams working again in ‘simple’ pipeline when software >> ISP is enabled for the given device. For now, only a single raw stream >> is supported and there are no immediate plans to change that unless >> there is a request for more. > > Cameras don't support multiple raw streams in general. By "only a single > raw stream is supported", do you mean you can't capture raw + processed > ? Yes. > I think that should be fixed, otherwise this sounds too much of a > hack. The primary purpose of this patch is to remedy the situation that `simple' pipeline can produce a raw stream only when software ISP is disabled in the sources for the given device. But I can further work on raw + processed if it solves a real problem. >> An example ‘cam’ invocation requesting a raw stream rather than a debayered stream: >> >> cam -c1 -C8 -s role=raw,width=1920,height=1080 -Ffile#.raw >> >> See the commit message for more information about the patch. >> >> The patch is RFC because I’m not sure where it is placed on the scale >> between a hack/PoC and a proper solution and because it’s not that >> useful until a control for exposure/gain adjustments is implemented. >> >> In the meantime, fixed exposure+gain can be hardwired in >> src/ipa/simple/soft_simple.cpp by changing the lines >> >> ctrls.set(V4L2_CID_EXPOSURE, …); >> ctrls.set(V4L2_CID_ANALOGUE_GAIN, …); >> >> to set the desired values (values used under the given conditions can be >> obtained from normal software ISP debayered output debug log). >> >> Milan Zamazal (1): >> libcamera: simple: Fix raw output >> >> src/libcamera/pipeline/simple/simple.cpp | 66 +++++++++++++++++++----- >> 1 file changed, 54 insertions(+), 12 deletions(-)
Hi Robert, Robert Mader <robert.mader@collabora.com> writes: > Hi, thanks for the patch! > > I gave this a quick try with Millipixels (https://source.puri.sm/Librem5/millipixels) which uses raw streams. It looks like the app will need manual > adjustments to work with this patch - but that's kinda expected IIUC > (Millipixels apparently doesn't request raw streams explicitly yet). Thanks for testing. > It looks like other API changes on master / in 0.4 will require manual > changes to the app anyway, so that shouldn't be a big deal. > > Do I understand correctly that exposure and gain were never supported > for raw streams yet? I think so, I guess they were meant to be set externally, but I'm not sure. > Because in that case the patch here *is* already useful for > Millipixels at it would allow it to co-exist with apps requiring swISP > without requiring hacks like this: > https://gitlab.postmarketos.org/postmarketOS/pmaports/-/blob/master/temp/libcamera/0002-libcamera-simple-Force-disable-softwareISP-for-milli.patch > > In other words: even without gain/exposure I'd be happy to have this in 0.4, especially if that allows us to enable the swISP on all platforms by > default. OK, let's see what we can reasonably do for 0.4. > On 22.11.24 21:13, Milan Zamazal wrote: >> This makes raw streams working again in ‘simple’ pipeline when software >> ISP is enabled for the given device. For now, only a single raw stream >> is supported and there are no immediate plans to change that unless >> there is a request for more. >> >> An example ‘cam’ invocation requesting a raw stream rather than a debayered stream: >> >> cam -c1 -C8 -s role=raw,width=1920,height=1080 -Ffile#.raw >> >> See the commit message for more information about the patch. >> >> The patch is RFC because I’m not sure where it is placed on the scale >> between a hack/PoC and a proper solution and because it’s not that >> useful until a control for exposure/gain adjustments is implemented. >> >> In the meantime, fixed exposure+gain can be hardwired in >> src/ipa/simple/soft_simple.cpp by changing the lines >> >> ctrls.set(V4L2_CID_EXPOSURE, …); >> ctrls.set(V4L2_CID_ANALOGUE_GAIN, …); >> >> to set the desired values (values used under the given conditions can be >> obtained from normal software ISP debayered output debug log). >> >> Milan Zamazal (1): >> libcamera: simple: Fix raw output >> >> src/libcamera/pipeline/simple/simple.cpp | 66 +++++++++++++++++++----- >> 1 file changed, 54 insertions(+), 12 deletions(-) >>