[vlc-devel] [PATCH 2/3] deinterlace: x86: add external asm emms function

[Janne Grunau]

On 2016-10-27 11:36:30 +0300, Rémi Denis-Courmont wrote:
> Le torstaina 20. lokakuuta 2016, 0.53.16 EEST Janne Grunau a écrit :
> > ---
> >  modules/video_filter/Makefile.am                   |  4 +++
> >  modules/video_filter/deinterlace/common.h          |  4 +++
> >  .../video_filter/deinterlace/deinterlace_x86.asm   | 34
> > ++++++++++++++++++++++ 3 files changed, 42 insertions(+)
> >  create mode 100644 modules/video_filter/deinterlace/deinterlace_x86.asm
> > 
> > diff --git a/modules/video_filter/Makefile.am
> > b/modules/video_filter/Makefile.am index 5d5fdaf..92c5bc3 100644
> > --- a/modules/video_filter/Makefile.am
> > +++ b/modules/video_filter/Makefile.am
> > @@ -120,6 +120,10 @@ libdeinterlace_plugin_la_SOURCES = \
> >  	video_filter/deinterlace/algo_ivtc.c video_filter/deinterlace/algo_ivtc.h
> >  # inline ASM doesn't build with -O0
> >  libdeinterlace_plugin_la_CFLAGS = $(AM_CFLAGS) -O2
> > +if HAVE_YASM
> > +libdeinterlace_plugin_la_CFLAGS += -DHAVE_YASM
> > +libdeinterlace_plugin_la_SOURCES +=
> > video_filter/deinterlace/deinterlace_x86.asm +endif
> 
> I dislike adding a source language conditionally to a target, because GNU/
> automake generally just sucks at dealing with such.

what is the alternative? adding noop default compiler for .asm files?

> >  if HAVE_NEON
> >  libdeinterlace_plugin_la_SOURCES += video_filter/deinterlace/merge_arm.S
> >  libdeinterlace_plugin_la_CFLAGS += -DCAN_COMPILE_ARM
> > diff --git a/modules/video_filter/deinterlace/common.h
> > b/modules/video_filter/deinterlace/common.h index dcd7e63..afce82b 100644
> > --- a/modules/video_filter/deinterlace/common.h
> > +++ b/modules/video_filter/deinterlace/common.h
> > @@ -35,4 +35,8 @@
> >  #define FFMIN(a,b)      __MIN(a,b)
> >  #define FFMIN3(a,b,c)   FFMIN(FFMIN(a,b),c)
> > 
> > +#if HAVE_YASM
> > +void vlcpriv_emms_ext_asm(void);
> > +#endif
> > +
> >  #endif
> > diff --git a/modules/video_filter/deinterlace/deinterlace_x86.asm
> > b/modules/video_filter/deinterlace/deinterlace_x86.asm new file mode 100644
> > index 0000000..60ce829
> > --- /dev/null
> > +++ b/modules/video_filter/deinterlace/deinterlace_x86.asm
> > @@ -0,0 +1,34 @@
> > +;**************************************************************************
> > *** +;* x86-optimized functions for deinterlace filter
> > +;**************************************************************************
> > *** +;* Cpyright (C) 2016- VLC authors and VideoLAN
> > +;* $Id: 1e2cf65f9467664726cfed16aa792342339946b1 $
> > +;*
> > +;* Authors: Janne Grunau <janne-vlc at jannau.net>
> > +;*
> > +;* This file is part of vlc.
> > +;*
> > +;* This program is free software; you can redistribute it and/or modify it
> > +;* under the terms of the GNU Lesser General Public License as published by
> > +;* the Free Software Foundation; either version 2.1 of the License, or +;*
> > (at your option) any later version.
> > +;*
> > +;* This program is distributed in the hope that it will be useful,
> > +;* but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +;* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > +;* GNU Lesser General Public License for more details.
> > +;*
> > +;* You should have received a copy of the GNU Lesser General Public License
> > +;* along with this program; if not, write to the Free Software Foundation,
> > +;* Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA.
> > +;*************************************************************************
> > ***** +
> > +%include "x86util.asm"
> > +
> > +SECTION_RODATA
> > +
> > +SECTION .text
> > +
> > +cglobal emms_ext_asm, 0,0,0
> > +        emms
> > +        RET
> 
> I must first say that I am in no way an expert in x86 ISA and ABI.
> 
> But I don´t really see what this function does. It has no input and no 
> output.

x86 is strange: fpu and mmx registers are shared and there is state this has to 
be reset to use fpu registers after mmx registers were used. The emms 
instruction (empty mmx state) does this.

Janne