| Message ID | 20210614104553.497626-1-hiroh@chromium.org |
|---|---|
| State | Accepted |
| Commit | 1fea2730a162905794277b7c21dee283c9cd3a02 |
| Headers | show |
| Series |
|
| Related | show |
Hi Hiro, Thank you for the patch. On Mon, Jun 14, 2021 at 07:45:53PM +0900, Hirokazu Honda wrote: > HAL_PIXEL_FORMAT_RAW_OPAQUE is requested only for > Zero-Shutter-Lag (ZSL). ZSL requires RAW and YUV reprocessing. > Since either of them is not supported by libcamera, supporting > RAW_OPAQUE format doesn't make sense. Drop the format from the > supported format list. > > Signed-off-by: Hirokazu Honda <hiroh@chromium.org> This looks sensible to me. Given that the formats below are specific to the IPU3, the code would need to be reworked when we'll implement ZSL support anyway. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> I'll wait for a second tag before merging. > --- > src/android/camera_device.cpp | 11 ----------- > 1 file changed, 11 deletions(-) > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp > index fe332ec3..14022aed 100644 > --- a/src/android/camera_device.cpp > +++ b/src/android/camera_device.cpp > @@ -120,17 +120,6 @@ const std::map<int, const Camera3Format> camera3FormatsMap = { > false, > "RAW16" > } > - }, { > - HAL_PIXEL_FORMAT_RAW_OPAQUE, { > - { > - formats::SBGGR10_IPU3, > - formats::SGBRG10_IPU3, > - formats::SGRBG10_IPU3, > - formats::SRGGB10_IPU3 > - }, > - false, > - "RAW_OPAQUE" > - } > }, > }; >
Hi Laurent, On Tue, Jun 15, 2021 at 7:44 AM Laurent Pinchart < laurent.pinchart@ideasonboard.com> wrote: > Hi Hiro, > > Thank you for the patch. > > On Mon, Jun 14, 2021 at 07:45:53PM +0900, Hirokazu Honda wrote: > > HAL_PIXEL_FORMAT_RAW_OPAQUE is requested only for > > Zero-Shutter-Lag (ZSL). ZSL requires RAW and YUV reprocessing. > > Since either of them is not supported by libcamera, supporting > > RAW_OPAQUE format doesn't make sense. Drop the format from the > > supported format list. > > > > Signed-off-by: Hirokazu Honda <hiroh@chromium.org> > > This looks sensible to me. Given that the formats below are specific to > the IPU3, the code would need to be reworked when we'll implement ZSL > support anyway. > > Just note: For ZSL capture request, the format is either RAW_OPAQUE or IMPLEMENTATION_DEFINED. In ChromeOS, IMPLEMENTATION_DEFINED is used for ZSL. Besides, gralloc implementation in ChromeOS doesn't support RAW_OPAQUE. So we should not list this format in supported formats at least in ChromeOS. We may want to support RAW_OPAQUE for other platforms though. -Hiro > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > I'll wait for a second tag before merging. > > > --- > > src/android/camera_device.cpp | 11 ----------- > > 1 file changed, 11 deletions(-) > > > > diff --git a/src/android/camera_device.cpp > b/src/android/camera_device.cpp > > index fe332ec3..14022aed 100644 > > --- a/src/android/camera_device.cpp > > +++ b/src/android/camera_device.cpp > > @@ -120,17 +120,6 @@ const std::map<int, const Camera3Format> > camera3FormatsMap = { > > false, > > "RAW16" > > } > > - }, { > > - HAL_PIXEL_FORMAT_RAW_OPAQUE, { > > - { > > - formats::SBGGR10_IPU3, > > - formats::SGBRG10_IPU3, > > - formats::SGRBG10_IPU3, > > - formats::SRGGB10_IPU3 > > - }, > > - false, > > - "RAW_OPAQUE" > > - } > > }, > > }; > > > > -- > Regards, > > Laurent Pinchart >
Hello, On Tue, Jun 15, 2021 at 01:44:12AM +0300, Laurent Pinchart wrote: > Hi Hiro, > > Thank you for the patch. > > On Mon, Jun 14, 2021 at 07:45:53PM +0900, Hirokazu Honda wrote: > > HAL_PIXEL_FORMAT_RAW_OPAQUE is requested only for > > Zero-Shutter-Lag (ZSL). ZSL requires RAW and YUV reprocessing. > > Since either of them is not supported by libcamera, supporting > > RAW_OPAQUE format doesn't make sense. Drop the format from the > > supported format list. > > > > Signed-off-by: Hirokazu Honda <hiroh@chromium.org> > > This looks sensible to me. Given that the formats below are specific to > the IPU3, the code would need to be reworked when we'll implement ZSL > support anyway. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > I'll wait for a second tag before merging. Have mine Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Those are IPU3 specific formats, not sure they belong here > > > --- > > src/android/camera_device.cpp | 11 ----------- > > 1 file changed, 11 deletions(-) > > > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp > > index fe332ec3..14022aed 100644 > > --- a/src/android/camera_device.cpp > > +++ b/src/android/camera_device.cpp > > @@ -120,17 +120,6 @@ const std::map<int, const Camera3Format> camera3FormatsMap = { > > false, > > "RAW16" > > } > > - }, { > > - HAL_PIXEL_FORMAT_RAW_OPAQUE, { > > - { > > - formats::SBGGR10_IPU3, > > - formats::SGBRG10_IPU3, > > - formats::SGRBG10_IPU3, > > - formats::SRGGB10_IPU3 > > - }, > > - false, > > - "RAW_OPAQUE" > > - } > > }, > > }; > > > > -- > Regards, > > Laurent Pinchart
Hi Hiro, On Tue, Jun 15, 2021 at 04:44:24PM +0900, Hirokazu Honda wrote: > On Tue, Jun 15, 2021 at 7:44 AM Laurent Pinchart wrote: > > On Mon, Jun 14, 2021 at 07:45:53PM +0900, Hirokazu Honda wrote: > > > HAL_PIXEL_FORMAT_RAW_OPAQUE is requested only for > > > Zero-Shutter-Lag (ZSL). ZSL requires RAW and YUV reprocessing. > > > Since either of them is not supported by libcamera, supporting > > > RAW_OPAQUE format doesn't make sense. Drop the format from the > > > supported format list. > > > > > > Signed-off-by: Hirokazu Honda <hiroh@chromium.org> > > > > This looks sensible to me. Given that the formats below are specific to > > the IPU3, the code would need to be reworked when we'll implement ZSL > > support anyway. > > Just note: > For ZSL capture request, the format is either RAW_OPAQUE or > IMPLEMENTATION_DEFINED. What's the practical difference between the two ? I'd expect both to map to the same opaque, device-specific RAW format. > In ChromeOS, IMPLEMENTATION_DEFINED is used for ZSL. > Besides, gralloc implementation in ChromeOS doesn't support RAW_OPAQUE. > So we should not list this format in supported formats at least in ChromeOS. > We may want to support RAW_OPAQUE for other platforms though. Is there a reason why minigbm couldn't support RAW_OPAQUE ? > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > I'll wait for a second tag before merging. > > > > > --- > > > src/android/camera_device.cpp | 11 ----------- > > > 1 file changed, 11 deletions(-) > > > > > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp > > > index fe332ec3..14022aed 100644 > > > --- a/src/android/camera_device.cpp > > > +++ b/src/android/camera_device.cpp > > > @@ -120,17 +120,6 @@ const std::map<int, const Camera3Format> camera3FormatsMap = { > > > false, > > > "RAW16" > > > } > > > - }, { > > > - HAL_PIXEL_FORMAT_RAW_OPAQUE, { > > > - { > > > - formats::SBGGR10_IPU3, > > > - formats::SGBRG10_IPU3, > > > - formats::SGRBG10_IPU3, > > > - formats::SRGGB10_IPU3 > > > - }, > > > - false, > > > - "RAW_OPAQUE" > > > - } > > > }, > > > }; > > >
Hi Laurent, On Tue, Jun 15, 2021 at 7:04 PM Laurent Pinchart < laurent.pinchart@ideasonboard.com> wrote: > Hi Hiro, > > On Tue, Jun 15, 2021 at 04:44:24PM +0900, Hirokazu Honda wrote: > > On Tue, Jun 15, 2021 at 7:44 AM Laurent Pinchart wrote: > > > On Mon, Jun 14, 2021 at 07:45:53PM +0900, Hirokazu Honda wrote: > > > > HAL_PIXEL_FORMAT_RAW_OPAQUE is requested only for > > > > Zero-Shutter-Lag (ZSL). ZSL requires RAW and YUV reprocessing. > > > > Since either of them is not supported by libcamera, supporting > > > > RAW_OPAQUE format doesn't make sense. Drop the format from the > > > > supported format list. > > > > > > > > Signed-off-by: Hirokazu Honda <hiroh@chromium.org> > > > > > > This looks sensible to me. Given that the formats below are specific to > > > the IPU3, the code would need to be reworked when we'll implement ZSL > > > support anyway. > > > > Just note: > > For ZSL capture request, the format is either RAW_OPAQUE or > > IMPLEMENTATION_DEFINED. > > What's the practical difference between the two ? I'd expect both to map > to the same opaque, device-specific RAW format. > > +Tomasz Figa <tfiga@chromium.org> I am not sure honestly as it is not documented anywhere. ChromeOS camera people told me that RAW_OPAQUE is optional to support and meant for vendor app use only. Only IMPLEMENTATION_DEFINED needs to be supported for PRIVATE reprocessing. > > In ChromeOS, IMPLEMENTATION_DEFINED is used for ZSL. > > Besides, gralloc implementation in ChromeOS doesn't support RAW_OPAQUE. > > So we should not list this format in supported formats at least in > ChromeOS. > > We may want to support RAW_OPAQUE for other platforms though. > > Is there a reason why minigbm couldn't support RAW_OPAQUE ? > > It must be because we don't use the RAW_OPAQUE buffer. Thus we don't need to support it at all. -Hiro > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > > > I'll wait for a second tag before merging. > > > > > > > --- > > > > src/android/camera_device.cpp | 11 ----------- > > > > 1 file changed, 11 deletions(-) > > > > > > > > diff --git a/src/android/camera_device.cpp > b/src/android/camera_device.cpp > > > > index fe332ec3..14022aed 100644 > > > > --- a/src/android/camera_device.cpp > > > > +++ b/src/android/camera_device.cpp > > > > @@ -120,17 +120,6 @@ const std::map<int, const Camera3Format> > camera3FormatsMap = { > > > > false, > > > > "RAW16" > > > > } > > > > - }, { > > > > - HAL_PIXEL_FORMAT_RAW_OPAQUE, { > > > > - { > > > > - formats::SBGGR10_IPU3, > > > > - formats::SGBRG10_IPU3, > > > > - formats::SGRBG10_IPU3, > > > > - formats::SRGGB10_IPU3 > > > > - }, > > > > - false, > > > > - "RAW_OPAQUE" > > > > - } > > > > }, > > > > }; > > > > > > -- > Regards, > > Laurent Pinchart >
diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index fe332ec3..14022aed 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -120,17 +120,6 @@ const std::map<int, const Camera3Format> camera3FormatsMap = { false, "RAW16" } - }, { - HAL_PIXEL_FORMAT_RAW_OPAQUE, { - { - formats::SBGGR10_IPU3, - formats::SGBRG10_IPU3, - formats::SGRBG10_IPU3, - formats::SRGGB10_IPU3 - }, - false, - "RAW_OPAQUE" - } }, };
HAL_PIXEL_FORMAT_RAW_OPAQUE is requested only for Zero-Shutter-Lag (ZSL). ZSL requires RAW and YUV reprocessing. Since either of them is not supported by libcamera, supporting RAW_OPAQUE format doesn't make sense. Drop the format from the supported format list. Signed-off-by: Hirokazu Honda <hiroh@chromium.org> --- src/android/camera_device.cpp | 11 ----------- 1 file changed, 11 deletions(-)