Hi Kaaira, On 14/06/2020 21:02, Kaaira Gupta wrote: > Currently there is no method to know the correct order of the colors for > a test image generated by tpg. Write a function that returns a string of > colors' order given a tpg. It returns a NULL pointer in case of test > patterns which do not have a well defined colors' order. Hence add a > NULL check for text in tpg_gen_text(). > > Signed-off-by: Kaaira Gupta <kgupta@xxxxxxxxxxxxx> > --- > drivers/media/common/v4l2-tpg/v4l2-tpg-core.c | 32 +++++++++++++++++-- > include/media/tpg/v4l2-tpg.h | 1 + > 2 files changed, 31 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c b/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c > index 50f1e0b28b25..c8257e860c6e 100644 > --- a/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c > +++ b/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c > @@ -1959,12 +1959,14 @@ void tpg_gen_text(const struct tpg_data *tpg, u8 *basep[TPG_MAX_PLANES][2], > unsigned step = V4L2_FIELD_HAS_T_OR_B(tpg->field) ? 2 : 1; > unsigned div = step; > unsigned first = 0; > - unsigned len = strlen(text); > + unsigned len; > unsigned p; > > - if (font8x16 == NULL || basep == NULL) > + if (font8x16 == NULL || basep == NULL || text == NULL) > return; > > + len = strlen(text); > + > /* Checks if it is possible to show string */ > if (y + 16 >= tpg->compose.height || x + 8 >= tpg->compose.width) > return; > @@ -2006,6 +2008,32 @@ void tpg_gen_text(const struct tpg_data *tpg, u8 *basep[TPG_MAX_PLANES][2], > } > EXPORT_SYMBOL_GPL(tpg_gen_text); > > +char *tpg_g_color_order(const struct tpg_data *tpg) > +{ > + switch (tpg->pattern) { > + case TPG_PAT_75_COLORBAR: > + return "Left to right: white, yellow, cyan, green, magenta, red, blue, black"; > + case TPG_PAT_100_COLORBAR: > + return "Left to right: white, yellow, cyan, green, magenta, red, blue, black"; > + case TPG_PAT_CSC_COLORBAR: > + return "Left to right: white, yellow, cyan, green, magenta, red, blue, black"; Those three options return identical strings. In C you can 'fallthrough' on case statements like this: > switch (tpg->pattern) { > case TPG_PAT_75_COLORBAR: > case TPG_PAT_100_COLORBAR: > case TPG_PAT_CSC_COLORBAR: > return "Left to right: white, yellow, cyan, green, magenta, red, blue, black"; So all three of those options will go to the same return statement. > + case TPG_PAT_100_HCOLORBAR: > + return "Top to bottom: white, yellow, cyan, green, magenta, red, blue, black"; It would be nice to be able to factor out the common color sequence from those (two remaining) strings too, as only the direction changes. But perhaps it's a pain to have to do a string join in this call, especially given it's static string data otherwise, but that's also a good reason not to duplicate unnecessarily (as it wastes space in memory). With those fixed, Reviewed-by: Kieran Bingham <kieran.bingham@xxxxxxxxxxxxxxxx> > + case TPG_PAT_BLACK: > + return "Black"; > + case TPG_PAT_WHITE: > + return "White"; > + case TPG_PAT_RED: > + return "Red"; > + case TPG_PAT_GREEN: > + return "Green"; > + case TPG_PAT_BLUE: > + return "Blue"; > + default: > + return NULL; > + } > +} > + > void tpg_update_mv_step(struct tpg_data *tpg) > { > int factor = tpg->mv_hor_mode > TPG_MOVE_NONE ? -1 : 1; > diff --git a/include/media/tpg/v4l2-tpg.h b/include/media/tpg/v4l2-tpg.h > index eb191e85d363..4f79cac87b85 100644 > --- a/include/media/tpg/v4l2-tpg.h > +++ b/include/media/tpg/v4l2-tpg.h > @@ -252,6 +252,7 @@ void tpg_fillbuffer(struct tpg_data *tpg, v4l2_std_id std, > bool tpg_s_fourcc(struct tpg_data *tpg, u32 fourcc); > void tpg_s_crop_compose(struct tpg_data *tpg, const struct v4l2_rect *crop, > const struct v4l2_rect *compose); > +char *tpg_g_color_order(const struct tpg_data *tpg); > > static inline void tpg_s_pattern(struct tpg_data *tpg, enum tpg_pattern pattern) > { > -- Regards -- Kieran