> -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of > Michael Niedermayer > Sent: Wednesday, January 15, 2020 05:55 AM > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH V7 1/2] libswscale/x86/yuv2rgb: Change > inline assembly into nasm code > > On Fri, Jan 10, 2020 at 01:38:15AM +0800, Ting Fu wrote: > > Signed-off-by: Ting Fu <ting...@intel.com> > > --- > > V7: > > Fix compile issue when user configure with --disable-mmx. > > Fix issue when running ./ffmpeg with --cpuflags mmx/ssse3. > > Adjust the SIMD verify logic in libswscale/x86/yuv2rgb.c > > > > libswscale/x86/Makefile | 1 + > > libswscale/x86/swscale.c | 16 +- > > libswscale/x86/yuv2rgb.c | 66 ++--- > > libswscale/x86/yuv2rgb_template.c | 467 ++++++------------------------ > > libswscale/x86/yuv_2_rgb.asm | 270 +++++++++++++++++ > > 5 files changed, 405 insertions(+), 415 deletions(-) create mode > > 100644 libswscale/x86/yuv_2_rgb.asm > > The commit message seems a bit terse > I think it should say if the sequence of instructions is unchanged and if it > was > benchmaked. If its the same speed, when the code is run the commit message > should say that too > > the principle of this (inline -> nasm) is fine of course. > > [...] > > -static inline int RENAME(yuv420_rgb16)(SwsContext *c, const uint8_t *src[], > > - int srcStride[], > > - int srcSliceY, int srcSliceH, > > - uint8_t *dst[], int dstStride[]) > > +static int RENAME(yuv420_rgb16)(SwsContext *c, const uint8_t *src[], > > + int srcStride[], > > + int srcSliceY, int > > srcSliceH, > > + uint8_t *dst[], int > > +dstStride[]) > > maybe the removial of inline should be a seperate patch also there is the > question why these wraper functions exist These do change from a a "free thing > in inline asm" to a call overhead with C->NASM > Hi Michael,
The wrapper functions initiate some variables and contain one 'for cycle'. The variable initiation needs to access to the 'c->dstW', furthermore macro SWS_MAX_ FILTER_SIZE is needed. Which means extra work and much more NASM code. If you still prefer to do all the things in assembly, I can change from 'C->NASM' to 'call NASM function directly' in another further patch( for current patch easier to review). Or in my opinion, the cost in C->NASM can be ignored, and the initiation work looks clearer in C, just let it be what it is now. What do you think? Thank you, Ting Fu [...] _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".