Currently there is no method to know if the test image generated by vimc
is correct (except for comparing it with a known 'correct' image). Add
text over the test image, representing the correct order of colors.
I have sent it as an RFC because we can add the text as an optional
control, and maybe we can print some other useful information as well
(like vivid does).
Signed-off-by: Kaaira Gupta <[email protected]>
---
drivers/media/test-drivers/vimc/Kconfig | 2 ++
drivers/media/test-drivers/vimc/vimc-core.c | 9 +++++++++
drivers/media/test-drivers/vimc/vimc-sensor.c | 8 ++++++++
3 files changed, 19 insertions(+)
diff --git a/drivers/media/test-drivers/vimc/Kconfig b/drivers/media/test-drivers/vimc/Kconfig
index 4068a67585f9..da4b2ad6e40c 100644
--- a/drivers/media/test-drivers/vimc/Kconfig
+++ b/drivers/media/test-drivers/vimc/Kconfig
@@ -2,6 +2,8 @@
config VIDEO_VIMC
tristate "Virtual Media Controller Driver (VIMC)"
depends on VIDEO_DEV && VIDEO_V4L2
+ select FONT_SUPPORT
+ select FONT_8x16
select MEDIA_CONTROLLER
select VIDEO_V4L2_SUBDEV_API
select VIDEOBUF2_VMALLOC
diff --git a/drivers/media/test-drivers/vimc/vimc-core.c b/drivers/media/test-drivers/vimc/vimc-core.c
index 11210aaa2551..8142bfbcbd49 100644
--- a/drivers/media/test-drivers/vimc/vimc-core.c
+++ b/drivers/media/test-drivers/vimc/vimc-core.c
@@ -5,10 +5,12 @@
* Copyright (C) 2015-2017 Helen Koike <[email protected]>
*/
+#include <linux/font.h>
#include <linux/init.h>
#include <linux/module.h>
#include <linux/platform_device.h>
#include <media/media-device.h>
+#include <media/tpg/v4l2-tpg.h>
#include <media/v4l2-device.h>
#include "vimc-common.h"
@@ -265,7 +267,14 @@ static int vimc_probe(struct platform_device *pdev)
{
struct vimc_device *vimc;
int ret;
+ const struct font_desc *font = find_font("VGA8x16");
+ if (font == NULL) {
+ pr_err("vimc: could not find font\n");
+ return -ENODEV;
+ }
+
+ tpg_set_font(font->data);
dev_dbg(&pdev->dev, "probe");
vimc = kzalloc(sizeof(*vimc), GFP_KERNEL);
diff --git a/drivers/media/test-drivers/vimc/vimc-sensor.c b/drivers/media/test-drivers/vimc/vimc-sensor.c
index a2f09ac9a360..4b13955c502a 100644
--- a/drivers/media/test-drivers/vimc/vimc-sensor.c
+++ b/drivers/media/test-drivers/vimc/vimc-sensor.c
@@ -185,10 +185,18 @@ static const struct v4l2_subdev_pad_ops vimc_sen_pad_ops = {
static void *vimc_sen_process_frame(struct vimc_ent_device *ved,
const void *sink_frame)
{
+ u8 *basep[TPG_MAX_PLANES][2];
+ char str[100];
struct vimc_sen_device *vsen = container_of(ved, struct vimc_sen_device,
ved);
+ tpg_calc_text_basep(&vsen->tpg, basep, 0, vsen->frame);
tpg_fill_plane_buffer(&vsen->tpg, 0, 0, vsen->frame);
+
+ snprintf(str, sizeof(str),
+ "Order: white, yellow, cyan, green, magenta, red, blue, black");
+ tpg_gen_text(&vsen->tpg, basep, 1, 1, str);
+
return vsen->frame;
}
--
2.17.1
Hi,
On 07.06.20 15:53, Kaaira Gupta wrote:
> Currently there is no method to know if the test image generated by vimc
> is correct (except for comparing it with a known 'correct' image). Add
> text over the test image, representing the correct order of colors.
>
> I have sent it as an RFC because we can add the text as an optional
> control, and maybe we can print some other useful information as well
> (like vivid does).
Yes, it seems like a good idea to add it as a control of the sensor.
>
> Signed-off-by: Kaaira Gupta <[email protected]>
> ---> drivers/media/test-drivers/vimc/Kconfig | 2 ++
> drivers/media/test-drivers/vimc/vimc-core.c | 9 +++++++++
> drivers/media/test-drivers/vimc/vimc-sensor.c | 8 ++++++++
> 3 files changed, 19 insertions(+)
>
> diff --git a/drivers/media/test-drivers/vimc/Kconfig b/drivers/media/test-drivers/vimc/Kconfig
> index 4068a67585f9..da4b2ad6e40c 100644
> --- a/drivers/media/test-drivers/vimc/Kconfig
> +++ b/drivers/media/test-drivers/vimc/Kconfig
> @@ -2,6 +2,8 @@
> config VIDEO_VIMC
> tristate "Virtual Media Controller Driver (VIMC)"
> depends on VIDEO_DEV && VIDEO_V4L2
> + select FONT_SUPPORT
> + select FONT_8x16
> select MEDIA_CONTROLLER
> select VIDEO_V4L2_SUBDEV_API
> select VIDEOBUF2_VMALLOC
> diff --git a/drivers/media/test-drivers/vimc/vimc-core.c b/drivers/media/test-drivers/vimc/vimc-core.c
> index 11210aaa2551..8142bfbcbd49 100644
> --- a/drivers/media/test-drivers/vimc/vimc-core.c
> +++ b/drivers/media/test-drivers/vimc/vimc-core.c
> @@ -5,10 +5,12 @@
> * Copyright (C) 2015-2017 Helen Koike <[email protected]>
> */
>
> +#include <linux/font.h>
> #include <linux/init.h>
> #include <linux/module.h>
> #include <linux/platform_device.h>
> #include <media/media-device.h>
> +#include <media/tpg/v4l2-tpg.h>
> #include <media/v4l2-device.h>
>
> #include "vimc-common.h"
> @@ -265,7 +267,14 @@ static int vimc_probe(struct platform_device *pdev)
> {
> struct vimc_device *vimc;
> int ret;
> + const struct font_desc *font = find_font("VGA8x16");
>
> + if (font == NULL) {
> + pr_err("vimc: could not find font\n");
> + return -ENODEV;
> + }
> +
> + tpg_set_font(font->data);
I think the code that set the format should move to the
code that registers the sensor in vimc-sensor.c
> dev_dbg(&pdev->dev, "probe");
>
> vimc = kzalloc(sizeof(*vimc), GFP_KERNEL);
> diff --git a/drivers/media/test-drivers/vimc/vimc-sensor.c b/drivers/media/test-drivers/vimc/vimc-sensor.c
> index a2f09ac9a360..4b13955c502a 100644
> --- a/drivers/media/test-drivers/vimc/vimc-sensor.c
> +++ b/drivers/media/test-drivers/vimc/vimc-sensor.c
> @@ -185,10 +185,18 @@ static const struct v4l2_subdev_pad_ops vimc_sen_pad_ops = {
> static void *vimc_sen_process_frame(struct vimc_ent_device *ved,
> const void *sink_frame)
> {
> + u8 *basep[TPG_MAX_PLANES][2];
> + char str[100];
> struct vimc_sen_device *vsen = container_of(ved, struct vimc_sen_device,
> ved);
>
> + tpg_calc_text_basep(&vsen->tpg, basep, 0, vsen->frame);
> tpg_fill_plane_buffer(&vsen->tpg, 0, 0, vsen->frame);
> +
> + snprintf(str, sizeof(str),
> + "Order: white, yellow, cyan, green, magenta, red, blue, black");
The colors are generated by the tpg, so I think it should be a feature of the tpg to print the colors.
For example, a function in v4l2-tpg-core.c that get the pattern as an argument and return
this string, or maybe returns a const pointer to the array of colors, or something like that.
Then maybe we can add a control in vivid for the same tpg feature.
Note also that the sensor has a control to change the pattern: vimc_sen_ctrl_test_pattern
So the string depends on that pattern.
Thanks,
Dafna
> + tpg_gen_text(&vsen->tpg, basep, 1, 1, str);
> +
> return vsen->frame;
> }
>
>
On 08/06/2020 09:10, Dafna Hirschfeld wrote:
> Hi,
>
> On 07.06.20 15:53, Kaaira Gupta wrote:
>> Currently there is no method to know if the test image generated by vimc
>> is correct (except for comparing it with a known 'correct' image). Add
>> text over the test image, representing the correct order of colors.
>>
>> I have sent it as an RFC because we can add the text as an optional
>> control, and maybe we can print some other useful information as well
>> (like vivid does).
>
> Yes, it seems like a good idea to add it as a control of the sensor.
>
>>
>> Signed-off-by: Kaaira Gupta <[email protected]>
>> ---> drivers/media/test-drivers/vimc/Kconfig | 2 ++
>> drivers/media/test-drivers/vimc/vimc-core.c | 9 +++++++++
>> drivers/media/test-drivers/vimc/vimc-sensor.c | 8 ++++++++
>> 3 files changed, 19 insertions(+)
>>
>> diff --git a/drivers/media/test-drivers/vimc/Kconfig b/drivers/media/test-drivers/vimc/Kconfig
>> index 4068a67585f9..da4b2ad6e40c 100644
>> --- a/drivers/media/test-drivers/vimc/Kconfig
>> +++ b/drivers/media/test-drivers/vimc/Kconfig
>> @@ -2,6 +2,8 @@
>> config VIDEO_VIMC
>> tristate "Virtual Media Controller Driver (VIMC)"
>> depends on VIDEO_DEV && VIDEO_V4L2
>> + select FONT_SUPPORT
>> + select FONT_8x16
>> select MEDIA_CONTROLLER
>> select VIDEO_V4L2_SUBDEV_API
>> select VIDEOBUF2_VMALLOC
>> diff --git a/drivers/media/test-drivers/vimc/vimc-core.c b/drivers/media/test-drivers/vimc/vimc-core.c
>> index 11210aaa2551..8142bfbcbd49 100644
>> --- a/drivers/media/test-drivers/vimc/vimc-core.c
>> +++ b/drivers/media/test-drivers/vimc/vimc-core.c
>> @@ -5,10 +5,12 @@
>> * Copyright (C) 2015-2017 Helen Koike <[email protected]>
>> */
>>
>> +#include <linux/font.h>
>> #include <linux/init.h>
>> #include <linux/module.h>
>> #include <linux/platform_device.h>
>> #include <media/media-device.h>
>> +#include <media/tpg/v4l2-tpg.h>
>> #include <media/v4l2-device.h>
>>
>> #include "vimc-common.h"
>> @@ -265,7 +267,14 @@ static int vimc_probe(struct platform_device *pdev)
>> {
>> struct vimc_device *vimc;
>> int ret;
>> + const struct font_desc *font = find_font("VGA8x16");
>>
>> + if (font == NULL) {
>> + pr_err("vimc: could not find font\n");
>> + return -ENODEV;
>> + }
>> +
>> + tpg_set_font(font->data);
>
> I think the code that set the format should move to the
> code that registers the sensor in vimc-sensor.c
>
>> dev_dbg(&pdev->dev, "probe");
>>
>> vimc = kzalloc(sizeof(*vimc), GFP_KERNEL);
>> diff --git a/drivers/media/test-drivers/vimc/vimc-sensor.c b/drivers/media/test-drivers/vimc/vimc-sensor.c
>> index a2f09ac9a360..4b13955c502a 100644
>> --- a/drivers/media/test-drivers/vimc/vimc-sensor.c
>> +++ b/drivers/media/test-drivers/vimc/vimc-sensor.c
>> @@ -185,10 +185,18 @@ static const struct v4l2_subdev_pad_ops vimc_sen_pad_ops = {
>> static void *vimc_sen_process_frame(struct vimc_ent_device *ved,
>> const void *sink_frame)
>> {
>> + u8 *basep[TPG_MAX_PLANES][2];
>> + char str[100];
>> struct vimc_sen_device *vsen = container_of(ved, struct vimc_sen_device,
>> ved);
>>
>> + tpg_calc_text_basep(&vsen->tpg, basep, 0, vsen->frame);
>> tpg_fill_plane_buffer(&vsen->tpg, 0, 0, vsen->frame);
>> +
>> + snprintf(str, sizeof(str),
>> + "Order: white, yellow, cyan, green, magenta, red, blue, black");
> The colors are generated by the tpg, so I think it should be a feature of the tpg to print the colors.
I agree. The tpg knows where each color is and based on the width and height it can
print the text. A tpg_gen_pattern_text() function that does the work would be very
nice.
It also doesn't make sense for all patterns, so this is really a nice feature to
incorporate into the TPG itself and enable it via a vivid and vimc control.
Regards,
Hans
>
> For example, a function in v4l2-tpg-core.c that get the pattern as an argument and return
> this string, or maybe returns a const pointer to the array of colors, or something like that.
> Then maybe we can add a control in vivid for the same tpg feature.
>
> Note also that the sensor has a control to change the pattern: vimc_sen_ctrl_test_pattern
> So the string depends on that pattern.
>
> Thanks,
> Dafna
>
>
>> + tpg_gen_text(&vsen->tpg, basep, 1, 1, str);
>> +
>> return vsen->frame;
>> }
>>
>>
Hello,
On Mon, Jun 08, 2020 at 11:33:21AM +0200, Hans Verkuil wrote:
> On 08/06/2020 09:10, Dafna Hirschfeld wrote:
> > On 07.06.20 15:53, Kaaira Gupta wrote:
> >> Currently there is no method to know if the test image generated by vimc
> >> is correct (except for comparing it with a known 'correct' image). Add
> >> text over the test image, representing the correct order of colors.
> >>
> >> I have sent it as an RFC because we can add the text as an optional
> >> control, and maybe we can print some other useful information as well
> >> (like vivid does).
> >
> > Yes, it seems like a good idea to add it as a control of the sensor.
> >
> >> Signed-off-by: Kaaira Gupta <[email protected]>
> >> ---> drivers/media/test-drivers/vimc/Kconfig | 2 ++
> >> drivers/media/test-drivers/vimc/vimc-core.c | 9 +++++++++
> >> drivers/media/test-drivers/vimc/vimc-sensor.c | 8 ++++++++
> >> 3 files changed, 19 insertions(+)
> >>
> >> diff --git a/drivers/media/test-drivers/vimc/Kconfig b/drivers/media/test-drivers/vimc/Kconfig
> >> index 4068a67585f9..da4b2ad6e40c 100644
> >> --- a/drivers/media/test-drivers/vimc/Kconfig
> >> +++ b/drivers/media/test-drivers/vimc/Kconfig
> >> @@ -2,6 +2,8 @@
> >> config VIDEO_VIMC
> >> tristate "Virtual Media Controller Driver (VIMC)"
> >> depends on VIDEO_DEV && VIDEO_V4L2
> >> + select FONT_SUPPORT
> >> + select FONT_8x16
> >> select MEDIA_CONTROLLER
> >> select VIDEO_V4L2_SUBDEV_API
> >> select VIDEOBUF2_VMALLOC
> >> diff --git a/drivers/media/test-drivers/vimc/vimc-core.c b/drivers/media/test-drivers/vimc/vimc-core.c
> >> index 11210aaa2551..8142bfbcbd49 100644
> >> --- a/drivers/media/test-drivers/vimc/vimc-core.c
> >> +++ b/drivers/media/test-drivers/vimc/vimc-core.c
> >> @@ -5,10 +5,12 @@
> >> * Copyright (C) 2015-2017 Helen Koike <[email protected]>
> >> */
> >>
> >> +#include <linux/font.h>
> >> #include <linux/init.h>
> >> #include <linux/module.h>
> >> #include <linux/platform_device.h>
> >> #include <media/media-device.h>
> >> +#include <media/tpg/v4l2-tpg.h>
> >> #include <media/v4l2-device.h>
> >>
> >> #include "vimc-common.h"
> >> @@ -265,7 +267,14 @@ static int vimc_probe(struct platform_device *pdev)
> >> {
> >> struct vimc_device *vimc;
> >> int ret;
> >> + const struct font_desc *font = find_font("VGA8x16");
> >>
> >> + if (font == NULL) {
> >> + pr_err("vimc: could not find font\n");
> >> + return -ENODEV;
> >> + }
> >> +
> >> + tpg_set_font(font->data);
> >
> > I think the code that set the format should move to the
> > code that registers the sensor in vimc-sensor.c
> >
> >> dev_dbg(&pdev->dev, "probe");
> >>
> >> vimc = kzalloc(sizeof(*vimc), GFP_KERNEL);
> >> diff --git a/drivers/media/test-drivers/vimc/vimc-sensor.c b/drivers/media/test-drivers/vimc/vimc-sensor.c
> >> index a2f09ac9a360..4b13955c502a 100644
> >> --- a/drivers/media/test-drivers/vimc/vimc-sensor.c
> >> +++ b/drivers/media/test-drivers/vimc/vimc-sensor.c
> >> @@ -185,10 +185,18 @@ static const struct v4l2_subdev_pad_ops vimc_sen_pad_ops = {
> >> static void *vimc_sen_process_frame(struct vimc_ent_device *ved,
> >> const void *sink_frame)
> >> {
> >> + u8 *basep[TPG_MAX_PLANES][2];
> >> + char str[100];
> >> struct vimc_sen_device *vsen = container_of(ved, struct vimc_sen_device,
> >> ved);
> >>
> >> + tpg_calc_text_basep(&vsen->tpg, basep, 0, vsen->frame);
> >> tpg_fill_plane_buffer(&vsen->tpg, 0, 0, vsen->frame);
> >> +
> >> + snprintf(str, sizeof(str),
> >> + "Order: white, yellow, cyan, green, magenta, red, blue, black");
> > The colors are generated by the tpg, so I think it should be a feature of the tpg to print the colors.
>
> I agree. The tpg knows where each color is and based on the width and height it can
> print the text. A tpg_gen_pattern_text() function that does the work would be very
> nice.
Could we go one step further, and print the colour name on each colour
bar ?
> It also doesn't make sense for all patterns, so this is really a nice feature to
> incorporate into the TPG itself and enable it via a vivid and vimc control.
>
> > For example, a function in v4l2-tpg-core.c that get the pattern as an argument and return
> > this string, or maybe returns a const pointer to the array of colors, or something like that.
> > Then maybe we can add a control in vivid for the same tpg feature.
> >
> > Note also that the sensor has a control to change the pattern: vimc_sen_ctrl_test_pattern
> > So the string depends on that pattern.
> >
> >> + tpg_gen_text(&vsen->tpg, basep, 1, 1, str);
> >> +
> >> return vsen->frame;
> >> }
> >>
--
Regards,
Laurent Pinchart
On 08/06/2020 14:12, Laurent Pinchart wrote:
> Hello,
>
> On Mon, Jun 08, 2020 at 11:33:21AM +0200, Hans Verkuil wrote:
>> On 08/06/2020 09:10, Dafna Hirschfeld wrote:
>>> On 07.06.20 15:53, Kaaira Gupta wrote:
>>>> Currently there is no method to know if the test image generated by vimc
>>>> is correct (except for comparing it with a known 'correct' image). Add
>>>> text over the test image, representing the correct order of colors.
>>>>
>>>> I have sent it as an RFC because we can add the text as an optional
>>>> control, and maybe we can print some other useful information as well
>>>> (like vivid does).
>>>
>>> Yes, it seems like a good idea to add it as a control of the sensor.
>>>
>>>> Signed-off-by: Kaaira Gupta <[email protected]>
>>>> ---> drivers/media/test-drivers/vimc/Kconfig | 2 ++
>>>> drivers/media/test-drivers/vimc/vimc-core.c | 9 +++++++++
>>>> drivers/media/test-drivers/vimc/vimc-sensor.c | 8 ++++++++
>>>> 3 files changed, 19 insertions(+)
>>>>
>>>> diff --git a/drivers/media/test-drivers/vimc/Kconfig b/drivers/media/test-drivers/vimc/Kconfig
>>>> index 4068a67585f9..da4b2ad6e40c 100644
>>>> --- a/drivers/media/test-drivers/vimc/Kconfig
>>>> +++ b/drivers/media/test-drivers/vimc/Kconfig
>>>> @@ -2,6 +2,8 @@
>>>> config VIDEO_VIMC
>>>> tristate "Virtual Media Controller Driver (VIMC)"
>>>> depends on VIDEO_DEV && VIDEO_V4L2
>>>> + select FONT_SUPPORT
>>>> + select FONT_8x16
>>>> select MEDIA_CONTROLLER
>>>> select VIDEO_V4L2_SUBDEV_API
>>>> select VIDEOBUF2_VMALLOC
>>>> diff --git a/drivers/media/test-drivers/vimc/vimc-core.c b/drivers/media/test-drivers/vimc/vimc-core.c
>>>> index 11210aaa2551..8142bfbcbd49 100644
>>>> --- a/drivers/media/test-drivers/vimc/vimc-core.c
>>>> +++ b/drivers/media/test-drivers/vimc/vimc-core.c
>>>> @@ -5,10 +5,12 @@
>>>> * Copyright (C) 2015-2017 Helen Koike <[email protected]>
>>>> */
>>>>
>>>> +#include <linux/font.h>
>>>> #include <linux/init.h>
>>>> #include <linux/module.h>
>>>> #include <linux/platform_device.h>
>>>> #include <media/media-device.h>
>>>> +#include <media/tpg/v4l2-tpg.h>
>>>> #include <media/v4l2-device.h>
>>>>
>>>> #include "vimc-common.h"
>>>> @@ -265,7 +267,14 @@ static int vimc_probe(struct platform_device *pdev)
>>>> {
>>>> struct vimc_device *vimc;
>>>> int ret;
>>>> + const struct font_desc *font = find_font("VGA8x16");
>>>>
>>>> + if (font == NULL) {
>>>> + pr_err("vimc: could not find font\n");
>>>> + return -ENODEV;
>>>> + }
>>>> +
>>>> + tpg_set_font(font->data);
>>>
>>> I think the code that set the format should move to the
>>> code that registers the sensor in vimc-sensor.c
>>>
>>>> dev_dbg(&pdev->dev, "probe");
>>>>
>>>> vimc = kzalloc(sizeof(*vimc), GFP_KERNEL);
>>>> diff --git a/drivers/media/test-drivers/vimc/vimc-sensor.c b/drivers/media/test-drivers/vimc/vimc-sensor.c
>>>> index a2f09ac9a360..4b13955c502a 100644
>>>> --- a/drivers/media/test-drivers/vimc/vimc-sensor.c
>>>> +++ b/drivers/media/test-drivers/vimc/vimc-sensor.c
>>>> @@ -185,10 +185,18 @@ static const struct v4l2_subdev_pad_ops vimc_sen_pad_ops = {
>>>> static void *vimc_sen_process_frame(struct vimc_ent_device *ved,
>>>> const void *sink_frame)
>>>> {
>>>> + u8 *basep[TPG_MAX_PLANES][2];
>>>> + char str[100];
>>>> struct vimc_sen_device *vsen = container_of(ved, struct vimc_sen_device,
>>>> ved);
>>>>
>>>> + tpg_calc_text_basep(&vsen->tpg, basep, 0, vsen->frame);
>>>> tpg_fill_plane_buffer(&vsen->tpg, 0, 0, vsen->frame);
>>>> +
>>>> + snprintf(str, sizeof(str),
>>>> + "Order: white, yellow, cyan, green, magenta, red, blue, black");
>>> The colors are generated by the tpg, so I think it should be a feature of the tpg to print the colors.
>>
>> I agree. The tpg knows where each color is and based on the width and height it can
>> print the text. A tpg_gen_pattern_text() function that does the work would be very
>> nice.
>
> Could we go one step further, and print the colour name on each colour
> bar ?
I think we mean the same thing: tpg_gen_pattern_text() would write the name of the
color on each color bar.
Regards,
Hans
>
>> It also doesn't make sense for all patterns, so this is really a nice feature to
>> incorporate into the TPG itself and enable it via a vivid and vimc control.
>>
>>> For example, a function in v4l2-tpg-core.c that get the pattern as an argument and return
>>> this string, or maybe returns a const pointer to the array of colors, or something like that.
>>> Then maybe we can add a control in vivid for the same tpg feature.
>>>
>>> Note also that the sensor has a control to change the pattern: vimc_sen_ctrl_test_pattern
>>> So the string depends on that pattern.
>>>
>>>> + tpg_gen_text(&vsen->tpg, basep, 1, 1, str);
>>>> +
>>>> return vsen->frame;
>>>> }
>>>>
>