[libcamera-devel] [PATCH v2 2/2] android: Log stream types using PixelFormat::toString()

[Umang Jain]

Hi

On 11/25/21 7:46 PM, Kieran Bingham wrote:
> Quoting Laurent Pinchart (2021-11-25 13:49:48)
>> On Thu, Nov 25, 2021 at 01:28:11PM +0000, Kieran Bingham wrote:
>>> Quoting Umang Jain (2021-11-25 13:12:24)
>>>> Log stream types using PixelFormat::toString() instead of hex values,
>>>> since that is more human readable.
>>>>
>>>> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
>>>> ---
>>>>   src/android/camera_capabilities.cpp | 5 +++--
>>>>   src/android/camera_device.cpp       | 2 +-
>>>>   2 files changed, 4 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
>>>> index f357902e..71f302c4 100644
>>>> --- a/src/android/camera_capabilities.cpp
>>>> +++ b/src/android/camera_capabilities.cpp
>>>> @@ -713,7 +713,8 @@ int CameraCapabilities::initializeStreamConfigurations()
>>>>          LOG(HAL, Debug) << "Collected stream configuration map: ";
>>>>          for (const auto &entry : streamConfigurations_)
>>>>                  LOG(HAL, Debug) << "{ " << entry.resolution.toString() << " - "
>>>> -                               << utils::hex(entry.androidFormat) << " }";
>>>> +                               << toPixelFormat(entry.androidFormat).toString()
>>> Do all androidFormats get correctly converted to libcamera formats
>>> currently?
>>>
>>> This might still need to print the Android format, or have an android
>>> specific string function... (Or print both the android hex string, and
>>> our representation of it?)
>> That's also my concern. If we receive from the camera service a format
>> we don't support, printing an "unknown" or similar name hinders
>> debugging. I'd rather keep the android format in hex in the locations
>> before we validate it.
> I think we should have both. Or only print the hex if it's unknown ...


In that case, the hex is printed if the format is unknown as part of 
Error logging. It is done through CameraCapabilites::toPixelFormat()

>
> --
> Kieran
>
>
>>>> +                               << " }";
>>>>   
>>>>          return 0;
>>>>   }
>>>> @@ -1303,7 +1304,7 @@ int CameraCapabilities::initializeStaticMetadata()
>>>>                  minFrameDurations.push_back(entry.minFrameDurationNsec);
>>>>   
>>>>                  LOG(HAL, Debug)
>>>> -                       << "Output Stream: " << utils::hex(entry.androidFormat)
>>>> +                       << "Output Stream: " << toPixelFormat(entry.androidFormat).toString()
>>>>                          << " (" << entry.resolution.toString() << ")["
>>>>                          << entry.minFrameDurationNsec << "]"
>>>>                          << "@" << fps;
>>>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
>>>> index ad4bf07c..cc6dccc1 100644
>>>> --- a/src/android/camera_device.cpp
>>>> +++ b/src/android/camera_device.cpp
>>>> @@ -566,7 +566,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
>>>>                                 << ", direction: " << directionToString(stream->stream_type)
>>>>                                 << ", width: " << stream->width
>>>>                                 << ", height: " << stream->height
>>>> -                              << ", format: " << utils::hex(stream->format)
>>>> +                              << ", format: " << format.toString()


Also, the 'format' here comes from

         PixelFormat format = capabilities_.toPixelFormat(stream->format);

a few lines above.

>>>>                                 << ", rotation: " << rotationToString(stream->rotation)
>>>>   #if defined(OS_CHROMEOS)
>>>>                                 << ", crop_rotate_scale_degrees: "
>> -- 
>> Regards,
>>
>> Laurent Pinchart