Re: [FFmpeg-devel] [PATCH] avcodec/decode: Reset MMX state for receive_frame decoders, too

[Anton Khirnov <anton@khirnov.net>]

Quoting Andreas Rheinhardt (2023-03-14 03:53:40)
> FFmpeg's assembly code currently does not abide by the
> plattform-specific ABIs wrt its handling of the X86 MMX flag:
> Resetting the MMX state is deferred to avoid doing it multiple times
> instead of ensuring that the CPU is in floating point state
> upon return from any function.
> 
> Furthermore, resetting said state is sometimes done generically,
> namely for all the decoders using the ordinary decode callback;
> yet this is not done for the decoders using the receive_frame API.
> 
> This led to problems when MJPEG (and the MJPEG-based decoders)
> were switched to the receive_frame API in commit
> e9a2a8777317d91af658f774c68442ac4aa726ec, because ff_mjpeg_decode_sos()
> only resets the MMX state on success, not on failure.
> Such issues are probably still possible with SMVJPEG, which still
> uses the receive_frame API. See issue #10210.
> 
> This commit therefore also resets the MMX state for
> the receive_frame API to avoid any more surprises of this sort.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
> Of course, I am in favour of actually abiding by the ABI,
> even if it would cost performance; notice that there are
> currently scenarios where emms_c() is called unnecessarily
> (it is called generically and therefore even for codecs
> that don't need it; furthermore, for some codecs it depends upon
> the CPU flags used whether MMX code is in use, so it is unnecessary
> for newer CPUs).

Ideally we'd kill off all MMX code and forget about such issues, but
that's presumably way easier said than done.

> 
>  libavcodec/decode.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> index d1ba7f167f..22a0f8eb25 100644
> --- a/libavcodec/decode.c
> +++ b/libavcodec/decode.c
> @@ -547,6 +547,7 @@ static int decode_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame)
>  
>      if (codec->cb_type == FF_CODEC_CB_TYPE_RECEIVE_FRAME) {
>          ret = codec->cb.receive_frame(avctx, frame);
> +        emms_c();

Should be ok.

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