> On Fri, Jul 01, 2022 at 01:34:34PM +0800, Wenbin Chen wrote:
> > For 422 frames we should not use hard coded 8 to calculate mb size for
> > uv plane. Chroma shift should be taken into consideration to be
> > compatiple with different sampling format.
> >
> > The error is reported by fate test when av_cpu_max_align() return 64
> > on the platform supporting AVX512. This is a hidden error and it is
> > exposed after commit 17a59a634c39b00a680c6ebbaea58db95594d13d.
> >
> > mpeg2enc has a mechanism to reuse frames. When it computes SSE (sum
> of
> > squared error) on current mb, reconstructed mb will be wrote to the
> > previous mb space, so that the memory can be saved. However if the align
> > is 64, the frame is shared in somewhere else, so the frame cannot be
> > reused and a new frame to store reconstrued data is created. Because the
> > height of mb is wrong when compute sse on 422 frame, starting from the
> > second line of macro block, changed data is read when frame is reused
> > (we need to read row 16 rather than row 8 if frame is 422), and unchanged
> > data is read when frame is not reused (a new frame is created so the
> > original frame will not be changed).
> >
> > That is why commit 17a59a634c39b00a680c6ebbaea58db95594d13d
> exposes this
> > issue, because it add av_cpu_max_align() and this function return 64 on
> > platform supporting AVX512 which lead to creating a frame in mpeg2enc,
> > and this lead to the different outputs.
> >
> > Signed-off-by: Wenbin Chen <wenbin.c...@intel.com>
> > ---
> > libavcodec/mpegvideo_enc.c | 29 +++++++++++++------
> > tests/ref/seek/vsynth_lena-mpeg2-422 | 40 +++++++++++++-------------
> > tests/ref/vsynth/vsynth1-mpeg2-422 | 8 +++---
> > tests/ref/vsynth/vsynth2-mpeg2-422 | 8 +++---
> > tests/ref/vsynth/vsynth3-mpeg2-422 | 8 +++---
> > tests/ref/vsynth/vsynth_lena-mpeg2-422 | 8 +++---
> > 6 files changed, 56 insertions(+), 45 deletions(-)
> >
> > diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
> > index d6a85a037a..c9d9e2a764 100644
> > --- a/libavcodec/mpegvideo_enc.c
> > +++ b/libavcodec/mpegvideo_enc.c
> > @@ -2558,24 +2558,35 @@ static int sse(MpegEncContext *s, uint8_t *src1,
> uint8_t *src2, int w, int h, in
> > static int sse_mb(MpegEncContext *s){
> > int w= 16;
> > int h= 16;
> > + int chroma_mb_w = w >> s->chroma_x_shift;
> > + int chroma_mb_h = h >> s->chroma_y_shift;
> >
> > if(s->mb_x*16 + 16 > s->width ) w= s->width - s->mb_x*16;
> > if(s->mb_y*16 + 16 > s->height) h= s->height- s->mb_y*16;
> >
> > if(w==16 && h==16)
> > if(s->avctx->mb_cmp == FF_CMP_NSSE){
>
> > - return s->mecc.nsse[0](s, s->new_picture->data[0] + s->mb_x * 16 +
> > s-
> >mb_y * s->linesize * 16, s->dest[0], s->linesize, 16) +
> > + return s->mecc.nsse[0](s, s->new_picture->data[0] + s->mb_x * 16 +
> s->mb_y * s->linesize * 16,
> > + s->dest[0], s->linesize, 16) +
>
> This doesnt belong in this patch, its not a functional change and makes it
> harder to see what is changed
>
> please seperate this in a seperate patch if you want to suggest this
> whitespace change
>
> thx
Ok, I will separate it into two patches and resubmit them.
Thanks
>
>
> > - s->mecc.nsse[1](s, s->new_picture->data[1] + s->mb_x * 8 +
> > s-
> >mb_y * s->uvlinesize * 8, s->dest[1], s->uvlinesize, 8) +
> > - s->mecc.nsse[1](s, s->new_picture->data[2] + s->mb_x * 8 +
> > s-
> >mb_y * s->uvlinesize * 8, s->dest[2], s->uvlinesize, 8);
> > + s->mecc.nsse[1](s, s->new_picture->data[1] + s->mb_x *
> chroma_mb_w + s->mb_y * s->uvlinesize * chroma_mb_h,
> > + s->dest[1], s->uvlinesize, chroma_mb_h) +
> > + s->mecc.nsse[1](s, s->new_picture->data[2] + s->mb_x *
> chroma_mb_w + s->mb_y * s->uvlinesize * chroma_mb_h,
> > + s->dest[2], s->uvlinesize, chroma_mb_h);
>
> [...]
> --
> Michael GnuPG fingerprint:
> 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> If you fake or manipulate statistics in a paper in physics you will never
> get a job again.
> If you fake or manipulate statistics in a paper in medicin you will get
> a job for life at the pharma industry.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
