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

Limin Wang lance.lmwang at gmail.com
Mon Aug 12 18:45:42 EEST 2019


On Mon, Aug 12, 2019 at 03:05:32PM +0200, Moritz Barsnick wrote:
> 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.

OK, fix it. It'll cause the subject change, so it'll start with new
thread:
https://patchwork.ffmpeg.org/patch/14439/

> 
> > +    { "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.

OK, fix them and send v2 patch.

> 
> > +    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.)
> 
Add patch#3 to simpilify it, change the type for frame and reuse it.

> Cheers,
> Moritz
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".


More information about the ffmpeg-devel mailing list