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

[Michael Niedermayer]

On Mon, Oct 24, 2016 at 02:36:23PM +0200, wm4 wrote:
> On Mon, 24 Oct 2016 13:24:38 +0200
> 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.
> > 
> > [...]
> 
> That fixes only the musl-specific issue. It could happen any time again
> with a set of different callers.

yes


> Unless you want to put special effort
> into fixing operation with musl (and keeping it working) it won't help
> much to fix the correctness issues.

i did want to do that, but ive a headache atm and the discussion
about emms yesterday makes me not want to touch emms anymore


> 
> If anything, it should probably say "all external" calls or such.

I see this as the long term goal, yes i agree it should be
be documented if theres consensus about it

The way i imagined it was to fix the practically relevant issues and
anything else thats easy or attached (like x86-64 for what is the same
on x86-32) before 3.2 and have 3.2 work with musl on all archs
then aim at fixing the rest over the following months unless some
roadblocks like performance issues are hit.

This incremental approuch has been hit with quite some opposition so
ill leave this to whoever likes to work on this

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Old school: Use the lowest level language in which you can solve the problem
            conveniently.
New school: Use the highest level language in which the latest supercomputer
            can solve the problem without the user falling asleep waiting.
-------------- 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/20161024/f04f8271/attachment.sig>