[FFmpeg-devel] [PATCH v2 1/2] libavcodec/mpegvideo_enc: fix multi-threaded motion estimation rounding for mpeg4

[FFmpeg-devel] [PATCH v2 1/2] libavcodec/mpegvideo_enc: fix multi-threaded motion estimation rounding for mpeg4

[Ramiro Polla]

ff_init_me() was being called after ff_update_duplicate_context(),
which caused the propagation of the initialization to other thread
contexts to be delayed by one frame.

In the case of mpeg4 (or flipflop_rounding), this would make the
hpel_put functions differ between the first thread (which would be
correctly initialized) and the other threads (which would be stale
from the previous frame).
---
 libavcodec/mpegvideo_enc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
index 2a75973ac4..b601a1a9e4 100644
--- a/libavcodec/mpegvideo_enc.c
+++ b/libavcodec/mpegvideo_enc.c
@@ -3623,6 +3623,9 @@ static int encode_picture(MpegEncContext *s)
         s->q_chroma_intra_matrix16 = s->q_intra_matrix16;
     }
 
+    if(ff_init_me(s)<0)
+        return -1;
+
     s->mb_intra=0; //for the rate distortion & bit compare functions
     for(i=1; i<context_count; i++){
         ret = ff_update_duplicate_context(s->thread_context[i], s);
@@ -3630,9 +3633,6 @@ static int encode_picture(MpegEncContext *s)
             return ret;
     }
 
-    if(ff_init_me(s)<0)
-        return -1;
-
     /* Estimate motion for every MB */
     if(s->pict_type != AV_PICTURE_TYPE_I){
         s->lambda  = (s->lambda  * s->me_penalty_compensation + 128) >> 8;
-- 
2.30.2

_______________________________________________
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".

[FFmpeg-devel] [PATCH v2 2/2] libavcodec/motion_est: fix penalty_factor for b frames

[Ramiro Polla]

In direct_search() and 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/vsynth1-mpeg4-thread    |  6 +++---
 tests/ref/vsynth/vsynth2-mpeg2-ivlc-qprd |  6 +++---
 tests/ref/vsynth/vsynth2-mpeg4-adap      |  8 ++++----
 tests/ref/vsynth/vsynth2-mpeg4-qprd      |  6 +++---
 tests/ref/vsynth/vsynth2-mpeg4-thread    |  6 +++---
 tests/ref/vsynth/vsynth_lena-mpeg4-rc    |  4 ++--
 7 files changed, 26 insertions(+), 25 deletions(-)

diff --git a/libavcodec/motion_est.c b/libavcodec/motion_est.c
index df9d1befa8..fb569ede8a 100644
--- a/libavcodec/motion_est.c
+++ b/libavcodec/motion_est.c
@@ -1127,9 +1127,6 @@ static int estimate_motion_b(MpegEncContext *s, int mb_x, int mb_y,
     const 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);
@@ -1495,7 +1492,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,22 +1513,27 @@ void ff_estimate_b_frame_motion(MpegEncContext * s,
         return;
     }
 
+    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 (s->codec_id == AV_CODEC_ID_MPEG4)
         dmin= direct_search(s, mb_x, mb_y);
     else
         dmin= INT_MAX;
+
 // 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/vsynth1-mpeg4-thread b/tests/ref/vsynth/vsynth1-mpeg4-thread
index 6b69fb4c12..6b110c49fb 100644
--- a/tests/ref/vsynth/vsynth1-mpeg4-thread
+++ b/tests/ref/vsynth/vsynth1-mpeg4-thread
@@ -1,4 +1,4 @@
-369ace2f9613261af869efd9fbb3c149 *tests/data/fate/vsynth1-mpeg4-thread.avi
-774754 tests/data/fate/vsynth1-mpeg4-thread.avi
-9aa327a244d5179acf7fe64dc1459bff *tests/data/fate/vsynth1-mpeg4-thread.out.rawvideo
+7761391e354266976a9e0155eff983dd *tests/data/fate/vsynth1-mpeg4-thread.avi
+774752 tests/data/fate/vsynth1-mpeg4-thread.avi
+bbdbe9af4f5b106b847595bf3040699f *tests/data/fate/vsynth1-mpeg4-thread.out.rawvideo
 stddev:   10.13 PSNR: 28.02 MAXDIFF:  183 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 35b2b6aac9..e058cd1ce3 100644
--- a/tests/ref/vsynth/vsynth2-mpeg4-adap
+++ b/tests/ref/vsynth/vsynth2-mpeg4-adap
@@ -1,4 +1,4 @@
-06a397fe43dab7b6cf56870410fbbbaf *tests/data/fate/vsynth2-mpeg4-adap.avi
-203000 tests/data/fate/vsynth2-mpeg4-adap.avi
-686565d42d8ba5aea790824b04fa0a18 *tests/data/fate/vsynth2-mpeg4-adap.out.rawvideo
-stddev:    4.55 PSNR: 34.95 MAXDIFF:   84 bytes:  7603200/  7603200
+9465ef120d560537d8fcfb5564782e01 *tests/data/fate/vsynth2-mpeg4-adap.avi
+203004 tests/data/fate/vsynth2-mpeg4-adap.avi
+d7851ab1ca9744f8e618a24193e5ef76 *tests/data/fate/vsynth2-mpeg4-adap.out.rawvideo
+stddev:    4.56 PSNR: 34.95 MAXDIFF:   84 bytes:  7603200/  7603200
diff --git a/tests/ref/vsynth/vsynth2-mpeg4-qprd b/tests/ref/vsynth/vsynth2-mpeg4-qprd
index 0a8786b89a..3face947c2 100644
--- a/tests/ref/vsynth/vsynth2-mpeg4-qprd
+++ b/tests/ref/vsynth/vsynth2-mpeg4-qprd
@@ -1,4 +1,4 @@
-4ddd2fef35854d9b387bbcbda03dc7f0 *tests/data/fate/vsynth2-mpeg4-qprd.avi
-248706 tests/data/fate/vsynth2-mpeg4-qprd.avi
-baa8d0d57a7fb5e393642cb20efed2c2 *tests/data/fate/vsynth2-mpeg4-qprd.out.rawvideo
+33fc3d5507cc8d2c8b63b8f811e62e4c *tests/data/fate/vsynth2-mpeg4-qprd.avi
+248734 tests/data/fate/vsynth2-mpeg4-qprd.avi
+61f8006e8903915056493fb1f05d1b2f *tests/data/fate/vsynth2-mpeg4-qprd.out.rawvideo
 stddev:    4.85 PSNR: 34.40 MAXDIFF:   85 bytes:  7603200/  7603200
diff --git a/tests/ref/vsynth/vsynth2-mpeg4-thread b/tests/ref/vsynth/vsynth2-mpeg4-thread
index 49c0ce0241..a1791c49ec 100644
--- a/tests/ref/vsynth/vsynth2-mpeg4-thread
+++ b/tests/ref/vsynth/vsynth2-mpeg4-thread
@@ -1,4 +1,4 @@
-92128f8adc4ac70a66fdddf58e46b923 *tests/data/fate/vsynth2-mpeg4-thread.avi
-268396 tests/data/fate/vsynth2-mpeg4-thread.avi
-f432bd8d897c7c8e286e385b77cedcfa *tests/data/fate/vsynth2-mpeg4-thread.out.rawvideo
+44df605055498a01afb53eaaabdb94b4 *tests/data/fate/vsynth2-mpeg4-thread.avi
+268394 tests/data/fate/vsynth2-mpeg4-thread.avi
+13240eaccc345bf4b45f24d44cfc5ca2 *tests/data/fate/vsynth2-mpeg4-thread.out.rawvideo
 stddev:    4.89 PSNR: 34.34 MAXDIFF:   86 bytes:  7603200/  7603200
diff --git a/tests/ref/vsynth/vsynth_lena-mpeg4-rc b/tests/ref/vsynth/vsynth_lena-mpeg4-rc
index fd6a998046..a21ddc87e3 100644
--- a/tests/ref/vsynth/vsynth_lena-mpeg4-rc
+++ b/tests/ref/vsynth/vsynth_lena-mpeg4-rc
@@ -1,4 +1,4 @@
-396a76466dee56e2714dfa42cebe3d2d *tests/data/fate/vsynth_lena-mpeg4-rc.avi
+3ae5a2590bdd0e80a95bf374b06c553f *tests/data/fate/vsynth_lena-mpeg4-rc.avi
 226314 tests/data/fate/vsynth_lena-mpeg4-rc.avi
-6e8b62e8c3bcbfdcc58afb69a0b1c4e3 *tests/data/fate/vsynth_lena-mpeg4-rc.out.rawvideo
+27c8771df4154f2be317465a3d3cbd56 *tests/data/fate/vsynth_lena-mpeg4-rc.out.rawvideo
 stddev:    4.23 PSNR: 35.60 MAXDIFF:   85 bytes:  7603200/  7603200
-- 
2.30.2

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v2 2/2] libavcodec/motion_est: fix penalty_factor for b frames

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 975 bytes --]

On Wed, May 08, 2024 at 05:19:50PM +0200, Ramiro Polla wrote:
> In direct_search() and 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/vsynth1-mpeg4-thread    |  6 +++---
>  tests/ref/vsynth/vsynth2-mpeg2-ivlc-qprd |  6 +++---
>  tests/ref/vsynth/vsynth2-mpeg4-adap      |  8 ++++----
>  tests/ref/vsynth/vsynth2-mpeg4-qprd      |  6 +++---
>  tests/ref/vsynth/vsynth2-mpeg4-thread    |  6 +++---
>  tests/ref/vsynth/vsynth_lena-mpeg4-rc    |  4 ++--
>  7 files changed, 26 insertions(+), 25 deletions(-)

probably ok

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Good people do not need laws to tell them to act responsibly, while bad
people will find a way around the laws. -- Plato

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v2 2/2] libavcodec/motion_est: fix penalty_factor for b frames

[Ramiro Polla]

On Wed, May 8, 2024 at 11:47 PM Michael Niedermayer
<michael@niedermayer.cc> wrote:
> On Wed, May 08, 2024 at 05:19:50PM +0200, Ramiro Polla wrote:
> > In direct_search() and 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/vsynth1-mpeg4-thread    |  6 +++---
> >  tests/ref/vsynth/vsynth2-mpeg2-ivlc-qprd |  6 +++---
> >  tests/ref/vsynth/vsynth2-mpeg4-adap      |  8 ++++----
> >  tests/ref/vsynth/vsynth2-mpeg4-qprd      |  6 +++---
> >  tests/ref/vsynth/vsynth2-mpeg4-thread    |  6 +++---
> >  tests/ref/vsynth/vsynth_lena-mpeg4-rc    |  4 ++--
> >  7 files changed, 26 insertions(+), 25 deletions(-)
>
> probably ok

Thanks. Pushed.
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v2 1/2] libavcodec/mpegvideo_enc: fix multi-threaded motion estimation rounding for mpeg4

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 926 bytes --]

On Wed, May 08, 2024 at 05:19:49PM +0200, Ramiro Polla wrote:
> ff_init_me() was being called after ff_update_duplicate_context(),
> which caused the propagation of the initialization to other thread
> contexts to be delayed by one frame.
> 
> In the case of mpeg4 (or flipflop_rounding), this would make the
> hpel_put functions differ between the first thread (which would be
> correctly initialized) and the other threads (which would be stale
> from the previous frame).
> ---
>  libavcodec/mpegvideo_enc.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

have you confirmed the actual used rounding matches after this
encoder & decoder side ?

if yes then this should be ok

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Elect your leaders based on what they did after the last election, not
based on what they say before an election.


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v2 1/2] libavcodec/mpegvideo_enc: fix multi-threaded motion estimation rounding for mpeg4

[Ramiro Polla]

On Thu, May 9, 2024 at 2:44 AM Michael Niedermayer
<michael@niedermayer.cc> wrote:
> On Wed, May 08, 2024 at 05:19:49PM +0200, Ramiro Polla wrote:
> > ff_init_me() was being called after ff_update_duplicate_context(),
> > which caused the propagation of the initialization to other thread
> > contexts to be delayed by one frame.
> >
> > In the case of mpeg4 (or flipflop_rounding), this would make the
> > hpel_put functions differ between the first thread (which would be
> > correctly initialized) and the other threads (which would be stale
> > from the previous frame).
> > ---
> >  libavcodec/mpegvideo_enc.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
>
> have you confirmed the actual used rounding matches after this
> encoder & decoder side ?

Yes, I just rechecked it. It used to be wrong (only the first slice
would use the correct hpel/qpel functions in the encoder according to
the no_rounding flag in the bitstream).

> if yes then this should be ok

Thanks. Pushed.
_______________________________________________
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".