[libcamera-devel] android: camera_device: Provide toString() helper for stream_type
diff mbox series

Message ID 20211125080518.779815-1-umang.jain@ideasonboard.com
State Accepted
Headers show
Series
  • [libcamera-devel] android: camera_device: Provide toString() helper for stream_type
Related show

Commit Message

Umang Jain Nov. 25, 2021, 8:05 a.m. UTC 
Provide a directionToString() helper to return a human-friendly name
for camera3_stream_t->stream_type. Replace the int value being printed
in configureStreams() INFO log with directionToString().

Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
---
 src/android/camera_device.cpp | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

Comments

Jacopo Mondi Nov. 25, 2021, 8:10 a.m. UTC | #1 
Hi Umang

On Thu, Nov 25, 2021 at 01:35:18PM +0530, Umang Jain wrote:
> Provide a directionToString() helper to return a human-friendly name
> for camera3_stream_t->stream_type. Replace the int value being printed
> in configureStreams() INFO log with directionToString().
>
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
>  src/android/camera_device.cpp | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index f2e0bdbd..804a71fd 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -175,6 +175,20 @@ const char *rotationToString(int rotation)
>  	return "INVALID";
>  }
>
> +const char *directionToString(int stream_type)
> +{
> +	switch (stream_type) {
> +	case CAMERA3_STREAM_OUTPUT:
> +		return "Output";
> +	case CAMERA3_STREAM_INPUT:
> +		return "Input";
> +	case CAMERA3_STREAM_BIDIRECTIONAL:
> +		return "Bidirectional";
> +	}
> +
> +	return "Unknown";

I'm not sure how this 'default' case is better handled, but I guess
it's fine for now

Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
   j

> +}
> +
>  #if defined(OS_CHROMEOS)
>  /*
>   * Check whether the crop_rotate_scale_degrees values for all streams in
> @@ -548,7 +562,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
>  		PixelFormat format = capabilities_.toPixelFormat(stream->format);
>
>  		LOG(HAL, Info) << "Stream #" << i
> -			       << ", direction: " << stream->stream_type
> +			       << ", direction: " << directionToString(stream->stream_type)
>  			       << ", width: " << stream->width
>  			       << ", height: " << stream->height
>  			       << ", format: " << utils::hex(stream->format)
> --
> 2.31.0
>
Hirokazu Honda Nov. 25, 2021, 9:01 a.m. UTC | #2 
Hi Umang,

On Thu, Nov 25, 2021 at 5:09 PM Jacopo Mondi <jacopo@jmondi.org> wrote:
>
> Hi Umang
>
> On Thu, Nov 25, 2021 at 01:35:18PM +0530, Umang Jain wrote:
> > Provide a directionToString() helper to return a human-friendly name
> > for camera3_stream_t->stream_type. Replace the int value being printed
> > in configureStreams() INFO log with directionToString().
> >
> > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> > ---
> >  src/android/camera_device.cpp | 16 +++++++++++++++-
> >  1 file changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > index f2e0bdbd..804a71fd 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -175,6 +175,20 @@ const char *rotationToString(int rotation)
> >       return "INVALID";
> >  }
> >
> > +const char *directionToString(int stream_type)
> > +{
> > +     switch (stream_type) {
> > +     case CAMERA3_STREAM_OUTPUT:
> > +             return "Output";
> > +     case CAMERA3_STREAM_INPUT:
> > +             return "Input";
> > +     case CAMERA3_STREAM_BIDIRECTIONAL:
> > +             return "Bidirectional";
> > +     }
> > +
> > +     return "Unknown";
>
> I'm not sure how this 'default' case is better handled, but I guess
> it's fine for now

Perhaps,

default:
   LOG(HAL, WARNING) << "Unknown stream type: " << stream_type;
   return "Unknown";


Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
-Hiro
>
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
>
> Thanks
>    j
>
> > +}
> > +
> >  #if defined(OS_CHROMEOS)
> >  /*
> >   * Check whether the crop_rotate_scale_degrees values for all streams in
> > @@ -548,7 +562,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
> >               PixelFormat format = capabilities_.toPixelFormat(stream->format);
> >
> >               LOG(HAL, Info) << "Stream #" << i
> > -                            << ", direction: " << stream->stream_type
> > +                            << ", direction: " << directionToString(stream->stream_type)
> >                              << ", width: " << stream->width
> >                              << ", height: " << stream->height
> >                              << ", format: " << utils::hex(stream->format)
> > --
> > 2.31.0
> >
Kieran Bingham Nov. 25, 2021, 9:40 a.m. UTC | #3 
Quoting Umang Jain (2021-11-25 08:05:18)
> Provide a directionToString() helper to return a human-friendly name
> for camera3_stream_t->stream_type. Replace the int value being printed
> in configureStreams() INFO log with directionToString().
> 

Great, Human readable is so much better.


> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
>  src/android/camera_device.cpp | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index f2e0bdbd..804a71fd 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -175,6 +175,20 @@ const char *rotationToString(int rotation)
>         return "INVALID";
>  }
>  
> +const char *directionToString(int stream_type)
> +{
> +       switch (stream_type) {
> +       case CAMERA3_STREAM_OUTPUT:
> +               return "Output";
> +       case CAMERA3_STREAM_INPUT:
> +               return "Input";
> +       case CAMERA3_STREAM_BIDIRECTIONAL:
> +               return "Bidirectional";
> +       }
> +
> +       return "Unknown";
> +}
> +
>  #if defined(OS_CHROMEOS)
>  /*
>   * Check whether the crop_rotate_scale_degrees values for all streams in
> @@ -548,7 +562,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
>                 PixelFormat format = capabilities_.toPixelFormat(stream->format);
>  
>                 LOG(HAL, Info) << "Stream #" << i
> -                              << ", direction: " << stream->stream_type
> +                              << ", direction: " << directionToString(stream->stream_type)
>                                << ", width: " << stream->width
>                                << ", height: " << stream->height
>                                << ", format: " << utils::hex(stream->format)

Is this one possible too? (Format?)


Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> -- 
> 2.31.0
>
Umang Jain Nov. 25, 2021, 11:19 a.m. UTC | #4 
Hi Kieran,

On 11/25/21 3:10 PM, Kieran Bingham wrote:
> Quoting Umang Jain (2021-11-25 08:05:18)
>> Provide a directionToString() helper to return a human-friendly name
>> for camera3_stream_t->stream_type. Replace the int value being printed
>> in configureStreams() INFO log with directionToString().
>>
> Great, Human readable is so much better.
>
>
>> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
>> ---
>>   src/android/camera_device.cpp | 16 +++++++++++++++-
>>   1 file changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
>> index f2e0bdbd..804a71fd 100644
>> --- a/src/android/camera_device.cpp
>> +++ b/src/android/camera_device.cpp
>> @@ -175,6 +175,20 @@ const char *rotationToString(int rotation)
>>          return "INVALID";
>>   }
>>   
>> +const char *directionToString(int stream_type)
>> +{
>> +       switch (stream_type) {
>> +       case CAMERA3_STREAM_OUTPUT:
>> +               return "Output";
>> +       case CAMERA3_STREAM_INPUT:
>> +               return "Input";
>> +       case CAMERA3_STREAM_BIDIRECTIONAL:
>> +               return "Bidirectional";
>> +       }
>> +
>> +       return "Unknown";
>> +}
>> +
>>   #if defined(OS_CHROMEOS)
>>   /*
>>    * Check whether the crop_rotate_scale_degrees values for all streams in
>> @@ -548,7 +562,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
>>                  PixelFormat format = capabilities_.toPixelFormat(stream->format);
>>   
>>                  LOG(HAL, Info) << "Stream #" << i
>> -                              << ", direction: " << stream->stream_type
>> +                              << ", direction: " << directionToString(stream->stream_type)
>>                                 << ", width: " << stream->width
>>                                 << ", height: " << stream->height
>>                                 << ", format: " << utils::hex(stream->format)
> Is this one possible too? (Format?)


Yes, good candidate as well. I can spot more cases for this

         DEBUG HAL camera_capabilities.cpp:1788 Output Stream: 
0x00000022 (320x240)[33333333]@30
         DEBUG HAL camera_capabilities.cpp:1788 Output Stream: 
0x00000022 (640x480)[33333333]@30
         DEBUG HAL camera_capabilities.cpp:1788 Output Stream: 
0x00000022 (1280x720)[33333333]@30
         DEBUG HAL camera_capabilities.cpp:1788 Output Stream: 
0x00000022 (1920x1080)[33333333]@30
         DEBUG HAL camera_capabilities.cpp:1788 Output Stream: 
0x00000022 (4160x3104)[33338000]@30
         DEBUG HAL camera_capabilities.cpp:1788 Output Stream: 
0x00000023 (320x240)[33333333]@30

So better to address them all in a separate patch. A formatsMap_ already 
lives in camera_capabilities as far as I can see, we can make use of in 
some form

>
>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>
>> -- 
>> 2.31.0
>>

Patch
diff mbox series 

diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index f2e0bdbd..804a71fd 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -175,6 +175,20 @@  const char *rotationToString(int rotation)
 	return "INVALID";
 }
 
+const char *directionToString(int stream_type)
+{
+	switch (stream_type) {
+	case CAMERA3_STREAM_OUTPUT:
+		return "Output";
+	case CAMERA3_STREAM_INPUT:
+		return "Input";
+	case CAMERA3_STREAM_BIDIRECTIONAL:
+		return "Bidirectional";
+	}
+
+	return "Unknown";
+}
+
 #if defined(OS_CHROMEOS)
 /*
  * Check whether the crop_rotate_scale_degrees values for all streams in
@@ -548,7 +562,7 @@  int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
 		PixelFormat format = capabilities_.toPixelFormat(stream->format);
 
 		LOG(HAL, Info) << "Stream #" << i
-			       << ", direction: " << stream->stream_type
+			       << ", direction: " << directionToString(stream->stream_type)
 			       << ", width: " << stream->width
 			       << ", height: " << stream->height
 			       << ", format: " << utils::hex(stream->format)