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

Michael Niedermayer michael at niedermayer.cc
Sun Oct 23 06:36:22 EEST 2016

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>

More information about the ffmpeg-devel mailing list