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

[Ronald S. Bultje]

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. I've said from the beginning that I highly dislike
littering the code with emms_c in individual decoders, and that's exactly
what you're doing here. This is insane.

Ronald