Message ID | 20190812153953.11839-2-lance.lmwang@gmail.com |
---|---|
State | Superseded |
Headers | show |
On Mon, Aug 12, 2019 at 23:39:52 +0800, lance.lmwang@gmail.com wrote: This looks very wrong. Does it work? > - 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); > + frame = frame/test->max_frames; Here, you reduce frame to frame/test->max_frames. > + if (tt == TEST_ALL && frame) /* draw a black frame at the beginning of each test */ > + tt = frame%(TEST_NB-1); Which makes this replacement correct. But: > - 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; > - case TEST_FREQ_CHROMA: freq_test(picref->data[1], picref->linesize[1], frame%test->max_frames); break; > - case TEST_AMP_LUMA: amp_test(picref->data[0], picref->linesize[0], frame%test->max_frames); break; > - case TEST_AMP_CHROMA: amp_test(picref->data[1], picref->linesize[1], frame%test->max_frames); break; > - case TEST_CBP: cbp_test(picref->data , picref->linesize , frame%test->max_frames); break; > - case TEST_MV: mv_test(picref->data[0], picref->linesize[0], frame%test->max_frames); break; > - case TEST_RING1: ring1_test(picref->data[0], picref->linesize[0], frame%test->max_frames); break; > - case TEST_RING2: ring2_test(picref->data[0], picref->linesize[0], frame%test->max_frames); break; > + case TEST_DC_LUMA: dc_test(picref->data[0], picref->linesize[0], 256, 256, frame); break; > + case TEST_DC_CHROMA: dc_test(picref->data[1], picref->linesize[1], 256, 256, frame); break; > + case TEST_FREQ_LUMA: freq_test(picref->data[0], picref->linesize[0], frame); break; > + case TEST_FREQ_CHROMA: freq_test(picref->data[1], picref->linesize[1], frame); break; > + case TEST_AMP_LUMA: amp_test(picref->data[0], picref->linesize[0], frame); break; > + case TEST_AMP_CHROMA: amp_test(picref->data[1], picref->linesize[1], frame); break; > + case TEST_CBP: cbp_test(picref->data , picref->linesize , frame); break; > + case TEST_MV: mv_test(picref->data[0], picref->linesize[0], frame); break; > + case TEST_RING1: ring1_test(picref->data[0], picref->linesize[0], frame); break; > + case TEST_RING2: ring2_test(picref->data[0], picref->linesize[0], frame); break; Here, you have effectively replaced "frame%test->max_frames" with "frame/test->max_frames" (now "frame"). Are you sure? Cheers, Moritz
On Thu, Aug 22, 2019 at 03:04:19PM +0200, Moritz Barsnick wrote: > On Mon, Aug 12, 2019 at 23:39:52 +0800, lance.lmwang@gmail.com wrote: > > This looks very wrong. Does it work? > > > - 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); > > + frame = frame/test->max_frames; > > Here, you reduce frame to frame/test->max_frames. good catch, it's wrong, it's simple replacement, so haven't check it carefully. Now I have fixed it and update the patch. Have checked with ffplay. > > > + if (tt == TEST_ALL && frame) /* draw a black frame at the beginning of each test */ > > + tt = frame%(TEST_NB-1); > > Which makes this replacement correct. But: > > > - 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; > > - case TEST_FREQ_CHROMA: freq_test(picref->data[1], picref->linesize[1], frame%test->max_frames); break; > > - case TEST_AMP_LUMA: amp_test(picref->data[0], picref->linesize[0], frame%test->max_frames); break; > > - case TEST_AMP_CHROMA: amp_test(picref->data[1], picref->linesize[1], frame%test->max_frames); break; > > - case TEST_CBP: cbp_test(picref->data , picref->linesize , frame%test->max_frames); break; > > - case TEST_MV: mv_test(picref->data[0], picref->linesize[0], frame%test->max_frames); break; > > - case TEST_RING1: ring1_test(picref->data[0], picref->linesize[0], frame%test->max_frames); break; > > - case TEST_RING2: ring2_test(picref->data[0], picref->linesize[0], frame%test->max_frames); break; > > + case TEST_DC_LUMA: dc_test(picref->data[0], picref->linesize[0], 256, 256, frame); break; > > + case TEST_DC_CHROMA: dc_test(picref->data[1], picref->linesize[1], 256, 256, frame); break; > > + case TEST_FREQ_LUMA: freq_test(picref->data[0], picref->linesize[0], frame); break; > > + case TEST_FREQ_CHROMA: freq_test(picref->data[1], picref->linesize[1], frame); break; > > + case TEST_AMP_LUMA: amp_test(picref->data[0], picref->linesize[0], frame); break; > > + case TEST_AMP_CHROMA: amp_test(picref->data[1], picref->linesize[1], frame); break; > > + case TEST_CBP: cbp_test(picref->data , picref->linesize , frame); break; > > + case TEST_MV: mv_test(picref->data[0], picref->linesize[0], frame); break; > > + case TEST_RING1: ring1_test(picref->data[0], picref->linesize[0], frame); break; > > + case TEST_RING2: ring2_test(picref->data[0], picref->linesize[0], frame); break; > > Here, you have effectively replaced "frame%test->max_frames" with > "frame/test->max_frames" (now "frame"). Are you sure? > > Cheers, > Moritz > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff --git a/libavfilter/vsrc_mptestsrc.c b/libavfilter/vsrc_mptestsrc.c index 4a2db18..14e66fd 100644 --- a/libavfilter/vsrc_mptestsrc.c +++ b/libavfilter/vsrc_mptestsrc.c @@ -308,7 +308,7 @@ static int request_frame(AVFilterLink *outlink) AVFrame *picref; int w = WIDTH, h = HEIGHT, cw = AV_CEIL_RSHIFT(w, test->hsub), ch = AV_CEIL_RSHIFT(h, test->vsub); - unsigned int frame = outlink->frame_count_in; + uint64_t frame = outlink->frame_count_in; enum test_type tt = test->test; int i; @@ -327,20 +327,21 @@ static int request_frame(AVFilterLink *outlink) memset(picref->data[2] + i*picref->linesize[2], 128, cw); } - 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); + frame = frame/test->max_frames; + if (tt == TEST_ALL && frame) /* draw a black frame at the beginning of each test */ + tt = frame%(TEST_NB-1); switch (tt) { - 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; - case TEST_FREQ_CHROMA: freq_test(picref->data[1], picref->linesize[1], frame%test->max_frames); break; - case TEST_AMP_LUMA: amp_test(picref->data[0], picref->linesize[0], frame%test->max_frames); break; - case TEST_AMP_CHROMA: amp_test(picref->data[1], picref->linesize[1], frame%test->max_frames); break; - case TEST_CBP: cbp_test(picref->data , picref->linesize , frame%test->max_frames); break; - case TEST_MV: mv_test(picref->data[0], picref->linesize[0], frame%test->max_frames); break; - case TEST_RING1: ring1_test(picref->data[0], picref->linesize[0], frame%test->max_frames); break; - case TEST_RING2: ring2_test(picref->data[0], picref->linesize[0], frame%test->max_frames); break; + case TEST_DC_LUMA: dc_test(picref->data[0], picref->linesize[0], 256, 256, frame); break; + case TEST_DC_CHROMA: dc_test(picref->data[1], picref->linesize[1], 256, 256, frame); break; + case TEST_FREQ_LUMA: freq_test(picref->data[0], picref->linesize[0], frame); break; + case TEST_FREQ_CHROMA: freq_test(picref->data[1], picref->linesize[1], frame); break; + case TEST_AMP_LUMA: amp_test(picref->data[0], picref->linesize[0], frame); break; + case TEST_AMP_CHROMA: amp_test(picref->data[1], picref->linesize[1], frame); break; + case TEST_CBP: cbp_test(picref->data , picref->linesize , frame); break; + case TEST_MV: mv_test(picref->data[0], picref->linesize[0], frame); break; + case TEST_RING1: ring1_test(picref->data[0], picref->linesize[0], frame); break; + case TEST_RING2: ring2_test(picref->data[0], picref->linesize[0], frame); break; } return ff_filter_frame(outlink, picref);