On Thu, Aug 06, 2020 at 03:22:34PM +0200, Hans Verkuil wrote: > On 06/08/2020 14:54, Sakari Ailus wrote: > > Hi Hans, > > > > On Thu, Aug 06, 2020 at 11:58:31AM +0200, Hans Verkuil wrote: > >> On 06/08/2020 11:50, Jacopo Mondi wrote: > >>> Hi Hans, > >>> > >>> On Thu, Aug 06, 2020 at 10:05:37AM +0200, Hans Verkuil wrote: > >>>> Hi Jacopo, > >>>> > >>>> Some review comments below: > >>>> > >>>> On 05/08/2020 12:57, Jacopo Mondi wrote: > >>>>> +Analog crop rectangle > >>>> > >>>> Why analog? It's just the crop rectangle, nothing analog about it. > >>>> > >>> > >>> We used the 'analogCrop' term in libcamera to differentiate the > >>> cropping which happens on the sensor pixel array matrix to select the > >>> region to process and produce image from. Sensor with an on-board > >>> scaler can perform other cropping steps to implement, in example digital > >>> zoom, so we expect to have a 'digital crop' phase as well. RAW > >>> sensors, in example, will only have an analogCrop rectangle. > >>> > >>> Quoting the libcamera definition of analog crop: > >>> > >>> * horizontal and vertical sizes define the portion of the pixel array which > >>> * is read-out and provided to the sensor's internal processing pipeline, before > >>> * any pixel sub-sampling method, such as pixel binning, skipping and averaging > >>> * take place. > >>> > >>> should I keep it or remove it ? > >> > >> It's a very confusing term. Especially since this API can also be used with analog > >> video capture devices (Composite/S-Video) where the video signal actually is analog. > >> > >> In the V4L2 API there is no such thing as 'analog crop', so please remove it. > > > > There isn't in the V4L2 API but I don't see why we couldn't document it > > here. Analogue crop is an established term related to raw (perhaps others, > > too?) camera sensors which describes cropping that is implemented by not > > reading parts of the pixel array. > > > > As this documentation only applies to camera sensors, I think it's entirely > > appropriate to document this here, and using that term. > > > > It's always been called just 'crop' in the V4L2 API, so renaming it suddenly > to something else is IMHO confusing. What you can do, however, is that in the This has been actually implemented a decade ago but it seems the documentation has either never been there or has disappeared. Most drivers hide this as they work on the frame interval and output size alone, ignoring the rest. Despite that, generally camera sensors do have both analogue and digital cropping capabilities with differing features (granularity and dependency to frame interval). > description of the "crop rectangle" you mention that "it is also known as > "analog crop" in the context of camera sensors. Just saying it's a crop rectangle isn't exactly wrong but it's incomplete. The frame interval calculation requires that information so this should be more than just a side note. > > With perhaps some more extensive explanation of the term. -- Sakari Ailus
