On 24/10/2023 21:09, Detlev Casanova wrote:
> When running tests with different input data, the stable output frames
> could be too similar and hide possible issues.
>
> This commit adds variation by using some codec specific parameters.
>
> Only HEVC and H.264 support this.
>
> Reviewed-by: Daniel Almeida <daniel.almeida@xxxxxxxxxxxxx>
> Signed-off-by: Detlev Casanova <detlev.casanova@xxxxxxxxxxxxx>
> ---
> drivers/media/test-drivers/visl/visl-core.c | 5 ++++
> drivers/media/test-drivers/visl/visl-dec.c | 27 +++++++++++++++++++++
> drivers/media/test-drivers/visl/visl.h | 1 +
> 3 files changed, 33 insertions(+)
>
> diff --git a/drivers/media/test-drivers/visl/visl-core.c b/drivers/media/test-drivers/visl/visl-core.c
> index d28d50afec02..e7466f6a91e1 100644
> --- a/drivers/media/test-drivers/visl/visl-core.c
> +++ b/drivers/media/test-drivers/visl/visl-core.c
> @@ -93,6 +93,11 @@ module_param(stable_output, bool, 0644);
> MODULE_PARM_DESC(stable_output,
> " only write stable data for a given input on the output frames");
>
> +bool codec_variability;
> +module_param(codec_variability, bool, 0644);
> +MODULE_PARM_DESC(codec_variability,
> + " add codec specific variability data to generate more unique frames. (Only h.264 and hevc)");
Why make this a module parameter instead of always doing this?
It's not clear from the commit log why a parameter is needed.
> +
> static const struct visl_ctrl_desc visl_fwht_ctrl_descs[] = {
> {
> .cfg.id = V4L2_CID_STATELESS_FWHT_PARAMS,
> diff --git a/drivers/media/test-drivers/visl/visl-dec.c b/drivers/media/test-drivers/visl/visl-dec.c
> index 61cfca49ead9..002d5e3b0ea4 100644
> --- a/drivers/media/test-drivers/visl/visl-dec.c
> +++ b/drivers/media/test-drivers/visl/visl-dec.c
> @@ -223,6 +223,26 @@ static void visl_tpg_fill_sequence(struct visl_ctx *ctx,
> }
> }
>
> +static bool visl_tpg_fill_codec_specific(struct visl_ctx *ctx,
> + struct visl_run *run,
> + char buf[], size_t bufsz)
> +{
> + switch (ctx->current_codec) {
> + case VISL_CODEC_H264:
> + scnprintf(buf, bufsz,
> + "H264: %u", run->h264.dpram->pic_order_cnt_lsb);
> + break;
> + case VISL_CODEC_HEVC:
> + scnprintf(buf, bufsz,
> + "HEVC: %d", run->hevc.dpram->pic_order_cnt_val);
> + break;
Perhaps mention here why these specific values are chosen?
> + default:
> + return false;
> + }
> +
> + return true;
> +}
> +
> static void visl_tpg_fill(struct visl_ctx *ctx, struct visl_run *run)
> {
> u8 *basep[TPG_MAX_PLANES][2];
> @@ -255,6 +275,13 @@ static void visl_tpg_fill(struct visl_ctx *ctx, struct visl_run *run)
> frame_dprintk(ctx->dev, run->dst->sequence, "");
> line++;
>
> + if (codec_variability && visl_tpg_fill_codec_specific(ctx, run, buf, TPG_STR_BUF_SZ)) {
> + tpg_gen_text(&ctx->tpg, basep, line++ * line_height, 16, buf);
> + frame_dprintk(ctx->dev, run->dst->sequence, "%s\n", buf);
> + frame_dprintk(ctx->dev, run->dst->sequence, "");
> + line++;
> + }
> +
> if (!stable_output) {
> visl_get_ref_frames(ctx, buf, TPG_STR_BUF_SZ, run);
>
> diff --git a/drivers/media/test-drivers/visl/visl.h b/drivers/media/test-drivers/visl/visl.h
> index 5a81b493f121..4ac2d1783020 100644
> --- a/drivers/media/test-drivers/visl/visl.h
> +++ b/drivers/media/test-drivers/visl/visl.h
> @@ -86,6 +86,7 @@ extern bool keep_bitstream_buffers;
> extern int bitstream_trace_frame_start;
> extern unsigned int bitstream_trace_nframes;
> extern bool stable_output;
> +extern bool codec_variability;
>
> #define frame_dprintk(dev, current, fmt, arg...) \
> do { \
Regards,
Hans
