[libcamera-devel] [PATCH v4] libcamera: v4l2PixelFormat: Replace hex with fourCC

[Kieran Bingham]

Hi Laurent,

On 27/03/2020 15:13, Laurent Pinchart wrote:
> Hi Kaaira,
> 
> Thank you for the patch.
> 
> On Fri, Mar 27, 2020 at 01:44:00AM +0530, Kaaira Gupta wrote:
>> Print fourCC characters instead of the hex value in toString() as they are
>> more informative. Also, write the tests for this in formats.cpp
>>
>> Signed-off-by: Kaaira Gupta <kgupta at es.iitr.ac.in>
>> ---
>>
>> Changes since v3:
>> 	- Reformatted the code
>> 	- Changed 'invalid' to "<INVALID>"
>> 	- Changed cerr message.
>>
>> Changes since v2:
>>         - reformatted the code
>>         - added cerr messages before TestFail
>>         - Changed test case to accomodate edge case
>>         - Increased buffer size of char array
>>         - Changed the maximum length of for loop.
>>
>> Changes since v1:
>>         - Add tests for checking this function.
>>         - use char[] instead of stringstream.
>>         - add checks for default value.
>>         - Print '.' for non-printable characters.
>>
>>  src/libcamera/v4l2_videodevice.cpp | 20 +++++++++++++++++---
>>  test/v4l2_videodevice/formats.cpp  | 22 ++++++++++++++++++++++
>>  2 files changed, 39 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
>> index b778181..eb33a68 100644
>> --- a/src/libcamera/v4l2_videodevice.cpp
>> +++ b/src/libcamera/v4l2_videodevice.cpp
>> @@ -336,9 +336,23 @@ bool V4L2BufferCache::Entry::operator==(const FrameBuffer &buffer) const
>>   */
>>  std::string V4L2PixelFormat::toString() const
>>  {
>> -	char str[11];
>> -	snprintf(str, 11, "0x%08x", fourcc_);
>> -	return str;
>> +	if (fourcc_ == 0)
>> +		return "<INVALID>";
>> +
>> +	char ss[8] = { static_cast<char>(fourcc_ & 0x7f),
>> +		       static_cast<char>((fourcc_ >> 8) & 0x7f),
>> +		       static_cast<char>((fourcc_ >> 16) & 0x7f),
>> +		       static_cast<char>((fourcc_ >> 24) & 0x7f) };
>> +
>> +	for (unsigned int i = 0; i < 4; i++) {
>> +		if (!isprint(ss[i]))
>> +			ss[i] = '.';
>> +	}
>> +
>> +	if (fourcc_ & (1 << 31))
>> +		strcat(ss, "-BE");
>> +
>> +	return ss;
>>  }
>>  
>>  /**
>> diff --git a/test/v4l2_videodevice/formats.cpp b/test/v4l2_videodevice/formats.cpp
>> index d504d17..d01eccd 100644
>> --- a/test/v4l2_videodevice/formats.cpp
>> +++ b/test/v4l2_videodevice/formats.cpp
>> @@ -47,6 +47,28 @@ protected:
>>  			return TestFail;
>>  		}
>>  
>> +		std::vector<std::pair<uint32_t, const char*>> formats{
>> +			{ V4L2_PIX_FMT_YUYV, "YUYV" },
>> +			{ 0, "<INVALID>" },
>> +			{ v4l2_fourcc(0, 1, 2, 3), "...." },
>> +			{ V4L2_PIX_FMT_Y16_BE, "Y16 -BE" }
>> +		};
>> +
>> +		for (const auto &format : formats) {
>> +			if (V4L2PixelFormat(format.first).toString() != format.second) {
>> +				cerr << "Failed to convert V4L2PixelFormat"
>> +				     << utils::hex(format.first) << "to string"
> 
> This file should include "utils.h". With this fixed,
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> 
> Kieran, can you fix that when applying ?

Done.

And pushed to master.

--
Kieran

> 
>> +				     << endl;
>> +				return TestFail;
>> +			}
>> +		}
>> +
>> +		if (V4L2PixelFormat().toString() != "<INVALID>") {
>> +			cerr << "Failed to convert default V4L2PixelFormat to string"
>> +			     << endl;
>> +			return TestFail;
>> +		}
>> +
>>  		return TestPass;
>>  	}
>>  };
> 

-- 
Regards
--
Kieran