[FFmpeg-devel] [PATCH] swscale: unscaled rgb48 <-> bgr48

[Paul B Mahol]

On 6/8/12, Michael Niedermayer <michaelni at gmx.at> wrote:
> On Fri, Jun 08, 2012 at 10:55:20AM +0000, Paul B Mahol wrote:
>> Signed-off-by: Paul B Mahol <onemda at gmail.com>
>> ---
>>  libswscale/rgb2rgb.c          |   21 ++++++++++++++++++++-
>>  libswscale/rgb2rgb.h          |    4 ++++
>>  libswscale/swscale_unscaled.c |   18 +++++++++++++++++-
>>  3 files changed, 41 insertions(+), 2 deletions(-)
>>
>> diff --git a/libswscale/rgb2rgb.c b/libswscale/rgb2rgb.c
>> index ba7d6de..6b3a01a 100644
>> --- a/libswscale/rgb2rgb.c
>> +++ b/libswscale/rgb2rgb.c
>> @@ -26,6 +26,7 @@
>>  #include <inttypes.h>
>>
>>  #include "libavutil/bswap.h"
>> +#include "libavutil/intreadwrite.h"
>>  #include "config.h"
>>  #include "rgb2rgb.h"
>>  #include "swscale.h"
>> @@ -314,7 +315,6 @@ void rgb12tobgr12(const uint8_t *src, uint8_t *dst,
>> int src_size)
>>      }
>>  }
>>
>> -
>>  #define DEFINE_SHUFFLE_BYTES(a, b, c, d)
>> \
>>  void shuffle_bytes_ ## a ## b ## c ## d(const uint8_t *src,
>> \
>>                                          uint8_t *dst, int src_size)
>> \
>> @@ -333,3 +333,22 @@ DEFINE_SHUFFLE_BYTES(0, 3, 2, 1)
>>  DEFINE_SHUFFLE_BYTES(1, 2, 3, 0)
>>  DEFINE_SHUFFLE_BYTES(3, 0, 1, 2)
>>  DEFINE_SHUFFLE_BYTES(3, 2, 1, 0)
>> +
>> +#define DEFINE_RGB48TOBGR48(i, o)
>>    \
>> +void rgb48tobgr48_ ## i ## o(const uint8_t *src, uint8_t *dst, int
>> src_size) \
>> +{
>>    \
>> +    uint16_t *d = (uint16_t *)dst;
>>    \
>> +    uint16_t *s = (uint16_t *)src;
>>    \
>> +    int i, num_pixels = src_size >> 1;
>>    \
>> +
>>    \
>> +    for (i = 0; i < num_pixels; i += 3) {
>>    \
>> +        AV_W ## o ## 16(&d[i    ], AV_R ## i ## 16(&s[i + 2]));
>>    \
>> +        AV_W ## o ## 16(&d[i + 1], AV_R ## i ## 16(&s[i + 1]));
>>    \
>> +        AV_W ## o ## 16(&d[i + 2], AV_R ## i ## 16(&s[i    ]));
>>    \
>> +    }
>>    \
>> +}
>
> the macro argument "i" and the variable "i" should use different
> identifiers to avoid unexpected bugs when something more complex is
> passed in as i
>
>
>> +
>> +DEFINE_RGB48TOBGR48(L, L)
>> +DEFINE_RGB48TOBGR48(B, B)
>> +DEFINE_RGB48TOBGR48(L, B)
>> +DEFINE_RGB48TOBGR48(B, L)
>
> only 2 are needed
>
> LL = BB
> LB = BL
>
>
>
>> diff --git a/libswscale/rgb2rgb.h b/libswscale/rgb2rgb.h
>> index c447986..89be977 100644
>> --- a/libswscale/rgb2rgb.h
>> +++ b/libswscale/rgb2rgb.h
>> @@ -52,6 +52,10 @@ extern void (*rgb32tobgr15)(const uint8_t *src, uint8_t
>> *dst, int src_size);
>>
>>  extern void (*shuffle_bytes_2103)(const uint8_t *src, uint8_t *dst, int
>> src_size);
>>
>> +void rgb48tobgr48_LL(const uint8_t *src, uint8_t *dst, int src_size);
>> +void rgb48tobgr48_BB(const uint8_t *src, uint8_t *dst, int src_size);
>> +void rgb48tobgr48_LB(const uint8_t *src, uint8_t *dst, int src_size);
>> +void rgb48tobgr48_BL(const uint8_t *src, uint8_t *dst, int src_size);
>
> they should have a sws_ prefix to avoid name clashes with other libs

All functions in that header do not have any prefix, and I do not
think prefix is really needed, because
those functions should be internal only.
>
> [...]
>> @@ -532,7 +539,7 @@ static rgbConvFn findRgbConvFn(SwsContext *c)
>>      rgbConvFn conv = NULL;
>>
>>  #define IS_NOT_NE(bpp, fmt) \
>> -    (((bpp + 7) >> 3) == 2 && \
>> +    (bpp <= 16 && \
>>       (!(av_pix_fmt_descriptors[fmt].flags & PIX_FMT_BE) !=
>> !HAVE_BIGENDIAN))
>>
>>      /* if this is non-native rgb444/555/565, don't handle it here. */
>
> this looks unrelated

It is related, because rgb48be <-> rgb48le would use scaled variant
which is extremely stupid idea.

>
> otherwise LGTM
>
> [...]