* Re: [PATCH RFC] libv4lconvert: SDR conversion from U8 to FLOAT
2013-11-10 17:16 [PATCH RFC] libv4lconvert: SDR conversion from U8 to FLOAT Antti Palosaari
@ 2013-11-11 13:14 ` Hans Verkuil
2013-11-11 13:40 ` Antti Palosaari
2013-11-11 14:19 ` Andy Walls
2013-11-16 17:27 ` Hans de Goede
2 siblings, 1 reply; 16+ messages in thread
From: Hans Verkuil @ 2013-11-11 13:14 UTC (permalink / raw)
To: Antti Palosaari; +Cc: linux-media
On 11/10/2013 06:16 PM, Antti Palosaari wrote:
> Convert unsigned 8 to float 32 [-1 to +1], which is commonly
> used format for baseband signals.
>
> Signed-off-by: Antti Palosaari <crope@iki.fi>
> ---
> contrib/freebsd/include/linux/videodev2.h | 4 ++++
> include/linux/videodev2.h | 4 ++++
> lib/libv4lconvert/libv4lconvert.c | 29 ++++++++++++++++++++++++++++-
> 3 files changed, 36 insertions(+), 1 deletion(-)
>
> diff --git a/contrib/freebsd/include/linux/videodev2.h b/contrib/freebsd/include/linux/videodev2.h
> index 1fcfaeb..8829400 100644
> --- a/contrib/freebsd/include/linux/videodev2.h
> +++ b/contrib/freebsd/include/linux/videodev2.h
> @@ -465,6 +465,10 @@ struct v4l2_pix_format {
> #define V4L2_PIX_FMT_SE401 v4l2_fourcc('S', '4', '0', '1') /* se401 janggu compressed rgb */
> #define V4L2_PIX_FMT_S5C_UYVY_JPG v4l2_fourcc('S', '5', 'C', 'I') /* S5C73M3 interleaved UYVY/JPEG */
>
> +/* SDR */
> +#define V4L2_PIX_FMT_FLOAT v4l2_fourcc('D', 'F', '3', '2') /* float 32-bit */
> +#define V4L2_PIX_FMT_U8 v4l2_fourcc('D', 'U', '0', '8') /* unsigned 8-bit */
> +
> /*
> * F O R M A T E N U M E R A T I O N
> */
> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> index 437f1b0..14299a6 100644
> --- a/include/linux/videodev2.h
> +++ b/include/linux/videodev2.h
> @@ -431,6 +431,10 @@ struct v4l2_pix_format {
> #define V4L2_PIX_FMT_SE401 v4l2_fourcc('S', '4', '0', '1') /* se401 janggu compressed rgb */
> #define V4L2_PIX_FMT_S5C_UYVY_JPG v4l2_fourcc('S', '5', 'C', 'I') /* S5C73M3 interleaved UYVY/JPEG */
>
> +/* SDR */
> +#define V4L2_PIX_FMT_FLOAT v4l2_fourcc('D', 'F', '3', '2') /* float 32-bit */
> +#define V4L2_PIX_FMT_U8 v4l2_fourcc('D', 'U', '0', '8') /* unsigned 8-bit */
I would prefer V4L2_PIX_FMT_SDR_FLOAT and _FMT_SDR_U8.
That way it is clear that this format refers to - and should be interpreted as - an SDR format.
Otherwise it looks fine to me (but it needs to be documented as well, of course).
Regards,
Hans
> +
> /*
> * F O R M A T E N U M E R A T I O N
> */
> diff --git a/lib/libv4lconvert/libv4lconvert.c b/lib/libv4lconvert/libv4lconvert.c
> index e2afc27..38c9125 100644
> --- a/lib/libv4lconvert/libv4lconvert.c
> +++ b/lib/libv4lconvert/libv4lconvert.c
> @@ -78,7 +78,8 @@ static void v4lconvert_get_framesizes(struct v4lconvert_data *data,
> { V4L2_PIX_FMT_RGB24, 24, 1, 5, 0 }, \
> { V4L2_PIX_FMT_BGR24, 24, 1, 5, 0 }, \
> { V4L2_PIX_FMT_YUV420, 12, 6, 1, 0 }, \
> - { V4L2_PIX_FMT_YVU420, 12, 6, 1, 0 }
> + { V4L2_PIX_FMT_YVU420, 12, 6, 1, 0 }, \
> + { V4L2_PIX_FMT_FLOAT, 0, 0, 0, 0 }
>
> static const struct v4lconvert_pixfmt supported_src_pixfmts[] = {
> SUPPORTED_DST_PIXFMTS,
> @@ -131,6 +132,8 @@ static const struct v4lconvert_pixfmt supported_src_pixfmts[] = {
> { V4L2_PIX_FMT_Y6, 8, 20, 20, 0 },
> { V4L2_PIX_FMT_Y10BPACK, 10, 20, 20, 0 },
> { V4L2_PIX_FMT_Y16, 16, 20, 20, 0 },
> + /* SDR formats */
> + { V4L2_PIX_FMT_U8, 0, 0, 0, 0 },
> };
>
> static const struct v4lconvert_pixfmt supported_dst_pixfmts[] = {
> @@ -1281,6 +1284,25 @@ static int v4lconvert_convert_pixfmt(struct v4lconvert_data *data,
> }
> break;
>
> + /* SDR */
> + case V4L2_PIX_FMT_U8:
> + switch (dest_pix_fmt) {
> + case V4L2_PIX_FMT_FLOAT:
> + {
> + /* 8-bit unsigned to 32-bit float */
> + unsigned int i;
> + float ftmp;
> + for (i = 0; i < src_size; i++) {
> + ftmp = *src++;
> + ftmp -= 127.5;
> + ftmp /= 127.5;
> + memcpy(dest, &ftmp, 4);
> + dest += 4;
> + }
> + }
> + }
> + break;
> +
> default:
> V4LCONVERT_ERR("Unknown src format in conversion\n");
> errno = EINVAL;
> @@ -1349,6 +1371,11 @@ int v4lconvert_convert(struct v4lconvert_data *data,
> temp_needed =
> my_src_fmt.fmt.pix.width * my_src_fmt.fmt.pix.height * 3 / 2;
> break;
> + /* SDR */
> + case V4L2_PIX_FMT_FLOAT:
> + dest_needed = src_size * 4; /* 8-bit to 32-bit */
> + temp_needed = dest_needed;
> + break;
> default:
> V4LCONVERT_ERR("Unknown dest format in conversion\n");
> errno = EINVAL;
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH RFC] libv4lconvert: SDR conversion from U8 to FLOAT
2013-11-11 13:14 ` Hans Verkuil
@ 2013-11-11 13:40 ` Antti Palosaari
2013-11-11 13:52 ` Hans Verkuil
2013-11-15 19:11 ` Antti Palosaari
0 siblings, 2 replies; 16+ messages in thread
From: Antti Palosaari @ 2013-11-11 13:40 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media
On 11.11.2013 15:14, Hans Verkuil wrote:
> On 11/10/2013 06:16 PM, Antti Palosaari wrote:
>> Convert unsigned 8 to float 32 [-1 to +1], which is commonly
>> used format for baseband signals.
>>
>> Signed-off-by: Antti Palosaari <crope@iki.fi>
>> ---
>> contrib/freebsd/include/linux/videodev2.h | 4 ++++
>> include/linux/videodev2.h | 4 ++++
>> lib/libv4lconvert/libv4lconvert.c | 29 ++++++++++++++++++++++++++++-
>> 3 files changed, 36 insertions(+), 1 deletion(-)
>>
>> diff --git a/contrib/freebsd/include/linux/videodev2.h b/contrib/freebsd/include/linux/videodev2.h
>> index 1fcfaeb..8829400 100644
>> --- a/contrib/freebsd/include/linux/videodev2.h
>> +++ b/contrib/freebsd/include/linux/videodev2.h
>> @@ -465,6 +465,10 @@ struct v4l2_pix_format {
>> #define V4L2_PIX_FMT_SE401 v4l2_fourcc('S', '4', '0', '1') /* se401 janggu compressed rgb */
>> #define V4L2_PIX_FMT_S5C_UYVY_JPG v4l2_fourcc('S', '5', 'C', 'I') /* S5C73M3 interleaved UYVY/JPEG */
>>
>> +/* SDR */
>> +#define V4L2_PIX_FMT_FLOAT v4l2_fourcc('D', 'F', '3', '2') /* float 32-bit */
>> +#define V4L2_PIX_FMT_U8 v4l2_fourcc('D', 'U', '0', '8') /* unsigned 8-bit */
>> +
>> /*
>> * F O R M A T E N U M E R A T I O N
>> */
>> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
>> index 437f1b0..14299a6 100644
>> --- a/include/linux/videodev2.h
>> +++ b/include/linux/videodev2.h
>> @@ -431,6 +431,10 @@ struct v4l2_pix_format {
>> #define V4L2_PIX_FMT_SE401 v4l2_fourcc('S', '4', '0', '1') /* se401 janggu compressed rgb */
>> #define V4L2_PIX_FMT_S5C_UYVY_JPG v4l2_fourcc('S', '5', 'C', 'I') /* S5C73M3 interleaved UYVY/JPEG */
>>
>> +/* SDR */
>> +#define V4L2_PIX_FMT_FLOAT v4l2_fourcc('D', 'F', '3', '2') /* float 32-bit */
>> +#define V4L2_PIX_FMT_U8 v4l2_fourcc('D', 'U', '0', '8') /* unsigned 8-bit */
>
> I would prefer V4L2_PIX_FMT_SDR_FLOAT and _FMT_SDR_U8.
>
> That way it is clear that this format refers to - and should be interpreted as - an SDR format.
>
> Otherwise it looks fine to me (but it needs to be documented as well, of course).
Thanks for the comments!
What do you think is it OK to abuse/reuse pixelformat for radio signals?
Basically the only one field needed is just that, whilst those image
only fields (width/height) are not needed at all. Good point to reuse
existing things as much as possible is that it does not bloat Kernel
data structures etc.
I am also going to make some tests to find out if actual float
conversion is faster against pre-calculated LUT, in Kernel or in
libv4lconvert and so. Worst scenario I have currently is Mirics ADC with
14-bit resolution => 16384 quantization levels => 32-bit float LUT will
be 16384 * 4 = 65536 bytes. Wonder if that much big LUT is allowed to
library - but maybe you could alloc() and populate LUT on the fly if
needed. Or maybe native conversion is fast enough.
regards
Antti
>
> Regards,
>
> Hans
>
>> +
>> /*
>> * F O R M A T E N U M E R A T I O N
>> */
>> diff --git a/lib/libv4lconvert/libv4lconvert.c b/lib/libv4lconvert/libv4lconvert.c
>> index e2afc27..38c9125 100644
>> --- a/lib/libv4lconvert/libv4lconvert.c
>> +++ b/lib/libv4lconvert/libv4lconvert.c
>> @@ -78,7 +78,8 @@ static void v4lconvert_get_framesizes(struct v4lconvert_data *data,
>> { V4L2_PIX_FMT_RGB24, 24, 1, 5, 0 }, \
>> { V4L2_PIX_FMT_BGR24, 24, 1, 5, 0 }, \
>> { V4L2_PIX_FMT_YUV420, 12, 6, 1, 0 }, \
>> - { V4L2_PIX_FMT_YVU420, 12, 6, 1, 0 }
>> + { V4L2_PIX_FMT_YVU420, 12, 6, 1, 0 }, \
>> + { V4L2_PIX_FMT_FLOAT, 0, 0, 0, 0 }
>>
>> static const struct v4lconvert_pixfmt supported_src_pixfmts[] = {
>> SUPPORTED_DST_PIXFMTS,
>> @@ -131,6 +132,8 @@ static const struct v4lconvert_pixfmt supported_src_pixfmts[] = {
>> { V4L2_PIX_FMT_Y6, 8, 20, 20, 0 },
>> { V4L2_PIX_FMT_Y10BPACK, 10, 20, 20, 0 },
>> { V4L2_PIX_FMT_Y16, 16, 20, 20, 0 },
>> + /* SDR formats */
>> + { V4L2_PIX_FMT_U8, 0, 0, 0, 0 },
>> };
>>
>> static const struct v4lconvert_pixfmt supported_dst_pixfmts[] = {
>> @@ -1281,6 +1284,25 @@ static int v4lconvert_convert_pixfmt(struct v4lconvert_data *data,
>> }
>> break;
>>
>> + /* SDR */
>> + case V4L2_PIX_FMT_U8:
>> + switch (dest_pix_fmt) {
>> + case V4L2_PIX_FMT_FLOAT:
>> + {
>> + /* 8-bit unsigned to 32-bit float */
>> + unsigned int i;
>> + float ftmp;
>> + for (i = 0; i < src_size; i++) {
>> + ftmp = *src++;
>> + ftmp -= 127.5;
>> + ftmp /= 127.5;
>> + memcpy(dest, &ftmp, 4);
>> + dest += 4;
>> + }
>> + }
>> + }
>> + break;
>> +
>> default:
>> V4LCONVERT_ERR("Unknown src format in conversion\n");
>> errno = EINVAL;
>> @@ -1349,6 +1371,11 @@ int v4lconvert_convert(struct v4lconvert_data *data,
>> temp_needed =
>> my_src_fmt.fmt.pix.width * my_src_fmt.fmt.pix.height * 3 / 2;
>> break;
>> + /* SDR */
>> + case V4L2_PIX_FMT_FLOAT:
>> + dest_needed = src_size * 4; /* 8-bit to 32-bit */
>> + temp_needed = dest_needed;
>> + break;
>> default:
>> V4LCONVERT_ERR("Unknown dest format in conversion\n");
>> errno = EINVAL;
>>
>
--
http://palosaari.fi/
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH RFC] libv4lconvert: SDR conversion from U8 to FLOAT
2013-11-11 13:40 ` Antti Palosaari
@ 2013-11-11 13:52 ` Hans Verkuil
2013-11-15 19:11 ` Antti Palosaari
1 sibling, 0 replies; 16+ messages in thread
From: Hans Verkuil @ 2013-11-11 13:52 UTC (permalink / raw)
To: Antti Palosaari; +Cc: linux-media
On 11/11/2013 02:40 PM, Antti Palosaari wrote:
> On 11.11.2013 15:14, Hans Verkuil wrote:
>> On 11/10/2013 06:16 PM, Antti Palosaari wrote:
>>> Convert unsigned 8 to float 32 [-1 to +1], which is commonly
>>> used format for baseband signals.
>>>
>>> Signed-off-by: Antti Palosaari <crope@iki.fi>
>>> ---
>>> contrib/freebsd/include/linux/videodev2.h | 4 ++++
>>> include/linux/videodev2.h | 4 ++++
>>> lib/libv4lconvert/libv4lconvert.c | 29 ++++++++++++++++++++++++++++-
>>> 3 files changed, 36 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/contrib/freebsd/include/linux/videodev2.h b/contrib/freebsd/include/linux/videodev2.h
>>> index 1fcfaeb..8829400 100644
>>> --- a/contrib/freebsd/include/linux/videodev2.h
>>> +++ b/contrib/freebsd/include/linux/videodev2.h
>>> @@ -465,6 +465,10 @@ struct v4l2_pix_format {
>>> #define V4L2_PIX_FMT_SE401 v4l2_fourcc('S', '4', '0', '1') /* se401 janggu compressed rgb */
>>> #define V4L2_PIX_FMT_S5C_UYVY_JPG v4l2_fourcc('S', '5', 'C', 'I') /* S5C73M3 interleaved UYVY/JPEG */
>>>
>>> +/* SDR */
>>> +#define V4L2_PIX_FMT_FLOAT v4l2_fourcc('D', 'F', '3', '2') /* float 32-bit */
>>> +#define V4L2_PIX_FMT_U8 v4l2_fourcc('D', 'U', '0', '8') /* unsigned 8-bit */
>>> +
>>> /*
>>> * F O R M A T E N U M E R A T I O N
>>> */
>>> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
>>> index 437f1b0..14299a6 100644
>>> --- a/include/linux/videodev2.h
>>> +++ b/include/linux/videodev2.h
>>> @@ -431,6 +431,10 @@ struct v4l2_pix_format {
>>> #define V4L2_PIX_FMT_SE401 v4l2_fourcc('S', '4', '0', '1') /* se401 janggu compressed rgb */
>>> #define V4L2_PIX_FMT_S5C_UYVY_JPG v4l2_fourcc('S', '5', 'C', 'I') /* S5C73M3 interleaved UYVY/JPEG */
>>>
>>> +/* SDR */
>>> +#define V4L2_PIX_FMT_FLOAT v4l2_fourcc('D', 'F', '3', '2') /* float 32-bit */
>>> +#define V4L2_PIX_FMT_U8 v4l2_fourcc('D', 'U', '0', '8') /* unsigned 8-bit */
>>
>> I would prefer V4L2_PIX_FMT_SDR_FLOAT and _FMT_SDR_U8.
>>
>> That way it is clear that this format refers to - and should be interpreted as - an SDR format.
>>
>> Otherwise it looks fine to me (but it needs to be documented as well, of course).
>
> Thanks for the comments!
>
> What do you think is it OK to abuse/reuse pixelformat for radio signals?
> Basically the only one field needed is just that, whilst those image
> only fields (width/height) are not needed at all. Good point to reuse
> existing things as much as possible is that it does not bloat Kernel
> data structures etc.
I've no problems with that. While usually the buffers contain images, this
is not always the case. Strictly speaking it is just a DMA API and pixelformat
is used to define the contents. We use it to transport VBI data as well, and
in rare cases even audio (even though we shouldn't).
Regards,
Hans
> I am also going to make some tests to find out if actual float
> conversion is faster against pre-calculated LUT, in Kernel or in
> libv4lconvert and so. Worst scenario I have currently is Mirics ADC with
> 14-bit resolution => 16384 quantization levels => 32-bit float LUT will
> be 16384 * 4 = 65536 bytes. Wonder if that much big LUT is allowed to
> library - but maybe you could alloc() and populate LUT on the fly if
> needed. Or maybe native conversion is fast enough.
>
> regards
> Antti
>
>
>>
>> Regards,
>>
>> Hans
>>
>>> +
>>> /*
>>> * F O R M A T E N U M E R A T I O N
>>> */
>>> diff --git a/lib/libv4lconvert/libv4lconvert.c b/lib/libv4lconvert/libv4lconvert.c
>>> index e2afc27..38c9125 100644
>>> --- a/lib/libv4lconvert/libv4lconvert.c
>>> +++ b/lib/libv4lconvert/libv4lconvert.c
>>> @@ -78,7 +78,8 @@ static void v4lconvert_get_framesizes(struct v4lconvert_data *data,
>>> { V4L2_PIX_FMT_RGB24, 24, 1, 5, 0 }, \
>>> { V4L2_PIX_FMT_BGR24, 24, 1, 5, 0 }, \
>>> { V4L2_PIX_FMT_YUV420, 12, 6, 1, 0 }, \
>>> - { V4L2_PIX_FMT_YVU420, 12, 6, 1, 0 }
>>> + { V4L2_PIX_FMT_YVU420, 12, 6, 1, 0 }, \
>>> + { V4L2_PIX_FMT_FLOAT, 0, 0, 0, 0 }
>>>
>>> static const struct v4lconvert_pixfmt supported_src_pixfmts[] = {
>>> SUPPORTED_DST_PIXFMTS,
>>> @@ -131,6 +132,8 @@ static const struct v4lconvert_pixfmt supported_src_pixfmts[] = {
>>> { V4L2_PIX_FMT_Y6, 8, 20, 20, 0 },
>>> { V4L2_PIX_FMT_Y10BPACK, 10, 20, 20, 0 },
>>> { V4L2_PIX_FMT_Y16, 16, 20, 20, 0 },
>>> + /* SDR formats */
>>> + { V4L2_PIX_FMT_U8, 0, 0, 0, 0 },
>>> };
>>>
>>> static const struct v4lconvert_pixfmt supported_dst_pixfmts[] = {
>>> @@ -1281,6 +1284,25 @@ static int v4lconvert_convert_pixfmt(struct v4lconvert_data *data,
>>> }
>>> break;
>>>
>>> + /* SDR */
>>> + case V4L2_PIX_FMT_U8:
>>> + switch (dest_pix_fmt) {
>>> + case V4L2_PIX_FMT_FLOAT:
>>> + {
>>> + /* 8-bit unsigned to 32-bit float */
>>> + unsigned int i;
>>> + float ftmp;
>>> + for (i = 0; i < src_size; i++) {
>>> + ftmp = *src++;
>>> + ftmp -= 127.5;
>>> + ftmp /= 127.5;
>>> + memcpy(dest, &ftmp, 4);
>>> + dest += 4;
>>> + }
>>> + }
>>> + }
>>> + break;
>>> +
>>> default:
>>> V4LCONVERT_ERR("Unknown src format in conversion\n");
>>> errno = EINVAL;
>>> @@ -1349,6 +1371,11 @@ int v4lconvert_convert(struct v4lconvert_data *data,
>>> temp_needed =
>>> my_src_fmt.fmt.pix.width * my_src_fmt.fmt.pix.height * 3 / 2;
>>> break;
>>> + /* SDR */
>>> + case V4L2_PIX_FMT_FLOAT:
>>> + dest_needed = src_size * 4; /* 8-bit to 32-bit */
>>> + temp_needed = dest_needed;
>>> + break;
>>> default:
>>> V4LCONVERT_ERR("Unknown dest format in conversion\n");
>>> errno = EINVAL;
>>>
>>
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH RFC] libv4lconvert: SDR conversion from U8 to FLOAT
2013-11-11 13:40 ` Antti Palosaari
2013-11-11 13:52 ` Hans Verkuil
@ 2013-11-15 19:11 ` Antti Palosaari
2013-11-15 19:13 ` Devin Heitmueller
1 sibling, 1 reply; 16+ messages in thread
From: Antti Palosaari @ 2013-11-15 19:11 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media
On 11.11.2013 15:40, Antti Palosaari wrote:
> On 11.11.2013 15:14, Hans Verkuil wrote:
>> On 11/10/2013 06:16 PM, Antti Palosaari wrote:
>>> Convert unsigned 8 to float 32 [-1 to +1], which is commonly
>>> used format for baseband signals.
> I am also going to make some tests to find out if actual float
> conversion is faster against pre-calculated LUT, in Kernel or in
> libv4lconvert and so. Worst scenario I have currently is Mirics ADC with
> 14-bit resolution => 16384 quantization levels => 32-bit float LUT will
> be 16384 * 4 = 65536 bytes. Wonder if that much big LUT is allowed to
> library - but maybe you could alloc() and populate LUT on the fly if
> needed. Or maybe native conversion is fast enough.
That integer to float conversion uses quite much CPU still, even I use
only 2M sampling rate.
When I do it inside Kernel, in URB completion handler at the same time
when copying data to videobuf2, using pre-calculated LUTs and using mmap
it eats 0.5% CPU to transfer stream to app.
When I do same but using libv4lconvert as that patch, it takes ~11% CPU.
And it was only 2M sampling rate, Mirics could go something like 15M.
I wonder if I can optimize libv4lconvert to go near in Kernel LUT
conversion...
CPU: AMD Phenom(tm) II X4 955 Processor
regards
Antti
--
http://palosaari.fi/
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH RFC] libv4lconvert: SDR conversion from U8 to FLOAT
2013-11-15 19:11 ` Antti Palosaari
@ 2013-11-15 19:13 ` Devin Heitmueller
2013-11-15 19:17 ` Antti Palosaari
0 siblings, 1 reply; 16+ messages in thread
From: Devin Heitmueller @ 2013-11-15 19:13 UTC (permalink / raw)
To: Antti Palosaari; +Cc: Hans Verkuil, Linux Media Mailing List
On Fri, Nov 15, 2013 at 2:11 PM, Antti Palosaari <crope@iki.fi> wrote:
> When I do it inside Kernel, in URB completion handler at the same time when
> copying data to videobuf2, using pre-calculated LUTs and using mmap it eats
> 0.5% CPU to transfer stream to app.
>
> When I do same but using libv4lconvert as that patch, it takes ~11% CPU.
How are you measuring? Interrupt handlers typically don't count
toward the CPU performance counters. It's possible that the cost is
the same but you're just not seeing it in "top".
Devin
--
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH RFC] libv4lconvert: SDR conversion from U8 to FLOAT
2013-11-15 19:13 ` Devin Heitmueller
@ 2013-11-15 19:17 ` Antti Palosaari
2013-11-16 14:11 ` Antti Palosaari
0 siblings, 1 reply; 16+ messages in thread
From: Antti Palosaari @ 2013-11-15 19:17 UTC (permalink / raw)
To: Devin Heitmueller; +Cc: Hans Verkuil, Linux Media Mailing List
On 15.11.2013 21:13, Devin Heitmueller wrote:
> On Fri, Nov 15, 2013 at 2:11 PM, Antti Palosaari <crope@iki.fi> wrote:
>> When I do it inside Kernel, in URB completion handler at the same time when
>> copying data to videobuf2, using pre-calculated LUTs and using mmap it eats
>> 0.5% CPU to transfer stream to app.
>>
>> When I do same but using libv4lconvert as that patch, it takes ~11% CPU.
>
> How are you measuring? Interrupt handlers typically don't count
> toward the CPU performance counters. It's possible that the cost is
> the same but you're just not seeing it in "top".
Yes, using top and it is URB interrupt handler where I do conversion. So
any idea how to measure? I think I can still switch LUT to float and see
if it makes difference.
regards
Antti
--
http://palosaari.fi/
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH RFC] libv4lconvert: SDR conversion from U8 to FLOAT
2013-11-15 19:17 ` Antti Palosaari
@ 2013-11-16 14:11 ` Antti Palosaari
2013-11-16 17:28 ` Hans de Goede
0 siblings, 1 reply; 16+ messages in thread
From: Antti Palosaari @ 2013-11-16 14:11 UTC (permalink / raw)
To: Devin Heitmueller; +Cc: Hans Verkuil, Linux Media Mailing List
On 15.11.2013 21:17, Antti Palosaari wrote:
> On 15.11.2013 21:13, Devin Heitmueller wrote:
>> On Fri, Nov 15, 2013 at 2:11 PM, Antti Palosaari <crope@iki.fi> wrote:
>>> When I do it inside Kernel, in URB completion handler at the same
>>> time when
>>> copying data to videobuf2, using pre-calculated LUTs and using mmap
>>> it eats
>>> 0.5% CPU to transfer stream to app.
>>>
>>> When I do same but using libv4lconvert as that patch, it takes ~11% CPU.
>>
>> How are you measuring? Interrupt handlers typically don't count
>> toward the CPU performance counters. It's possible that the cost is
>> the same but you're just not seeing it in "top".
>
> Yes, using top and it is URB interrupt handler where I do conversion. So
> any idea how to measure? I think I can still switch LUT to float and see
> if it makes difference.
I did some more tests. I added LUT to libv4lconvert and CPU usage of
process dropped to ~3.5%. It is very simple app that just feeds data
from device using mmap and conversion is done by libv4lconvert. Output
is feed to standard out which I dumped to /dev/null on tests.
So it is quite clear that runtime float conversion is CPU hungry when
conversion rate goes that high (up to 30M conversions per sec).
It is still not very much when compared to CPU needed for average signal
processing after that, but it will increase directly CPU usage of that
application. So is there idea to add threads for libv4lconvert in order
to get conversion out from application context ?
regards
Antti
--
http://palosaari.fi/
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH RFC] libv4lconvert: SDR conversion from U8 to FLOAT
2013-11-16 14:11 ` Antti Palosaari
@ 2013-11-16 17:28 ` Hans de Goede
0 siblings, 0 replies; 16+ messages in thread
From: Hans de Goede @ 2013-11-16 17:28 UTC (permalink / raw)
To: Antti Palosaari, Devin Heitmueller; +Cc: Hans Verkuil, Linux Media Mailing List
Hi,
On 11/16/2013 03:11 PM, Antti Palosaari wrote:
> On 15.11.2013 21:17, Antti Palosaari wrote:
>> On 15.11.2013 21:13, Devin Heitmueller wrote:
>>> On Fri, Nov 15, 2013 at 2:11 PM, Antti Palosaari <crope@iki.fi> wrote:
>>>> When I do it inside Kernel, in URB completion handler at the same
>>>> time when
>>>> copying data to videobuf2, using pre-calculated LUTs and using mmap
>>>> it eats
>>>> 0.5% CPU to transfer stream to app.
>>>>
>>>> When I do same but using libv4lconvert as that patch, it takes ~11% CPU.
>>>
>>> How are you measuring? Interrupt handlers typically don't count
>>> toward the CPU performance counters. It's possible that the cost is
>>> the same but you're just not seeing it in "top".
>>
>> Yes, using top and it is URB interrupt handler where I do conversion. So
>> any idea how to measure? I think I can still switch LUT to float and see
>> if it makes difference.
>
> I did some more tests. I added LUT to libv4lconvert and CPU usage of process dropped to ~3.5%. It is very simple app that just feeds data from device using mmap and conversion is done by libv4lconvert. Output is feed to standard out which I dumped to /dev/null on tests.
>
> So it is quite clear that runtime float conversion is CPU hungry when conversion rate goes that high (up to 30M conversions per sec).
>
> It is still not very much when compared to CPU needed for average signal processing after that, but it will increase directly CPU usage of that application. So is there idea to add threads for libv4lconvert in order to get conversion out from application context ?
I do not want to start using threads for libv4lconvert, if an app wants to do
the conversion in another thread, it can simply do all the v4l stuff in a
separate capture thread, like gstreamer does for example.
Regards,
Hans
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH RFC] libv4lconvert: SDR conversion from U8 to FLOAT
2013-11-10 17:16 [PATCH RFC] libv4lconvert: SDR conversion from U8 to FLOAT Antti Palosaari
2013-11-11 13:14 ` Hans Verkuil
@ 2013-11-11 14:19 ` Andy Walls
2013-11-11 14:38 ` Hans Verkuil
2013-11-18 17:43 ` Antti Palosaari
2013-11-16 17:27 ` Hans de Goede
2 siblings, 2 replies; 16+ messages in thread
From: Andy Walls @ 2013-11-11 14:19 UTC (permalink / raw)
To: Antti Palosaari; +Cc: linux-media
On Sun, 2013-11-10 at 19:16 +0200, Antti Palosaari wrote:
> Convert unsigned 8 to float 32 [-1 to +1], which is commonly
> used format for baseband signals.
Hi Annti,
I don't think this a good idea. Floating point representations are
inherently non-portable. Even though most everything now uses IEEE-754
representation, things like denormaliazed numbers may be treated
differently by different machines. If someone saves the data to a file,
endianess issues aside, there are no guarantees that a different machine
reading is going to interpret all the floating point data from that file
properly.
I really would recommend staying with scaled integer representations or
explicit integer mantissa, exponent representations.
Two more comments below...
> Signed-off-by: Antti Palosaari <crope@iki.fi>
> ---
> contrib/freebsd/include/linux/videodev2.h | 4 ++++
> include/linux/videodev2.h | 4 ++++
> lib/libv4lconvert/libv4lconvert.c | 29 ++++++++++++++++++++++++++++-
> 3 files changed, 36 insertions(+), 1 deletion(-)
>
> diff --git a/contrib/freebsd/include/linux/videodev2.h b/contrib/freebsd/include/linux/videodev2.h
> index 1fcfaeb..8829400 100644
> --- a/contrib/freebsd/include/linux/videodev2.h
> +++ b/contrib/freebsd/include/linux/videodev2.h
> @@ -465,6 +465,10 @@ struct v4l2_pix_format {
> #define V4L2_PIX_FMT_SE401 v4l2_fourcc('S', '4', '0', '1') /* se401 janggu compressed rgb */
> #define V4L2_PIX_FMT_S5C_UYVY_JPG v4l2_fourcc('S', '5', 'C', 'I') /* S5C73M3 interleaved UYVY/JPEG */
>
> +/* SDR */
> +#define V4L2_PIX_FMT_FLOAT v4l2_fourcc('D', 'F', '3', '2') /* float 32-bit */
> +#define V4L2_PIX_FMT_U8 v4l2_fourcc('D', 'U', '0', '8') /* unsigned 8-bit */
> +
> /*
> * F O R M A T E N U M E R A T I O N
> */
> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> index 437f1b0..14299a6 100644
> --- a/include/linux/videodev2.h
> +++ b/include/linux/videodev2.h
> @@ -431,6 +431,10 @@ struct v4l2_pix_format {
> #define V4L2_PIX_FMT_SE401 v4l2_fourcc('S', '4', '0', '1') /* se401 janggu compressed rgb */
> #define V4L2_PIX_FMT_S5C_UYVY_JPG v4l2_fourcc('S', '5', 'C', 'I') /* S5C73M3 interleaved UYVY/JPEG */
>
> +/* SDR */
> +#define V4L2_PIX_FMT_FLOAT v4l2_fourcc('D', 'F', '3', '2') /* float 32-bit */
> +#define V4L2_PIX_FMT_U8 v4l2_fourcc('D', 'U', '0', '8') /* unsigned 8-bit */
> +
> /*
> * F O R M A T E N U M E R A T I O N
> */
> diff --git a/lib/libv4lconvert/libv4lconvert.c b/lib/libv4lconvert/libv4lconvert.c
> index e2afc27..38c9125 100644
> --- a/lib/libv4lconvert/libv4lconvert.c
> +++ b/lib/libv4lconvert/libv4lconvert.c
> @@ -78,7 +78,8 @@ static void v4lconvert_get_framesizes(struct v4lconvert_data *data,
> { V4L2_PIX_FMT_RGB24, 24, 1, 5, 0 }, \
> { V4L2_PIX_FMT_BGR24, 24, 1, 5, 0 }, \
> { V4L2_PIX_FMT_YUV420, 12, 6, 1, 0 }, \
> - { V4L2_PIX_FMT_YVU420, 12, 6, 1, 0 }
> + { V4L2_PIX_FMT_YVU420, 12, 6, 1, 0 }, \
> + { V4L2_PIX_FMT_FLOAT, 0, 0, 0, 0 }
>
> static const struct v4lconvert_pixfmt supported_src_pixfmts[] = {
> SUPPORTED_DST_PIXFMTS,
> @@ -131,6 +132,8 @@ static const struct v4lconvert_pixfmt supported_src_pixfmts[] = {
> { V4L2_PIX_FMT_Y6, 8, 20, 20, 0 },
> { V4L2_PIX_FMT_Y10BPACK, 10, 20, 20, 0 },
> { V4L2_PIX_FMT_Y16, 16, 20, 20, 0 },
> + /* SDR formats */
> + { V4L2_PIX_FMT_U8, 0, 0, 0, 0 },
> };
>
> static const struct v4lconvert_pixfmt supported_dst_pixfmts[] = {
> @@ -1281,6 +1284,25 @@ static int v4lconvert_convert_pixfmt(struct v4lconvert_data *data,
> }
> break;
>
> + /* SDR */
> + case V4L2_PIX_FMT_U8:
> + switch (dest_pix_fmt) {
> + case V4L2_PIX_FMT_FLOAT:
> + {
> + /* 8-bit unsigned to 32-bit float */
> + unsigned int i;
> + float ftmp;
> + for (i = 0; i < src_size; i++) {
> + ftmp = *src++;
> + ftmp -= 127.5;
> + ftmp /= 127.5;
> + memcpy(dest, &ftmp, 4);
> + dest += 4;
Replace the 4's with sizeof(float).
You have no guarantees that sizeof(float) == 4, but it is usally a safe
assumption for 'float' on Unix.
sizeof(long double) is certainly different for IA32 machines (80 bits)
vs. other 32 bit platforms. I was burned by this many years ago on a
RedHat 9.0 machine (the GNU Ada Translator's libm bindings made some bad
assumptions about the size of float types).
> + }
> + }
> + }
> + break;
> +
> default:
> V4LCONVERT_ERR("Unknown src format in conversion\n");
> errno = EINVAL;
> @@ -1349,6 +1371,11 @@ int v4lconvert_convert(struct v4lconvert_data *data,
> temp_needed =
> my_src_fmt.fmt.pix.width * my_src_fmt.fmt.pix.height * 3 / 2;
> break;
> + /* SDR */
> + case V4L2_PIX_FMT_FLOAT:
> + dest_needed = src_size * 4; /* 8-bit to 32-bit */
Change the 4 to sizeof(float).
> + temp_needed = dest_needed;
> + break;
> default:
> V4LCONVERT_ERR("Unknown dest format in conversion\n");
> errno = EINVAL;
Regards,
Andy
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH RFC] libv4lconvert: SDR conversion from U8 to FLOAT
2013-11-11 14:19 ` Andy Walls
@ 2013-11-11 14:38 ` Hans Verkuil
2013-11-14 13:45 ` Antti Palosaari
2013-11-18 17:43 ` Antti Palosaari
1 sibling, 1 reply; 16+ messages in thread
From: Hans Verkuil @ 2013-11-11 14:38 UTC (permalink / raw)
To: Andy Walls; +Cc: Antti Palosaari, linux-media
On 11/11/2013 03:19 PM, Andy Walls wrote:
> On Sun, 2013-11-10 at 19:16 +0200, Antti Palosaari wrote:
>> Convert unsigned 8 to float 32 [-1 to +1], which is commonly
>> used format for baseband signals.
>
> Hi Annti,
>
> I don't think this a good idea. Floating point representations are
> inherently non-portable. Even though most everything now uses IEEE-754
> representation, things like denormaliazed numbers may be treated
> differently by different machines. If someone saves the data to a file,
> endianess issues aside, there are no guarantees that a different machine
> reading is going to interpret all the floating point data from that file
> properly.
>
> I really would recommend staying with scaled integer representations or
> explicit integer mantissa, exponent representations.
For what it's worth: ALSA does support float format as well (both LE and BE).
Regards,
Hans
>
> Two more comments below...
>
>> Signed-off-by: Antti Palosaari <crope@iki.fi>
>> ---
>> contrib/freebsd/include/linux/videodev2.h | 4 ++++
>> include/linux/videodev2.h | 4 ++++
>> lib/libv4lconvert/libv4lconvert.c | 29 ++++++++++++++++++++++++++++-
>> 3 files changed, 36 insertions(+), 1 deletion(-)
>>
>> diff --git a/contrib/freebsd/include/linux/videodev2.h b/contrib/freebsd/include/linux/videodev2.h
>> index 1fcfaeb..8829400 100644
>> --- a/contrib/freebsd/include/linux/videodev2.h
>> +++ b/contrib/freebsd/include/linux/videodev2.h
>> @@ -465,6 +465,10 @@ struct v4l2_pix_format {
>> #define V4L2_PIX_FMT_SE401 v4l2_fourcc('S', '4', '0', '1') /* se401 janggu compressed rgb */
>> #define V4L2_PIX_FMT_S5C_UYVY_JPG v4l2_fourcc('S', '5', 'C', 'I') /* S5C73M3 interleaved UYVY/JPEG */
>>
>> +/* SDR */
>> +#define V4L2_PIX_FMT_FLOAT v4l2_fourcc('D', 'F', '3', '2') /* float 32-bit */
>> +#define V4L2_PIX_FMT_U8 v4l2_fourcc('D', 'U', '0', '8') /* unsigned 8-bit */
>> +
>> /*
>> * F O R M A T E N U M E R A T I O N
>> */
>> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
>> index 437f1b0..14299a6 100644
>> --- a/include/linux/videodev2.h
>> +++ b/include/linux/videodev2.h
>> @@ -431,6 +431,10 @@ struct v4l2_pix_format {
>> #define V4L2_PIX_FMT_SE401 v4l2_fourcc('S', '4', '0', '1') /* se401 janggu compressed rgb */
>> #define V4L2_PIX_FMT_S5C_UYVY_JPG v4l2_fourcc('S', '5', 'C', 'I') /* S5C73M3 interleaved UYVY/JPEG */
>>
>> +/* SDR */
>> +#define V4L2_PIX_FMT_FLOAT v4l2_fourcc('D', 'F', '3', '2') /* float 32-bit */
>> +#define V4L2_PIX_FMT_U8 v4l2_fourcc('D', 'U', '0', '8') /* unsigned 8-bit */
>> +
>> /*
>> * F O R M A T E N U M E R A T I O N
>> */
>> diff --git a/lib/libv4lconvert/libv4lconvert.c b/lib/libv4lconvert/libv4lconvert.c
>> index e2afc27..38c9125 100644
>> --- a/lib/libv4lconvert/libv4lconvert.c
>> +++ b/lib/libv4lconvert/libv4lconvert.c
>> @@ -78,7 +78,8 @@ static void v4lconvert_get_framesizes(struct v4lconvert_data *data,
>> { V4L2_PIX_FMT_RGB24, 24, 1, 5, 0 }, \
>> { V4L2_PIX_FMT_BGR24, 24, 1, 5, 0 }, \
>> { V4L2_PIX_FMT_YUV420, 12, 6, 1, 0 }, \
>> - { V4L2_PIX_FMT_YVU420, 12, 6, 1, 0 }
>> + { V4L2_PIX_FMT_YVU420, 12, 6, 1, 0 }, \
>> + { V4L2_PIX_FMT_FLOAT, 0, 0, 0, 0 }
>>
>> static const struct v4lconvert_pixfmt supported_src_pixfmts[] = {
>> SUPPORTED_DST_PIXFMTS,
>> @@ -131,6 +132,8 @@ static const struct v4lconvert_pixfmt supported_src_pixfmts[] = {
>> { V4L2_PIX_FMT_Y6, 8, 20, 20, 0 },
>> { V4L2_PIX_FMT_Y10BPACK, 10, 20, 20, 0 },
>> { V4L2_PIX_FMT_Y16, 16, 20, 20, 0 },
>> + /* SDR formats */
>> + { V4L2_PIX_FMT_U8, 0, 0, 0, 0 },
>> };
>>
>> static const struct v4lconvert_pixfmt supported_dst_pixfmts[] = {
>> @@ -1281,6 +1284,25 @@ static int v4lconvert_convert_pixfmt(struct v4lconvert_data *data,
>> }
>> break;
>>
>> + /* SDR */
>> + case V4L2_PIX_FMT_U8:
>> + switch (dest_pix_fmt) {
>> + case V4L2_PIX_FMT_FLOAT:
>> + {
>> + /* 8-bit unsigned to 32-bit float */
>> + unsigned int i;
>> + float ftmp;
>> + for (i = 0; i < src_size; i++) {
>> + ftmp = *src++;
>> + ftmp -= 127.5;
>> + ftmp /= 127.5;
>> + memcpy(dest, &ftmp, 4);
>> + dest += 4;
>
> Replace the 4's with sizeof(float).
>
> You have no guarantees that sizeof(float) == 4, but it is usally a safe
> assumption for 'float' on Unix.
>
> sizeof(long double) is certainly different for IA32 machines (80 bits)
> vs. other 32 bit platforms. I was burned by this many years ago on a
> RedHat 9.0 machine (the GNU Ada Translator's libm bindings made some bad
> assumptions about the size of float types).
>
>
>> + }
>> + }
>> + }
>> + break;
>> +
>> default:
>> V4LCONVERT_ERR("Unknown src format in conversion\n");
>> errno = EINVAL;
>> @@ -1349,6 +1371,11 @@ int v4lconvert_convert(struct v4lconvert_data *data,
>> temp_needed =
>> my_src_fmt.fmt.pix.width * my_src_fmt.fmt.pix.height * 3 / 2;
>> break;
>> + /* SDR */
>> + case V4L2_PIX_FMT_FLOAT:
>> + dest_needed = src_size * 4; /* 8-bit to 32-bit */
>
> Change the 4 to sizeof(float).
>
>> + temp_needed = dest_needed;
>> + break;
>> default:
>> V4LCONVERT_ERR("Unknown dest format in conversion\n");
>> errno = EINVAL;
>
> Regards,
> Andy
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH RFC] libv4lconvert: SDR conversion from U8 to FLOAT
2013-11-11 14:38 ` Hans Verkuil
@ 2013-11-14 13:45 ` Antti Palosaari
2013-11-14 13:50 ` Hans Verkuil
0 siblings, 1 reply; 16+ messages in thread
From: Antti Palosaari @ 2013-11-14 13:45 UTC (permalink / raw)
To: Hans Verkuil, Andy Walls; +Cc: linux-media
On 11.11.2013 16:38, Hans Verkuil wrote:
> On 11/11/2013 03:19 PM, Andy Walls wrote:
>> On Sun, 2013-11-10 at 19:16 +0200, Antti Palosaari wrote:
>>> Convert unsigned 8 to float 32 [-1 to +1], which is commonly
>>> used format for baseband signals.
>>
>> Hi Annti,
>>
>> I don't think this a good idea. Floating point representations are
>> inherently non-portable. Even though most everything now uses IEEE-754
>> representation, things like denormaliazed numbers may be treated
>> differently by different machines. If someone saves the data to a file,
>> endianess issues aside, there are no guarantees that a different machine
>> reading is going to interpret all the floating point data from that file
>> properly.
>>
>> I really would recommend staying with scaled integer representations or
>> explicit integer mantissa, exponent representations.
>
> For what it's worth: ALSA does support float format as well (both LE and BE).
I want use existing data formats and that [-1 to +1] scaled 32-bit
IEEE-754 floating point is de facto format for SDR application (actually
pair of floats as a complex).
Doing conversion inside libv4lconvert makes it very easy for write
application. Currently I have implemented GNU Radio and SDRsharp plugins
that feeds data from device via libv4l2 using mmap and conversion.
Thanks to pointing endianess issue, I didn't though it all. I suspect
those apps just relies to local endianess. So do I have to implement
float format conversion with both endianess?
>
> Regards,
>
> Hans
>
>>
>> Two more comments below...
>>
>>> Signed-off-by: Antti Palosaari <crope@iki.fi>
>>> ---
>>> contrib/freebsd/include/linux/videodev2.h | 4 ++++
>>> include/linux/videodev2.h | 4 ++++
>>> lib/libv4lconvert/libv4lconvert.c | 29 ++++++++++++++++++++++++++++-
>>> 3 files changed, 36 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/contrib/freebsd/include/linux/videodev2.h b/contrib/freebsd/include/linux/videodev2.h
>>> index 1fcfaeb..8829400 100644
>>> --- a/contrib/freebsd/include/linux/videodev2.h
>>> +++ b/contrib/freebsd/include/linux/videodev2.h
>>> @@ -465,6 +465,10 @@ struct v4l2_pix_format {
>>> #define V4L2_PIX_FMT_SE401 v4l2_fourcc('S', '4', '0', '1') /* se401 janggu compressed rgb */
>>> #define V4L2_PIX_FMT_S5C_UYVY_JPG v4l2_fourcc('S', '5', 'C', 'I') /* S5C73M3 interleaved UYVY/JPEG */
>>>
>>> +/* SDR */
>>> +#define V4L2_PIX_FMT_FLOAT v4l2_fourcc('D', 'F', '3', '2') /* float 32-bit */
>>> +#define V4L2_PIX_FMT_U8 v4l2_fourcc('D', 'U', '0', '8') /* unsigned 8-bit */
>>> +
>>> /*
>>> * F O R M A T E N U M E R A T I O N
>>> */
>>> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
>>> index 437f1b0..14299a6 100644
>>> --- a/include/linux/videodev2.h
>>> +++ b/include/linux/videodev2.h
>>> @@ -431,6 +431,10 @@ struct v4l2_pix_format {
>>> #define V4L2_PIX_FMT_SE401 v4l2_fourcc('S', '4', '0', '1') /* se401 janggu compressed rgb */
>>> #define V4L2_PIX_FMT_S5C_UYVY_JPG v4l2_fourcc('S', '5', 'C', 'I') /* S5C73M3 interleaved UYVY/JPEG */
>>>
>>> +/* SDR */
>>> +#define V4L2_PIX_FMT_FLOAT v4l2_fourcc('D', 'F', '3', '2') /* float 32-bit */
>>> +#define V4L2_PIX_FMT_U8 v4l2_fourcc('D', 'U', '0', '8') /* unsigned 8-bit */
>>> +
>>> /*
>>> * F O R M A T E N U M E R A T I O N
>>> */
>>> diff --git a/lib/libv4lconvert/libv4lconvert.c b/lib/libv4lconvert/libv4lconvert.c
>>> index e2afc27..38c9125 100644
>>> --- a/lib/libv4lconvert/libv4lconvert.c
>>> +++ b/lib/libv4lconvert/libv4lconvert.c
>>> @@ -78,7 +78,8 @@ static void v4lconvert_get_framesizes(struct v4lconvert_data *data,
>>> { V4L2_PIX_FMT_RGB24, 24, 1, 5, 0 }, \
>>> { V4L2_PIX_FMT_BGR24, 24, 1, 5, 0 }, \
>>> { V4L2_PIX_FMT_YUV420, 12, 6, 1, 0 }, \
>>> - { V4L2_PIX_FMT_YVU420, 12, 6, 1, 0 }
>>> + { V4L2_PIX_FMT_YVU420, 12, 6, 1, 0 }, \
>>> + { V4L2_PIX_FMT_FLOAT, 0, 0, 0, 0 }
>>>
>>> static const struct v4lconvert_pixfmt supported_src_pixfmts[] = {
>>> SUPPORTED_DST_PIXFMTS,
>>> @@ -131,6 +132,8 @@ static const struct v4lconvert_pixfmt supported_src_pixfmts[] = {
>>> { V4L2_PIX_FMT_Y6, 8, 20, 20, 0 },
>>> { V4L2_PIX_FMT_Y10BPACK, 10, 20, 20, 0 },
>>> { V4L2_PIX_FMT_Y16, 16, 20, 20, 0 },
>>> + /* SDR formats */
>>> + { V4L2_PIX_FMT_U8, 0, 0, 0, 0 },
>>> };
>>>
>>> static const struct v4lconvert_pixfmt supported_dst_pixfmts[] = {
>>> @@ -1281,6 +1284,25 @@ static int v4lconvert_convert_pixfmt(struct v4lconvert_data *data,
>>> }
>>> break;
>>>
>>> + /* SDR */
>>> + case V4L2_PIX_FMT_U8:
>>> + switch (dest_pix_fmt) {
>>> + case V4L2_PIX_FMT_FLOAT:
>>> + {
>>> + /* 8-bit unsigned to 32-bit float */
>>> + unsigned int i;
>>> + float ftmp;
>>> + for (i = 0; i < src_size; i++) {
>>> + ftmp = *src++;
>>> + ftmp -= 127.5;
>>> + ftmp /= 127.5;
>>> + memcpy(dest, &ftmp, 4);
>>> + dest += 4;
>>
>> Replace the 4's with sizeof(float).
>>
>> You have no guarantees that sizeof(float) == 4, but it is usally a safe
>> assumption for 'float' on Unix.
>>
>> sizeof(long double) is certainly different for IA32 machines (80 bits)
>> vs. other 32 bit platforms. I was burned by this many years ago on a
>> RedHat 9.0 machine (the GNU Ada Translator's libm bindings made some bad
>> assumptions about the size of float types).
Will do.
>>
>>
>>> + }
>>> + }
>>> + }
>>> + break;
>>> +
>>> default:
>>> V4LCONVERT_ERR("Unknown src format in conversion\n");
>>> errno = EINVAL;
>>> @@ -1349,6 +1371,11 @@ int v4lconvert_convert(struct v4lconvert_data *data,
>>> temp_needed =
>>> my_src_fmt.fmt.pix.width * my_src_fmt.fmt.pix.height * 3 / 2;
>>> break;
>>> + /* SDR */
>>> + case V4L2_PIX_FMT_FLOAT:
>>> + dest_needed = src_size * 4; /* 8-bit to 32-bit */
>>
>> Change the 4 to sizeof(float).
>>
>>> + temp_needed = dest_needed;
>>> + break;
>>> default:
>>> V4LCONVERT_ERR("Unknown dest format in conversion\n");
>>> errno = EINVAL;
>>
>> Regards,
>> Andy
regards
Antti
--
http://palosaari.fi/
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH RFC] libv4lconvert: SDR conversion from U8 to FLOAT
2013-11-14 13:45 ` Antti Palosaari
@ 2013-11-14 13:50 ` Hans Verkuil
0 siblings, 0 replies; 16+ messages in thread
From: Hans Verkuil @ 2013-11-14 13:50 UTC (permalink / raw)
To: Antti Palosaari; +Cc: Andy Walls, linux-media
On 11/14/13 14:45, Antti Palosaari wrote:
> On 11.11.2013 16:38, Hans Verkuil wrote:
>> On 11/11/2013 03:19 PM, Andy Walls wrote:
>>> On Sun, 2013-11-10 at 19:16 +0200, Antti Palosaari wrote:
>>>> Convert unsigned 8 to float 32 [-1 to +1], which is commonly
>>>> used format for baseband signals.
>>>
>>> Hi Annti,
>>>
>>> I don't think this a good idea. Floating point representations are
>>> inherently non-portable. Even though most everything now uses IEEE-754
>>> representation, things like denormaliazed numbers may be treated
>>> differently by different machines. If someone saves the data to a file,
>>> endianess issues aside, there are no guarantees that a different machine
>>> reading is going to interpret all the floating point data from that file
>>> properly.
>>>
>>> I really would recommend staying with scaled integer representations or
>>> explicit integer mantissa, exponent representations.
>>
>> For what it's worth: ALSA does support float format as well (both LE and BE).
>
> I want use existing data formats and that [-1 to +1] scaled 32-bit IEEE-754 floating point is de facto format for SDR application (actually pair of floats as a complex).
>
> Doing conversion inside libv4lconvert makes it very easy for write application. Currently I have implemented GNU Radio and SDRsharp plugins that feeds data from device via libv4l2 using mmap and conversion.
>
> Thanks to pointing endianess issue, I didn't though it all. I suspect those apps just relies to local endianess. So do I have to implement float format conversion with both endianess?
I would say that it should be written with endianness conversion in mind. It's not
likely to be used on a big-endian system, but it should be possible to support it
without too much work.
Regards,
Hans
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH RFC] libv4lconvert: SDR conversion from U8 to FLOAT
2013-11-11 14:19 ` Andy Walls
2013-11-11 14:38 ` Hans Verkuil
@ 2013-11-18 17:43 ` Antti Palosaari
1 sibling, 0 replies; 16+ messages in thread
From: Antti Palosaari @ 2013-11-18 17:43 UTC (permalink / raw)
To: Andy Walls; +Cc: linux-media
On 11.11.2013 16:19, Andy Walls wrote:
> On Sun, 2013-11-10 at 19:16 +0200, Antti Palosaari wrote:
>> Convert unsigned 8 to float 32 [-1 to +1], which is commonly
>> used format for baseband signals.
>
> Hi Annti,
>
> I don't think this a good idea. Floating point representations are
> inherently non-portable. Even though most everything now uses IEEE-754
> representation, things like denormaliazed numbers may be treated
> differently by different machines. If someone saves the data to a file,
> endianess issues aside, there are no guarantees that a different machine
> reading is going to interpret all the floating point data from that file
> properly.
>
> I really would recommend staying with scaled integer representations or
> explicit integer mantissa, exponent representations.
Do you mean scaled presentation like a sample is always 32 signed
integer and what ever ADC resolution is, it is scaled to 32-bit signed
int and returned?
What I would like to implement is 8-bit int, 16-bit int and maybe 32-bit
int (if there comes ADC outputting more than 16-bit). These all
conversions are done inside Kernel, which actually has price about
nothing as it is simple integer math with bit shifting (scaling == bit
shifting). If you do that kind of conversion on USB URB interrupt at the
same time as memory copy from URB to videobuf2 is needed, it is
basically free.
Then it is up to caller to select int8, int16, int32 and driver does the
rest, selects actual ADC resolution using info like sampling rate.
Also returning SDR floats directly from Kernel driver could be very
handy, but as floats are not allowed in Kernel...
But now all conversions are in the libv4l. However, it is possible to
add new formats to driver later - removing existing formats from driver
is about impossible.
regards
Antti
--
http://palosaari.fi/
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH RFC] libv4lconvert: SDR conversion from U8 to FLOAT
2013-11-10 17:16 [PATCH RFC] libv4lconvert: SDR conversion from U8 to FLOAT Antti Palosaari
2013-11-11 13:14 ` Hans Verkuil
2013-11-11 14:19 ` Andy Walls
@ 2013-11-16 17:27 ` Hans de Goede
2013-11-16 17:34 ` Antti Palosaari
2 siblings, 1 reply; 16+ messages in thread
From: Hans de Goede @ 2013-11-16 17:27 UTC (permalink / raw)
To: Antti Palosaari, linux-media
Hi,
On 11/10/2013 06:16 PM, Antti Palosaari wrote:
> Convert unsigned 8 to float 32 [-1 to +1], which is commonly
> used format for baseband signals.
I've no objection to adding this, but this will need some special casing
I think. The current patch looks wrong.
> Signed-off-by: Antti Palosaari <crope@iki.fi>
> ---
> contrib/freebsd/include/linux/videodev2.h | 4 ++++
> include/linux/videodev2.h | 4 ++++
> lib/libv4lconvert/libv4lconvert.c | 29 ++++++++++++++++++++++++++++-
> 3 files changed, 36 insertions(+), 1 deletion(-)
>
> diff --git a/contrib/freebsd/include/linux/videodev2.h b/contrib/freebsd/include/linux/videodev2.h
> index 1fcfaeb..8829400 100644
> --- a/contrib/freebsd/include/linux/videodev2.h
> +++ b/contrib/freebsd/include/linux/videodev2.h
> @@ -465,6 +465,10 @@ struct v4l2_pix_format {
> #define V4L2_PIX_FMT_SE401 v4l2_fourcc('S', '4', '0', '1') /* se401 janggu compressed rgb */
> #define V4L2_PIX_FMT_S5C_UYVY_JPG v4l2_fourcc('S', '5', 'C', 'I') /* S5C73M3 interleaved UYVY/JPEG */
>
> +/* SDR */
> +#define V4L2_PIX_FMT_FLOAT v4l2_fourcc('D', 'F', '3', '2') /* float 32-bit */
> +#define V4L2_PIX_FMT_U8 v4l2_fourcc('D', 'U', '0', '8') /* unsigned 8-bit */
> +
> /*
> * F O R M A T E N U M E R A T I O N
> */
> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> index 437f1b0..14299a6 100644
> --- a/include/linux/videodev2.h
> +++ b/include/linux/videodev2.h
> @@ -431,6 +431,10 @@ struct v4l2_pix_format {
> #define V4L2_PIX_FMT_SE401 v4l2_fourcc('S', '4', '0', '1') /* se401 janggu compressed rgb */
> #define V4L2_PIX_FMT_S5C_UYVY_JPG v4l2_fourcc('S', '5', 'C', 'I') /* S5C73M3 interleaved UYVY/JPEG */
>
> +/* SDR */
> +#define V4L2_PIX_FMT_FLOAT v4l2_fourcc('D', 'F', '3', '2') /* float 32-bit */
> +#define V4L2_PIX_FMT_U8 v4l2_fourcc('D', 'U', '0', '8') /* unsigned 8-bit */
> +
> /*
> * F O R M A T E N U M E R A T I O N
> */
> diff --git a/lib/libv4lconvert/libv4lconvert.c b/lib/libv4lconvert/libv4lconvert.c
> index e2afc27..38c9125 100644
> --- a/lib/libv4lconvert/libv4lconvert.c
> +++ b/lib/libv4lconvert/libv4lconvert.c
> @@ -78,7 +78,8 @@ static void v4lconvert_get_framesizes(struct v4lconvert_data *data,
> { V4L2_PIX_FMT_RGB24, 24, 1, 5, 0 }, \
> { V4L2_PIX_FMT_BGR24, 24, 1, 5, 0 }, \
> { V4L2_PIX_FMT_YUV420, 12, 6, 1, 0 }, \
> - { V4L2_PIX_FMT_YVU420, 12, 6, 1, 0 }
> + { V4L2_PIX_FMT_YVU420, 12, 6, 1, 0 }, \
> + { V4L2_PIX_FMT_FLOAT, 0, 0, 0, 0 }
>
This looks wrong, here you claim that V4L2_PIX_FMT_FLOAT is a supported destination
format. which suggests there will be conversion code from any of the
supported_src_pixfmts to it, which you don't add (and I don't think we will want
to add.
> static const struct v4lconvert_pixfmt supported_src_pixfmts[] = {
> SUPPORTED_DST_PIXFMTS,
> @@ -131,6 +132,8 @@ static const struct v4lconvert_pixfmt supported_src_pixfmts[] = {
> { V4L2_PIX_FMT_Y6, 8, 20, 20, 0 },
> { V4L2_PIX_FMT_Y10BPACK, 10, 20, 20, 0 },
> { V4L2_PIX_FMT_Y16, 16, 20, 20, 0 },
> + /* SDR formats */
> + { V4L2_PIX_FMT_U8, 0, 0, 0, 0 },
> };
Likewise this will tell libv4lconvert that it can convert from V4L2_PIX_FMT_U8 to
any of the supported destination formats, which again is not true.
I suggest simply adding a hardcoded test for the SDR formats to relevant code paths
which use supported_src_pixfmts and when seeing V4L2_PIX_FMT_U8 as source only
support V4L2_PIX_FMT_FLOAT as dest, and short-circuit a whole bunch of other tests
done.
>
> static const struct v4lconvert_pixfmt supported_dst_pixfmts[] = {
> @@ -1281,6 +1284,25 @@ static int v4lconvert_convert_pixfmt(struct v4lconvert_data *data,
> }
> break;
>
> + /* SDR */
> + case V4L2_PIX_FMT_U8:
> + switch (dest_pix_fmt) {
> + case V4L2_PIX_FMT_FLOAT:
> + {
> + /* 8-bit unsigned to 32-bit float */
> + unsigned int i;
> + float ftmp;
> + for (i = 0; i < src_size; i++) {
> + ftmp = *src++;
> + ftmp -= 127.5;
> + ftmp /= 127.5;
> + memcpy(dest, &ftmp, 4);
> + dest += 4;
> + }
> + }
> + }
> + break;
> +
> default:
> V4LCONVERT_ERR("Unknown src format in conversion\n");
> errno = EINVAL;
> @@ -1349,6 +1371,11 @@ int v4lconvert_convert(struct v4lconvert_data *data,
> temp_needed =
> my_src_fmt.fmt.pix.width * my_src_fmt.fmt.pix.height * 3 / 2;
> break;
> + /* SDR */
> + case V4L2_PIX_FMT_FLOAT:
> + dest_needed = src_size * 4; /* 8-bit to 32-bit */
> + temp_needed = dest_needed;
> + break;
> default:
> V4LCONVERT_ERR("Unknown dest format in conversion\n");
> errno = EINVAL;
>
Regards,
Hans
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH RFC] libv4lconvert: SDR conversion from U8 to FLOAT
2013-11-16 17:27 ` Hans de Goede
@ 2013-11-16 17:34 ` Antti Palosaari
0 siblings, 0 replies; 16+ messages in thread
From: Antti Palosaari @ 2013-11-16 17:34 UTC (permalink / raw)
To: Hans de Goede, linux-media
Hello Hans!
Actually I has already fixed version according to Andy and Hans V.
comments, which I was just planning to send for you too, just due to
that issue format selection issue!
On 16.11.2013 19:27, Hans de Goede wrote:
> Hi,
>> @@ -78,7 +78,8 @@ static void v4lconvert_get_framesizes(struct
>> v4lconvert_data *data,
>> { V4L2_PIX_FMT_RGB24, 24, 1, 5, 0 }, \
>> { V4L2_PIX_FMT_BGR24, 24, 1, 5, 0 }, \
>> { V4L2_PIX_FMT_YUV420, 12, 6, 1, 0 }, \
>> - { V4L2_PIX_FMT_YVU420, 12, 6, 1, 0 }
>> + { V4L2_PIX_FMT_YVU420, 12, 6, 1, 0 }, \
>> + { V4L2_PIX_FMT_FLOAT, 0, 0, 0, 0 }
>>
>
> This looks wrong, here you claim that V4L2_PIX_FMT_FLOAT is a supported
> destination
> format. which suggests there will be conversion code from any of the
> supported_src_pixfmts to it, which you don't add (and I don't think we
> will want
> to add.
>
>> static const struct v4lconvert_pixfmt supported_src_pixfmts[] = {
>> SUPPORTED_DST_PIXFMTS,
>> @@ -131,6 +132,8 @@ static const struct v4lconvert_pixfmt
>> supported_src_pixfmts[] = {
>> { V4L2_PIX_FMT_Y6, 8, 20, 20, 0 },
>> { V4L2_PIX_FMT_Y10BPACK, 10, 20, 20, 0 },
>> { V4L2_PIX_FMT_Y16, 16, 20, 20, 0 },
>> + /* SDR formats */
>> + { V4L2_PIX_FMT_U8, 0, 0, 0, 0 },
>> };
>
> Likewise this will tell libv4lconvert that it can convert from
> V4L2_PIX_FMT_U8 to
> any of the supported destination formats, which again is not true.
>
> I suggest simply adding a hardcoded test for the SDR formats to relevant
> code paths
> which use supported_src_pixfmts and when seeing V4L2_PIX_FMT_U8 as
> source only
> support V4L2_PIX_FMT_FLOAT as dest, and short-circuit a whole bunch of
> other tests
> done.
Sounds reasonable. I will try implement it after I do some more
conversions tests.
regards
Antti
--
http://palosaari.fi/
^ permalink raw reply [flat|nested] 16+ messages in thread