[Ffmpeg-devel] libswscale coding style

[Marc Hoffman]

Diego Biurrun writes:
 > I've just implemented pre-commit hooks for libswscale that check for
 > tabs and trailing whitespace, same as for FFmpeg.
 > 
 > I'm in the process of removing trailing whitespace and tabs and
 > reformatting the files while I'm at it.
 > 
 > What's the preferred style for inline ASM, things like
 > rgb2rgb_template.c:100?
 > 
 > Please hold off on committing to libswscale for now, otherwise I'll have
 > lots of conflicts to clear up.
 > 
 > Diego


I would like to see the reference codes and the MMX stuff completely
separated out.  Make a C reference model and then allow that behavior
to be overloaded with function pointers.  At the frame or band
resolution it doesn't matter if its inlined or a set of 10 function
calls to get there.

rgb2rgb.c has a series of functions like this:

static void rgb24to32_C (const uint8_t *src,uint8_t *dst,long src_size)
{
  uint8_t *dest = dst;
  const uint8_t *s = src;
  const uint8_t *end;
  end = s + src_size;

  while(s < end)
  {
#ifdef WORDS_BIGENDIAN
    /* RGB24 (= R,G,B) -> RGB32 (= A,B,G,R) */
    *dest++ = 0;
    *dest++ = s[2];
    *dest++ = s[1];
    *dest++ = s[0];
    s+=3;
#else
    *dest++ = *s++;
    *dest++ = *s++;
    *dest++ = *s++;
    *dest++ = 0;
#endif
  }
}


...

init_rgbfuncs (SWSOPS *ops) {

   ops->rgb24to32 = rgb24to32_C;

   ...
...
}


not sure what to do with the inline stuff it doesn't really help all that much.

then have an rgb2rgb_mmx.c that has:

static rgb24to32 (const uint8_t *src,uint8_t *dst,long src_size)
{
  uint8_t *dest = dst;
  const uint8_t *s = src;
  const uint8_t *end;
  const uint8_t *mm_end;

  end = s + src_size;

  __asm __volatile(PREFETCH"	%0"::"m"(*s):"memory");
  mm_end = end - 23;
  __asm __volatile("movq	%0, %%mm7"::"m"(mask32):"memory");
  while(s < mm_end)
  {
    __asm __volatile(
	PREFETCH"	32%1\n\t"
	"movd	%1, %%mm0\n\t"
	"punpckldq 3%1, %%mm0\n\t"
	"movd	6%1, %%mm1\n\t"
	"punpckldq 9%1, %%mm1\n\t"
	"movd	12%1, %%mm2\n\t"
	"punpckldq 15%1, %%mm2\n\t"
	"movd	18%1, %%mm3\n\t"
	"punpckldq 21%1, %%mm3\n\t"
	"pand	%%mm7, %%mm0\n\t"
	"pand	%%mm7, %%mm1\n\t"
	"pand	%%mm7, %%mm2\n\t"
	"pand	%%mm7, %%mm3\n\t"
	MOVNTQ"	%%mm0, %0\n\t"
	MOVNTQ"	%%mm1, 8%0\n\t"
	MOVNTQ"	%%mm2, 16%0\n\t"
	MOVNTQ"	%%mm3, 24%0"
	:"=m"(*dest)
	:"m"(*s)
	:"memory");
    dest += 32;
    s += 24;
  }
  __asm __volatile(SFENCE:::"memory");
  __asm __volatile(EMMS:::"memory");

  while(s < end)
  {
#ifdef WORDS_BIGENDIAN
    /* RGB24 (= R,G,B) -> RGB32 (= A,B,G,R) */
    *dest++ = 0;
    *dest++ = s[2];
    *dest++ = s[1];
    *dest++ = s[0];
    s+=3;
#else
    *dest++ = *s++;
    *dest++ = *s++;
    *dest++ = *s++;
    *dest++ = 0;
#endif
  }
}


ff_mmx_init_rgbfuncs (SWSOPS *ops) {

   ops->rgb24to32 = rgb24to32_MMX;

   ...
   ...
}



and then an 

rgb2rgb_altivec.c


and then an 

rgb2rgb_bfin.c




Then wrap the whole thing up in a single init who calls the
init_rgbfuncs then the architecture specific one.