[FFmpeg-devel] [PATCH] avutil/x86/emms: Document the emms_c() vs alloc/free relation.

[Ronald S. Bultje]

Hi,

On Mon, Oct 24, 2016 at 7:24 AM, Michael Niedermayer <michael at niedermayer.cc
> wrote:

> On Mon, Oct 24, 2016 at 09:38:00AM +0200, wm4 wrote:
> > On Sun, 23 Oct 2016 05:37:25 +0200
> > Michael Niedermayer <michael at niedermayer.cc> wrote:
> >
> > > Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> > > ---
> > >  libavutil/x86/emms.h | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/libavutil/x86/emms.h b/libavutil/x86/emms.h
> > > index 6fda6e2..42c18e2 100644
> > > --- a/libavutil/x86/emms.h
> > > +++ b/libavutil/x86/emms.h
> > > @@ -31,6 +31,8 @@ void avpriv_emms_yasm(void);
> > >   * Empty mmx state.
> > >   * this must be called between any dsp function and float/double code.
> > >   * for example sin(); dsp->idct_put(); emms_c(); cos()
> > > + * Note, *alloc() and *free() also use float code in some libc
> implementations
> > > + * thus this also applies to them or any function using them.
> > >   */
> > >  static av_always_inline void emms_c(void)
> > >  {
> >
> > Overly specific and useless information. It's an implementation detail
>
> If we place emms_c() so as to ensure that the FPU state is clean
> before all calls to *alloc() and *free() (as is done in the posted
> patchset) then we need to document this
> so others working on the code are aware of it and wont mistakly break
> it.
>
> If/when a different or more complete solution is implemented then this
> note needs to be adjusted accordingly.


I am doubtful that the patches that implement the partial solution
("hack"?) should be pushed, or that we should (by documenting it) advocate
partial solutions ("hacks") in general.

Ronald