[FFmpeg-devel] [PATCH v1 1/2] avfilter/vsrc_mptestsrc: add option to set the max number frames generated for each tests

[Moritz Barsnick]

On Mon, Aug 12, 2019 at 19:21:45 +0800, lance.lmwang at gmail.com wrote:
> +Set the max number frames generated for each tests:

Grammar nit:
  Set the maximum number of frames generated for each test

(Note:
 - use the complete word maximum
 - "test", "not tests", when using "for each" singular
 - *of* frames.

> +    { "max_frames", "Set the max number frames generated for each tests", OFFSET(max_frames), AV_OPT_TYPE_INT, {.i64 = 30}, 1, INT64_MAX, FLAGS },
> +    { "m",          " Set the max number frames generated for each tests", OFFSET(max_frames), AV_OPT_TYPE_INT, {.i64 = 30}, 1, INT64_MAX, FLAGS },

Same here. Also note that you introduced an additional space in the
second string which doesn't belong there.

> +    if (tt == TEST_ALL && frame%test->max_frames) /* draw a black frame at the beginning of each test */
> +        tt = (frame/test->max_frames)%(TEST_NB-1);
[...]
> +    case TEST_DC_LUMA:       dc_test(picref->data[0], picref->linesize[0], 256, 256, frame%test->max_frames); break;
> +    case TEST_DC_CHROMA:     dc_test(picref->data[1], picref->linesize[1], 256, 256, frame%test->max_frames); break;
> +    case TEST_FREQ_LUMA:   freq_test(picref->data[0], picref->linesize[0], frame%test->max_frames); break;
[...]

frame%test->max_frames could probably be assigned to a variable, that
might avoid a lot of code, unless compilers know how to optimize this.
(Might not belong into this functional patch though.)

Cheers,
Moritz