| Message ID | 20200322155525.23410-2-ramiro.polla@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/2] avcodec/mpegvideo_enc: fix multi-threaded motion estimation rounding for mpeg4 | expand |
| Context | Check | Description |
|---|---|---|
| andriy/ffmpeg-patchwork | success | Make fate finished |
On Sun, Mar 22, 2020 at 04:55:25PM +0100, Ramiro Polla wrote: > In ff_estimate_b_frame_motion(), penalty_factor would be used before > being initialized in estimate_motion_b(). Also, the initialization > would happen more than once unnecessarily. > --- > libavcodec/motion_est.c | 15 ++++++++------- > tests/ref/vsynth/vsynth2-mpeg2-422 | 6 +++--- > tests/ref/vsynth/vsynth2-mpeg2-ivlc-qprd | 6 +++--- > tests/ref/vsynth/vsynth2-mpeg4-adap | 6 +++--- > 4 files changed, 17 insertions(+), 16 deletions(-) > > diff --git a/libavcodec/motion_est.c b/libavcodec/motion_est.c > index 02c75fd470..1feb46cec3 100644 > --- a/libavcodec/motion_est.c > +++ b/libavcodec/motion_est.c > @@ -1123,9 +1123,6 @@ static int estimate_motion_b(MpegEncContext *s, int mb_x, int mb_y, > uint8_t * const mv_penalty= c->mv_penalty[f_code] + MAX_DMV; > int mv_scale; > > - c->penalty_factor = get_penalty_factor(s->lambda, s->lambda2, c->avctx->me_cmp); > - c->sub_penalty_factor= get_penalty_factor(s->lambda, s->lambda2, c->avctx->me_sub_cmp); > - c->mb_penalty_factor = get_penalty_factor(s->lambda, s->lambda2, c->avctx->mb_cmp); > c->current_mv_penalty= mv_penalty; > > get_limits(s, 16*mb_x, 16*mb_y); > @@ -1491,7 +1488,6 @@ void ff_estimate_b_frame_motion(MpegEncContext * s, > int mb_x, int mb_y) > { > MotionEstContext * const c= &s->me; > - const int penalty_factor= c->mb_penalty_factor; > int fmin, bmin, dmin, fbmin, bimin, fimin; > int type=0; > const int xy = mb_y*s->mb_stride + mb_x; > @@ -1517,18 +1513,23 @@ void ff_estimate_b_frame_motion(MpegEncContext * s, > dmin= direct_search(s, mb_x, mb_y); > else > dmin= INT_MAX; > + > + c->penalty_factor = get_penalty_factor(s->lambda, s->lambda2, c->avctx->me_cmp); > + c->sub_penalty_factor= get_penalty_factor(s->lambda, s->lambda2, c->avctx->me_sub_cmp); > + c->mb_penalty_factor = get_penalty_factor(s->lambda, s->lambda2, c->avctx->mb_cmp); If mb_penalty_factor isnt correct in this before this maybe isnt enough as the direct_search() uses mb_penalty_factor thx [...]
On Mon, Mar 23, 2020 at 10:42 PM Michael Niedermayer <michael@niedermayer.cc> wrote: > On Sun, Mar 22, 2020 at 04:55:25PM +0100, Ramiro Polla wrote: > > In ff_estimate_b_frame_motion(), penalty_factor would be used before > > being initialized in estimate_motion_b(). Also, the initialization > > would happen more than once unnecessarily. > > --- > > libavcodec/motion_est.c | 15 ++++++++------- > > tests/ref/vsynth/vsynth2-mpeg2-422 | 6 +++--- > > tests/ref/vsynth/vsynth2-mpeg2-ivlc-qprd | 6 +++--- > > tests/ref/vsynth/vsynth2-mpeg4-adap | 6 +++--- > > 4 files changed, 17 insertions(+), 16 deletions(-) > > > > diff --git a/libavcodec/motion_est.c b/libavcodec/motion_est.c > > index 02c75fd470..1feb46cec3 100644 > > --- a/libavcodec/motion_est.c > > +++ b/libavcodec/motion_est.c > > @@ -1123,9 +1123,6 @@ static int estimate_motion_b(MpegEncContext *s, int mb_x, int mb_y, > > uint8_t * const mv_penalty= c->mv_penalty[f_code] + MAX_DMV; > > int mv_scale; > > > > - c->penalty_factor = get_penalty_factor(s->lambda, s->lambda2, c->avctx->me_cmp); > > - c->sub_penalty_factor= get_penalty_factor(s->lambda, s->lambda2, c->avctx->me_sub_cmp); > > - c->mb_penalty_factor = get_penalty_factor(s->lambda, s->lambda2, c->avctx->mb_cmp); > > c->current_mv_penalty= mv_penalty; > > > > get_limits(s, 16*mb_x, 16*mb_y); > > > > > @@ -1491,7 +1488,6 @@ void ff_estimate_b_frame_motion(MpegEncContext * s, > > int mb_x, int mb_y) > > { > > MotionEstContext * const c= &s->me; > > - const int penalty_factor= c->mb_penalty_factor; > > int fmin, bmin, dmin, fbmin, bimin, fimin; > > int type=0; > > const int xy = mb_y*s->mb_stride + mb_x; > > @@ -1517,18 +1513,23 @@ void ff_estimate_b_frame_motion(MpegEncContext * s, > > dmin= direct_search(s, mb_x, mb_y); > > else > > dmin= INT_MAX; > > + > > + c->penalty_factor = get_penalty_factor(s->lambda, s->lambda2, c->avctx->me_cmp); > > + c->sub_penalty_factor= get_penalty_factor(s->lambda, s->lambda2, c->avctx->me_sub_cmp); > > + c->mb_penalty_factor = get_penalty_factor(s->lambda, s->lambda2, c->avctx->mb_cmp); > > If mb_penalty_factor isnt correct in this before this maybe isnt enough > as the direct_search() uses mb_penalty_factor Fixed. New patch attached.
diff --git a/libavcodec/motion_est.c b/libavcodec/motion_est.c index 02c75fd470..1feb46cec3 100644 --- a/libavcodec/motion_est.c +++ b/libavcodec/motion_est.c @@ -1123,9 +1123,6 @@ static int estimate_motion_b(MpegEncContext *s, int mb_x, int mb_y, uint8_t * const mv_penalty= c->mv_penalty[f_code] + MAX_DMV; int mv_scale; - c->penalty_factor = get_penalty_factor(s->lambda, s->lambda2, c->avctx->me_cmp); - c->sub_penalty_factor= get_penalty_factor(s->lambda, s->lambda2, c->avctx->me_sub_cmp); - c->mb_penalty_factor = get_penalty_factor(s->lambda, s->lambda2, c->avctx->mb_cmp); c->current_mv_penalty= mv_penalty; get_limits(s, 16*mb_x, 16*mb_y); @@ -1491,7 +1488,6 @@ void ff_estimate_b_frame_motion(MpegEncContext * s, int mb_x, int mb_y) { MotionEstContext * const c= &s->me; - const int penalty_factor= c->mb_penalty_factor; int fmin, bmin, dmin, fbmin, bimin, fimin; int type=0; const int xy = mb_y*s->mb_stride + mb_x; @@ -1517,18 +1513,23 @@ void ff_estimate_b_frame_motion(MpegEncContext * s, dmin= direct_search(s, mb_x, mb_y); else dmin= INT_MAX; + + c->penalty_factor = get_penalty_factor(s->lambda, s->lambda2, c->avctx->me_cmp); + c->sub_penalty_factor= get_penalty_factor(s->lambda, s->lambda2, c->avctx->me_sub_cmp); + c->mb_penalty_factor = get_penalty_factor(s->lambda, s->lambda2, c->avctx->mb_cmp); + // FIXME penalty stuff for non-MPEG-4 c->skip=0; fmin = estimate_motion_b(s, mb_x, mb_y, s->b_forw_mv_table, 0, s->f_code) + - 3 * penalty_factor; + 3 * c->mb_penalty_factor; c->skip=0; bmin = estimate_motion_b(s, mb_x, mb_y, s->b_back_mv_table, 2, s->b_code) + - 2 * penalty_factor; + 2 * c->mb_penalty_factor; ff_dlog(s, " %d %d ", s->b_forw_mv_table[xy][0], s->b_forw_mv_table[xy][1]); c->skip=0; - fbmin= bidir_refine(s, mb_x, mb_y) + penalty_factor; + fbmin= bidir_refine(s, mb_x, mb_y) + c->mb_penalty_factor; ff_dlog(s, "%d %d %d %d\n", dmin, fmin, bmin, fbmin); if (s->avctx->flags & AV_CODEC_FLAG_INTERLACED_ME) { diff --git a/tests/ref/vsynth/vsynth2-mpeg2-422 b/tests/ref/vsynth/vsynth2-mpeg2-422 index ec7244f9f9..e945a4cc0e 100644 --- a/tests/ref/vsynth/vsynth2-mpeg2-422 +++ b/tests/ref/vsynth/vsynth2-mpeg2-422 @@ -1,4 +1,4 @@ -b2fa9b73c3547191ecc01b8163abd4e5 *tests/data/fate/vsynth2-mpeg2-422.mpeg2video -379164 tests/data/fate/vsynth2-mpeg2-422.mpeg2video -704f6a96f93c2409219bd48b74169041 *tests/data/fate/vsynth2-mpeg2-422.out.rawvideo +6fc8dc1d76379e459051ca393101c090 *tests/data/fate/vsynth2-mpeg2-422.mpeg2video +379173 tests/data/fate/vsynth2-mpeg2-422.mpeg2video +9199d5aaa1709d2584e21e58d76d44fb *tests/data/fate/vsynth2-mpeg2-422.out.rawvideo stddev: 4.17 PSNR: 35.73 MAXDIFF: 70 bytes: 7603200/ 7603200 diff --git a/tests/ref/vsynth/vsynth2-mpeg2-ivlc-qprd b/tests/ref/vsynth/vsynth2-mpeg2-ivlc-qprd index 16de39edfc..f5bbecfcb2 100644 --- a/tests/ref/vsynth/vsynth2-mpeg2-ivlc-qprd +++ b/tests/ref/vsynth/vsynth2-mpeg2-ivlc-qprd @@ -1,4 +1,4 @@ -907a30295ed8323780eee08e606af0ab *tests/data/fate/vsynth2-mpeg2-ivlc-qprd.mpeg2video -269722 tests/data/fate/vsynth2-mpeg2-ivlc-qprd.mpeg2video -d2d9793bf8f3427b5cc17a1be78ddd64 *tests/data/fate/vsynth2-mpeg2-ivlc-qprd.out.rawvideo +f612ea89aa79a7f7b93a8acf332705c4 *tests/data/fate/vsynth2-mpeg2-ivlc-qprd.mpeg2video +269723 tests/data/fate/vsynth2-mpeg2-ivlc-qprd.mpeg2video +88e17886e6383755829d7da519fd5e79 *tests/data/fate/vsynth2-mpeg2-ivlc-qprd.out.rawvideo stddev: 5.54 PSNR: 33.25 MAXDIFF: 94 bytes: 7603200/ 7603200 diff --git a/tests/ref/vsynth/vsynth2-mpeg4-adap b/tests/ref/vsynth/vsynth2-mpeg4-adap index a3223f6363..1ae0a65e4f 100644 --- a/tests/ref/vsynth/vsynth2-mpeg4-adap +++ b/tests/ref/vsynth/vsynth2-mpeg4-adap @@ -1,4 +1,4 @@ -4bff98da2342836476da817428594403 *tests/data/fate/vsynth2-mpeg4-adap.avi -213508 tests/data/fate/vsynth2-mpeg4-adap.avi -0c709f2b81f4593eaa29490332c2cb39 *tests/data/fate/vsynth2-mpeg4-adap.out.rawvideo +fcb79c0dcc00b306b79c354e589b6b69 *tests/data/fate/vsynth2-mpeg4-adap.avi +213526 tests/data/fate/vsynth2-mpeg4-adap.avi +71a34a48a81485f938d2c60a3d34ed39 *tests/data/fate/vsynth2-mpeg4-adap.out.rawvideo stddev: 4.87 PSNR: 34.36 MAXDIFF: 86 bytes: 7603200/ 7603200