[FFmpeg-devel] PATCH BlackFin YUV2 to RGB Color Space Converters

[Marc Hoffman]

Hi,

Luca Barbato writes:
 > Marc Hoffman wrote:
 > > Blackfin optimized YUV420 to RGB CSC Color Space Converters.
 > 
 > > Please review thank you.
 > 
 > > +#define clock()      ({ int _t; asm volatile ("%0=cycles;" : "=d" (_t)); _t; })
 > 
 > why _t?

I used it in the code to verify I got the same values as what you get
with utime.  It's not even used good catch I will remove it.

 > 
 > why not adding START/STOP_TIMER bfin?
 > 
 > > +
 > > +#define L1CODE __attribute__ ((l1_text))
 > 
 > shouldn't that be put somewhere else since looks like you will use it in
 > other code?

Not sure where this would go. Can I share an include file between
dsputils and swscale?

 > 
 > > +
 > > +extern void
 > > +ff_bfin_yuv2rgb555_line (uint16_t *Y, uint16_t *U, uint16_t *V, uint8_t *out, int w, uint32_t *coeffs)
 > > +     L1CODE;
 > 
 > Ugly to read, please 79 cols and

thats fine that you don't like it. I will change it.

 > 
 > > +
 > > +     extern void
 > > +     ff_bfin_yuv2rgb565_line (uint16_t *Y, uint16_t *U, uint16_t *V, uint8_t *out, int w, uint32_t *coeffs)
 > > +     L1CODE;
 > 
 > Why indentation for protos?

And thats just a style
 > 
 > > +
 > > +     extern void
 > > +     ff_bfin_yuv2rgb24_line (uint16_t *Y, uint16_t *U, uint16_t *V, uint8_t *out, int w, uint32_t *coeffs)
 > > +     L1CODE;
 > > +
 > > +     typedef void (* ltransform_t)(uint16_t *Y, uint16_t *U, uint16_t *V, uint8_t *out, int w, uint32_t *coeffs);
 > > +
 > > +
 > 
 > > +
 > > +
 > > +SwsFunc ff_bfin_yuv2rgb_get_func_ptr (SwsContext *c)
 > > +{
 > > +    SwsFunc f = 0;
 > 
 > NULL?

Sure, actually it doesn't need to be initialized I will remove the = 0.

 > 
 > > +    New factorization to elliminate the truncation error which was
 > typo                         ^
 > 
 > 
 > That is a quick review...

Thanks for your input.

Marc