[FFmpeg-devel] [PATCH 09/13] avcodec/svq1dec: clear MMX state after MB decode loop

[Michael Niedermayer]

On Sat, Oct 22, 2016 at 10:10:01PM -0400, Ronald S. Bultje wrote:
> Hi,
> 
> general comment about all other dec patches.
> 
> On Sat, Oct 22, 2016 at 3:02 PM, Michael Niedermayer <michael at niedermayer.cc
> > wrote:
> 
> > Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> > ---
> >  libavcodec/svq1dec.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/libavcodec/svq1dec.c b/libavcodec/svq1dec.c
> > index 2b72e08..0fe222e 100644
> > --- a/libavcodec/svq1dec.c
> > +++ b/libavcodec/svq1dec.c
> > @@ -744,6 +744,7 @@ static int svq1_decode_frame(AVCodecContext *avctx,
> > void *data,
> >              }
> >          }
> >      }
> > +    emms_c();
> 
> 
> This is hideous, you're sprinkling emms_c in various places to make some
> stupid test pass. The test is morbidly stupid and there is no general
> consensus on patterns to be followed as for where to place emms_c. Someone
> who doesn't know any better will litter each new decoder with 10-20 calls
> to emms_c just because he found that other decoders do it in undocumented,
> unexplained and unclear locations also.
> 
> If you want this to be a "thing", you need to design and document carefully
> where emms_c is necessary. Then come up with some system that makes this
> work by itself.

Your mail sounds quite aggressive to me, did i say something to anger
you ? It was certainly not intended

About this patchset
FFmpeg is broken ATM (both in theory and practice with some libcs),
the way MMX code is used is not correct, emms_c()
calls are missing and required. The obvious way to fix that
(in practice) is adding emms_c() calls where they are missing.
I can document why each call is needed, if thats wanted?

I dont think hiding the calls by some trick would make it 
better. The calls must be carefully placed and knowing what the calls
do makes that easier to maintain than a frame_end() function which
would have to be ordered so it comes before any alloc/free/ref/unref/
... as well but for which thie requirement is less obvious
in fact even the emms in the thread progress is probably not that great
of an idea because of this.

[...]
-- 
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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20161023/0841fd03/attachment.sig>