| Message ID | 20210105123128.617543-8-jacopo@jmondi.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, Thank you for the patch. On Tue, Jan 05, 2021 at 01:31:25PM +0100, Jacopo Mondi wrote: > Initialize the pixel array properties in the UVC pipeline handler as > they're now initialized in the CameraSensor class, which the UVC > pipeline handler does not ue. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > index 7cb310e20511..1efced949762 100644 > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > @@ -509,6 +509,19 @@ int UVCCameraData::init(MediaDevice *media) > properties_.set(properties::Location, properties::CameraLocationExternal); > properties_.set(properties::Model, utils::toAscii(media->model())); > > + /* > + * Get the current format in order to initialize the sensor array > + * properties. > + */ Could we use the largest format instead ? Otherwise, if the current format isn't the largest, we'll end up with a pixel array size that is smaller than what the camera could produce. Other than that this is exactly what I had in mind. > + V4L2DeviceFormat format; > + ret = video_->getFormat(&format); > + if (ret) > + return ret; > + > + properties_.set(properties::PixelArraySize, format.size); > + properties_.set(properties::PixelArrayActiveAreas, > + { Rectangle(format.size) }); > + > /* Initialise the supported controls. */ > ControlInfoMap::Map ctrls; >
Hi Laurent, On Tue, Jan 05, 2021 at 02:51:25PM +0200, Laurent Pinchart wrote: > Hi Jacopo, > > Thank you for the patch. > > On Tue, Jan 05, 2021 at 01:31:25PM +0100, Jacopo Mondi wrote: > > Initialize the pixel array properties in the UVC pipeline handler as > > they're now initialized in the CameraSensor class, which the UVC > > pipeline handler does not ue. > > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > --- > > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 13 +++++++++++++ > > 1 file changed, 13 insertions(+) > > > > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > > index 7cb310e20511..1efced949762 100644 > > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > > @@ -509,6 +509,19 @@ int UVCCameraData::init(MediaDevice *media) > > properties_.set(properties::Location, properties::CameraLocationExternal); > > properties_.set(properties::Model, utils::toAscii(media->model())); > > > > + /* > > + * Get the current format in order to initialize the sensor array > > + * properties. > > + */ > > Could we use the largest format instead ? Otherwise, if the current > format isn't the largest, we'll end up with a pixel array size that is > smaller than what the camera could produce. > I (wrongly) assumed the default size is usually the largest available one. Would something like: --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp @@ -8,6 +8,7 @@ #include <algorithm> #include <fstream> #include <iomanip> +#include <limits> #include <math.h> #include <memory> #include <tuple> @@ -514,6 +515,8 @@ int UVCCameraData::init(MediaDevice *media) * properties. */ V4L2DeviceFormat format; + format.size.width = std::numeric_limits<unsigned int>::max(); + format.size.height = format.size.width; ret = video_->getFormat(&format); if (ret) return ret; work better ? > Other than that this is exactly what I had in mind. > > > + V4L2DeviceFormat format; > > + ret = video_->getFormat(&format); > > + if (ret) > > + return ret; > > + > > + properties_.set(properties::PixelArraySize, format.size); > > + properties_.set(properties::PixelArrayActiveAreas, > > + { Rectangle(format.size) }); > > + > > /* Initialise the supported controls. */ > > ControlInfoMap::Map ctrls; > > > > -- > Regards, > > Laurent Pinchart
Hi Jacopo, On Tue, Jan 05, 2021 at 02:45:03PM +0100, Jacopo Mondi wrote: > On Tue, Jan 05, 2021 at 02:51:25PM +0200, Laurent Pinchart wrote: > > On Tue, Jan 05, 2021 at 01:31:25PM +0100, Jacopo Mondi wrote: > > > Initialize the pixel array properties in the UVC pipeline handler as > > > they're now initialized in the CameraSensor class, which the UVC > > > pipeline handler does not ue. > > > > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > > --- > > > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 13 +++++++++++++ > > > 1 file changed, 13 insertions(+) > > > > > > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > > > index 7cb310e20511..1efced949762 100644 > > > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > > > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > > > @@ -509,6 +509,19 @@ int UVCCameraData::init(MediaDevice *media) > > > properties_.set(properties::Location, properties::CameraLocationExternal); > > > properties_.set(properties::Model, utils::toAscii(media->model())); > > > > > > + /* > > > + * Get the current format in order to initialize the sensor array > > > + * properties. > > > + */ > > > > Could we use the largest format instead ? Otherwise, if the current > > format isn't the largest, we'll end up with a pixel array size that is > > smaller than what the camera could produce. > > > > I (wrongly) assumed the default size is usually the largest available > one. > > Would something like: > > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > @@ -8,6 +8,7 @@ > #include <algorithm> > #include <fstream> > #include <iomanip> > +#include <limits> > #include <math.h> > #include <memory> > #include <tuple> > @@ -514,6 +515,8 @@ int UVCCameraData::init(MediaDevice *media) > * properties. > */ > V4L2DeviceFormat format; > + format.size.width = std::numeric_limits<unsigned int>::max(); > + format.size.height = format.size.width; > ret = video_->getFormat(&format); > if (ret) > return ret; > > work better ? That won't change anything, getFormat() retrieves the current format, the format argument is output only. tryFormat() would be an option, but it will only give you the largest size for the default pixel format. The best option is to retrieve the largest size from video_->formats(). > > Other than that this is exactly what I had in mind. > > > > > + V4L2DeviceFormat format; > > > + ret = video_->getFormat(&format); > > > + if (ret) > > > + return ret; > > > + > > > + properties_.set(properties::PixelArraySize, format.size); > > > + properties_.set(properties::PixelArrayActiveAreas, > > > + { Rectangle(format.size) }); > > > + > > > /* Initialise the supported controls. */ > > > ControlInfoMap::Map ctrls; > > >
Hi Laurent, On Thu, Jan 07, 2021 at 05:09:26AM +0200, Laurent Pinchart wrote: > Hi Jacopo, > > On Tue, Jan 05, 2021 at 02:45:03PM +0100, Jacopo Mondi wrote: > > On Tue, Jan 05, 2021 at 02:51:25PM +0200, Laurent Pinchart wrote: > > > On Tue, Jan 05, 2021 at 01:31:25PM +0100, Jacopo Mondi wrote: > > > > Initialize the pixel array properties in the UVC pipeline handler as > > > > they're now initialized in the CameraSensor class, which the UVC > > > > pipeline handler does not ue. > > > > > > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > > > --- > > > > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 13 +++++++++++++ > > > > 1 file changed, 13 insertions(+) > > > > > > > > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > > > > index 7cb310e20511..1efced949762 100644 > > > > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > > > > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > > > > @@ -509,6 +509,19 @@ int UVCCameraData::init(MediaDevice *media) > > > > properties_.set(properties::Location, properties::CameraLocationExternal); > > > > properties_.set(properties::Model, utils::toAscii(media->model())); > > > > > > > > + /* > > > > + * Get the current format in order to initialize the sensor array > > > > + * properties. > > > > + */ > > > > > > Could we use the largest format instead ? Otherwise, if the current > > > format isn't the largest, we'll end up with a pixel array size that is > > > smaller than what the camera could produce. > > > > > > > I (wrongly) assumed the default size is usually the largest available > > one. > > > > Would something like: > > > > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp > > @@ -8,6 +8,7 @@ > > #include <algorithm> > > #include <fstream> > > #include <iomanip> > > +#include <limits> > > #include <math.h> > > #include <memory> > > #include <tuple> > > @@ -514,6 +515,8 @@ int UVCCameraData::init(MediaDevice *media) > > * properties. > > */ > > V4L2DeviceFormat format; > > + format.size.width = std::numeric_limits<unsigned int>::max(); > > + format.size.height = format.size.width; > > ret = video_->getFormat(&format); > > if (ret) > > return ret; > > > > work better ? > > That won't change anything, getFormat() retrieves the current format, > the format argument is output only. tryFormat() would be an option, but > it will only give you the largest size for the default pixel format. The > best option is to retrieve the largest size from video_->formats(). Ouch, I mean 'setFormat()' with a V4L2DeviceFormat with sizes = MAX_INT. But as you suggested, that would work for the current pixelformat only. Looking into the formats() map might be a better option. Thanks j > > > > Other than that this is exactly what I had in mind. > > > > > > > + V4L2DeviceFormat format; > > > > + ret = video_->getFormat(&format); > > > > + if (ret) > > > > + return ret; > > > > + > > > > + properties_.set(properties::PixelArraySize, format.size); > > > > + properties_.set(properties::PixelArrayActiveAreas, > > > > + { Rectangle(format.size) }); > > > > + > > > > /* Initialise the supported controls. */ > > > > ControlInfoMap::Map ctrls; > > > > > > -- > Regards, > > Laurent Pinchart
diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp index 7cb310e20511..1efced949762 100644 --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp @@ -509,6 +509,19 @@ int UVCCameraData::init(MediaDevice *media) properties_.set(properties::Location, properties::CameraLocationExternal); properties_.set(properties::Model, utils::toAscii(media->model())); + /* + * Get the current format in order to initialize the sensor array + * properties. + */ + V4L2DeviceFormat format; + ret = video_->getFormat(&format); + if (ret) + return ret; + + properties_.set(properties::PixelArraySize, format.size); + properties_.set(properties::PixelArrayActiveAreas, + { Rectangle(format.size) }); + /* Initialise the supported controls. */ ControlInfoMap::Map ctrls;
Initialize the pixel array properties in the UVC pipeline handler as they're now initialized in the CameraSensor class, which the UVC pipeline handler does not ue. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+)