[libcamera-devel] [PATCH 2/5] libcamera: ipu3: Return video output default configuration
Jacopo Mondi
jacopo at jmondi.org
Fri Feb 22 12:08:42 CET 2019
Hi Kieran, Laurent,
On Fri, Feb 22, 2019 at 10:57:35AM +0000, Kieran Bingham wrote:
> Hi Jacopo,
>
> On 22/02/2019 10:33, Jacopo Mondi wrote:
> > Hi Jerry,
> > thanks for your comment
> >
> > On Fri, Feb 22, 2019 at 02:30:09AM +0000, Hu, Jerry W wrote:
> >> Hi,
> >>
> >> May I know why hard coded the width and height?
> >>>> + config.width = 2560;
> >>>> + config.height = 1920;
> >>>> + config.pixelFormat = V4L2_PIX_FMT_NV12;
> >>
> >
> >
> > This happens in the 'streamConfiguration()' methods, which
> > applications use to obtain a known good default resolution and image
> > format the camera can provide. In this case I've selected 2560x1920
> > NV12 format as "default" for the single stream the IPU3 camera
> > provides at the moment.
> >
> > Once application receives a known default format it is then free to
> > change any parameter of that, and provide it to libcamera using the
> > Camera::configureStreams() methods. The Camera instance will pass the
> > configurations to the pipeline handlers, that wil try to apply it to
> > the hardware.
> >
> > To sum up: we need to provide to applications a default configuration
> > suitable for the hardware, and that's why we hardcode those values
> > here. Starting from that default configuration, applications can
> > configure streams with the desired width, height and pixel format and
> > ask libcamera to apply it to the hardware.
> >
> > Does this clarify your concerns?
>
> I think it was discussed somewhere but I'm not sure where, but what is
> the reason why we can't ask the hardware for it's "current" format to
> set the 'default' ?
>
Because at the current stage the 'hardware' is both the sensor
provided resolution and the output resolution provided by the ImgU,
which do not match, if we use their defaults. And as images will be
provided from ImgU not the CIO2 as before, I cannot just claim the
format applied on the sensor and report that.
That resolution is known to work for the current use case, and I put
it there not to break capture. Later on, with full ImgU support should
westart from the sensor resolution and adjust it to one the ImgU can
provide, or start from the default one applied to the ImgU unit, and
make sure the sensor can provide something big enough to fit into
that?
Also, I know realize this was probably what Jerry was asking in is
email, not an explanation of the stream format negotiation
procedure...
> I feel like anything we define statically in the pipeline is just as
> wrong as any initial default provided by the hardware ..
>
Why do you consider initial defaults provided by the hardware wrong? I
mean, the issue here to me is "which default", but somewhere it has to
be defined, right?
Thanks
j
> --
> Kieran
>
>
>
>
> > Thanks
> > j
> >
> >> Thanks,
> >> -Jerry
> >>
> >> -----Original Message-----
> >> From: libcamera-devel [mailto:libcamera-devel-bounces at lists.libcamera.org] On Behalf Of Jacopo Mondi
> >> Sent: Friday, February 22, 2019 12:21 AM
> >> To: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> >> Cc: libcamera-devel at lists.libcamera.org
> >> Subject: Re: [libcamera-devel] [PATCH 2/5] libcamera: ipu3: Return video output default configuration
> >>
> >> Hi Niklas,
> >>
> >> On Thu, Feb 21, 2019 at 04:58:35PM +0100, Niklas Söderlund wrote:
> >>> Hi Jacopo,
> >>>
> >>> Thanks for your patch.
> >>>
> >>> On 2019-02-20 14:17:54 +0100, Jacopo Mondi wrote:
> >>>> Return default configuration for the output stream produced by the
> >>>> imgu 'output' video node.
> >>>>
> >>>> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> >>>> ---
> >>>> src/libcamera/pipeline/ipu3/ipu3.cpp | 16 +++-------------
> >>>> 1 file changed, 3 insertions(+), 13 deletions(-)
> >>>>
> >>>> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp
> >>>> b/src/libcamera/pipeline/ipu3/ipu3.cpp
> >>>> index 9694d0ce51ab..9065073913a2 100644
> >>>> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> >>>> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> >>>> @@ -99,21 +99,11 @@ PipelineHandlerIPU3::streamConfiguration(Camera
> >>>> *camera, {
> >>>> IPU3CameraData *data = cameraData(camera);
> >>>> std::map<Stream *, StreamConfiguration> configs;
> >>>> - V4L2SubdeviceFormat format = {};
> >>>> -
> >>>> - /*
> >>>> - * FIXME: As of now, return the image format reported by the sensor.
> >>>> - * In future good defaults should be provided for each stream.
> >>>> - */
> >>>> - if (data->sensor_->getFormat(0, &format)) {
> >>>> - LOG(IPU3, Error) << "Failed to create stream configurations";
> >>>> - return configs;
> >>>> - }
> >>>>
> >>>> StreamConfiguration config = {};
> >>>> - config.width = format.width;
> >>>> - config.height = format.height;
> >>>> - config.pixelFormat = V4L2_PIX_FMT_IPU3_SGRBG10;
> >>>> + config.width = 2560;
> >>>> + config.height = 1920;
> >>>> + config.pixelFormat = V4L2_PIX_FMT_NV12;
> >>>
> >>> This is a good change and trust that the format you selected are the
> >>> correct one so I have not verified it by reading or testing on the IPU3.
> >>>
> >>> Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> >>>
> >>> I'm just curious why the pixelformat changes, did something change in
> >>> the upstream driver or is this in preparation of some later change?
> >>>
> >>
> >> We're preparing to provide frames to the application from the IMGU not from the CIO2 unit. The output format of the two is different.
> >>
> >> Thanks
> >> j
> >>
> >>>> config.bufferCount = 4;
> >>>>
> >>>> configs[&data->stream_] = config;
> >>>> --
> >>>> 2.20.1
> >>>>
> >>>> _______________________________________________
> >>>> libcamera-devel mailing list
> >>>> libcamera-devel at lists.libcamera.org
> >>>> https://lists.libcamera.org/listinfo/libcamera-devel
> >>>
> >>> --
> >>> Regards,
> >>> Niklas Söderlund
> >>>
> >>> _______________________________________________
> >>> libcamera-devel mailing list
> >>> libcamera-devel at lists.libcamera.org
> >>> https://lists.libcamera.org/listinfo/libcamera-devel
>
> --
> Regards
> --
> Kieran
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20190222/1d79923b/attachment.sig>
More information about the libcamera-devel
mailing list