Add a new OutputWindows control#346
Conversation
Ensure platform specific controls are advertised as still being available after calling configure(). Additionally, provide the correct defaults for the platform specific ScalerCrops control, even if there's only a single stream. This is more convenient for applications which no longer have to perform different actions depending on whether it's available. Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
We sometimes need to know which stream (according to its index in the camera configuration) corresponds to which ISP output branch (0 or 1). Previously this was stored within the cropParams_ (for use by the ScalerCrops controls). Here we remove the ISP index to its own array, making it easier for other functions to refer to it in future. Additionally, when implementing cropping, we rename the "index" variable to "ispIndex" for future clarity. Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
This control allows the camera images to occupy only a smaller "window" within the overall output. It is principally intended for machine learning applications that might want to create letterboxed versions of the camera image directly in a larger square output. Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
Add a small OutputInfo structure to store some extra useful information about the output buffers, such as the pixel alignment required for output format, and the minimum image width we can make before having to resort to software downscaling. This information will be used by future features. Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
We add the OutputWindows control and apply it to the pipeline at the same time as the ScalerCrops (the applyScalerCrops function is renamed to applyScalerCropsAndWindows). The platform specific implementation for the Pi5/PiSP checks the validity (size and alignment) of the output windows using the extra OutputInfo that is now available. We only allow modification of the output window when no software downscaling is required, as this limits the amount of pipeline reconfiguration we might need to perform at runtime (though we could revisit this at a later date). Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
|
@naushir Not a thing to merge, but would appreciate any feedback before I consider trying to upstream! |
|
We already support offsets (in FrameBuffer::Plane) and stride, I'd rather use that than a new control. |
Hi Laurent, that's an interesting point. I think there are a couple of slightly different things to think about here, namely the underlying implementation of such a feature, and its API.
There's clearly going to be an "offset" somewhere, but I wasn't really too sure how the one in the FrameBuffer::Plane is meant to be used. The comments/documentation that I saw (e.g. here) left me unclear whether this was its intended purpose, or whether V4L2 drivers would see the offsets (and I'm suspicious our drivers would ignore them in any case). So would there have to be some other mechanism actually to apply them? Anyway, it would be nice to understand this better.
So I was keen to avoid an API where users have to worry about strides and offset calculations for themselves (especially where multiplanar formats are concerned). My first reaction was to put an (x,y) offset into the StreamConfiguration, and I think this is quite natural and could work well. However, I was bothered that it would be regarded a rather niche use case which is why I shied away and made it a Pi only thing. But a scheme like this, where you can only set the offsets at configuration time, should be easier to implement, so I'd be happy to do a version of that if it was regarded as preferable. |
The FrameBuffer::Plane::offset field is meant to be set by the user. It may not flow correctly all the way to the hardware in the existing kernel and userspace implementation, but that would "just" be a bug.
What bothers me is that we would then have two ways to achieve the same result (through FrameBuffer::Plane::offset and through an offset in StreamConfiguration, or a control as proposed here). When that happens, interactions between the two options are usually poorly documented and implemented differently in pipeline handlers, causing headaches for developers. Looking at this patch series, the definition of the new control mentions resizing, and does not define how it interacts with ScalerCrop. This is also an issue. I think this needs to go back to the drawing board to design a solution easier to understand and implement. |
2 participants
This control lets you direct the camera image at only a smaller "window" within the normal output image.
It is intended primarily for ML applications that might want to create a square image but where the camera output is "letterboxed" within it.