[v2,3/4] virtual: Validate stride and frameSize
diff mbox series

Message ID 20261009194007.23175-4-robert.mader@collabora.com
State Superseded
Headers show
Series
  • virtual: Support more common pixel formats
Related show

Commit Message

Robert Mader Oct. 9, 2026, 7:40 p.m. UTC
If a client requested a specific stride that's incompatible with what
we'll produce, let the client know.

Signed-off-by: Robert Mader <robert.mader@collabora.com>
---
 src/libcamera/pipeline/virtual/virtual.cpp | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

Comments

Kieran Bingham Oct. 12, 2026, 1:28 p.m. UTC | #1
Quoting Robert Mader (2026-10-09 20:40:06)
> If a client requested a specific stride that's incompatible with what
> we'll produce, let the client know.
> 
> Signed-off-by: Robert Mader <robert.mader@collabora.com>
> ---
>  src/libcamera/pipeline/virtual/virtual.cpp | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp
> index 7ed2d0b7f596..6e97981b0f40 100644
> --- a/src/libcamera/pipeline/virtual/virtual.cpp
> +++ b/src/libcamera/pipeline/virtual/virtual.cpp
> @@ -240,13 +240,20 @@ CameraConfiguration::Status VirtualCameraConfiguration::validate()
>                         adjusted = true;
>                 }
>  
> +               auto stride = info.stride(cfg.size.width, 0);
> +               auto frameSize = info.frameSize(cfg.size);
> +               if ((cfg.stride && cfg.stride != stride) ||
> +                   (cfg.frameSize && cfg.frameSize != frameSize)) {
> +                       status = Adjusted;
> +                       adjusted = true;
> +               }
> +               cfg.stride = stride;
> +               cfg.frameSize = frameSize;

Can the application ask for a specific stride here at all?

-=-
Kieran

> +
>                 if (adjusted)
>                         LOG(Virtual, Info)
>                                 << "Stream configuration adjusted to " << cfg.toString();
>  
> -               cfg.stride = info.stride(cfg.size.width, 0, 1);
> -               cfg.frameSize = info.frameSize(cfg.size, 1);
> -
>                 cfg.bufferCount = VirtualCameraConfiguration::kBufferCount;
>         }
>  
> -- 
> 2.56.0
>
Barnabás Pőcze Oct. 12, 2026, 2:18 p.m. UTC | #2
2026. 10. 12. 15:28 keltezéssel, Kieran Bingham írta:
> Quoting Robert Mader (2026-10-09 20:40:06)
>> If a client requested a specific stride that's incompatible with what
>> we'll produce, let the client know.
>>
>> Signed-off-by: Robert Mader <robert.mader@collabora.com>
>> ---
>>   src/libcamera/pipeline/virtual/virtual.cpp | 13 ++++++++++---
>>   1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp
>> index 7ed2d0b7f596..6e97981b0f40 100644
>> --- a/src/libcamera/pipeline/virtual/virtual.cpp
>> +++ b/src/libcamera/pipeline/virtual/virtual.cpp
>> @@ -240,13 +240,20 @@ CameraConfiguration::Status VirtualCameraConfiguration::validate()
>>                          adjusted = true;
>>                  }
>>   
>> +               auto stride = info.stride(cfg.size.width, 0);
>> +               auto frameSize = info.frameSize(cfg.size);
>> +               if ((cfg.stride && cfg.stride != stride) ||
>> +                   (cfg.frameSize && cfg.frameSize != frameSize)) {
>> +                       status = Adjusted;
>> +                       adjusted = true;
>> +               }
>> +               cfg.stride = stride;
>> +               cfg.frameSize = frameSize;
> 
> Can the application ask for a specific stride here at all?

No. I think the main obstacle is getting per-plane stride information
into `StreamConfiguration` and `FrameBuffer`. Otherwise things are
a bit fragile wrt. what exactly are the strides of the later planes.

Or alternatively v4l2 compatible stride calculation should be implemented
in `PixelFormat` for the later planes, because currently there does not
appear to be an implementation in libcamera. And then the per-plane strides
could possibly be postponed.

https://gitlab.freedesktop.org/camera/libcamera/-/work_items/360
https://gitlab.freedesktop.org/camera/libcamera/-/work_items/9



> 
> -=-
> Kieran
> 
>> +
>>                  if (adjusted)
>>                          LOG(Virtual, Info)
>>                                  << "Stream configuration adjusted to " << cfg.toString();
>>   
>> -               cfg.stride = info.stride(cfg.size.width, 0, 1);
>> -               cfg.frameSize = info.frameSize(cfg.size, 1);
>> -
>>                  cfg.bufferCount = VirtualCameraConfiguration::kBufferCount;
>>          }
>>   
>> -- 
>> 2.56.0
>>

Patch
diff mbox series

diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp
index 7ed2d0b7f596..6e97981b0f40 100644
--- a/src/libcamera/pipeline/virtual/virtual.cpp
+++ b/src/libcamera/pipeline/virtual/virtual.cpp
@@ -240,13 +240,20 @@  CameraConfiguration::Status VirtualCameraConfiguration::validate()
 			adjusted = true;
 		}
 
+		auto stride = info.stride(cfg.size.width, 0);
+		auto frameSize = info.frameSize(cfg.size);
+		if ((cfg.stride && cfg.stride != stride) ||
+		    (cfg.frameSize && cfg.frameSize != frameSize)) {
+			status = Adjusted;
+			adjusted = true;
+		}
+		cfg.stride = stride;
+		cfg.frameSize = frameSize;
+
 		if (adjusted)
 			LOG(Virtual, Info)
 				<< "Stream configuration adjusted to " << cfg.toString();
 
-		cfg.stride = info.stride(cfg.size.width, 0, 1);
-		cfg.frameSize = info.frameSize(cfg.size, 1);
-
 		cfg.bufferCount = VirtualCameraConfiguration::kBufferCount;
 	}