[FFmpeg-devel] make swscale's palette functions public

Reinhard Tartler siretart
Sat May 15 18:39:59 CEST 2010


On Sat, May 15, 2010 at 17:49:28 (CEST), Stefano Sabatini wrote:

> On date Saturday 2010-05-15 17:06:02 +0200, Reinhard Tartler encoded:
>> On Fri, May 14, 2010 at 09:16:28 (CEST), Reinhard Tartler wrote:
>> 
>> >> its probably a good idea but the functions need to be documented in
>> >> teh header
>> >
>> > In this case I propose the following patch for swscale:
>> 
>> Updated Patch with improved documentation by Attila:
>> 
>> Index: swscale.c
>> ===================================================================
>> --- swscale.c	(revision 31179)
>> +++ swscale.c	(working copy)
>> @@ -1424,12 +1424,12 @@
>>  
>>      if (usePal(srcFormat)) {
>>          switch (dstFormat) {
>> -        case PIX_FMT_RGB32  : conv = palette8topacked32; break;
>> -        case PIX_FMT_BGR32  : conv = palette8topacked32; break;
>> -        case PIX_FMT_BGR32_1: conv = palette8topacked32; break;
>> -        case PIX_FMT_RGB32_1: conv = palette8topacked32; break;
>> -        case PIX_FMT_RGB24  : conv = palette8topacked24; break;
>> -        case PIX_FMT_BGR24  : conv = palette8topacked24; break;
>> +        case PIX_FMT_RGB32  : conv = sws_palette8topacked32; break;
>> +        case PIX_FMT_BGR32  : conv = sws_palette8topacked32; break;
>> +        case PIX_FMT_BGR32_1: conv = sws_palette8topacked32; break;
>> +        case PIX_FMT_RGB32_1: conv = sws_palette8topacked32; break;
>> +        case PIX_FMT_RGB24  : conv = sws_palette8topacked24; break;
>> +        case PIX_FMT_BGR24  : conv = sws_palette8topacked24; break;
>>          }
>>      }
>>  
>> @@ -1962,3 +1962,70 @@
>>      return sws_scale(c, src, srcStride, srcSliceY, srcSliceH, dst, dstStride);
>>  }
>>  #endif
>> +
>> +/**
>> + * Convert the palette to the same packet 32-bit format as the palette
>> + */
>
> What's the point of duplicating docs in the implementation?

Perhaps Attila can comment on this better than me since he suggested
that. The comment is not really duplicated, this comment is about the
implementation, the documentation in the header is for users.

>> +void sws_palette8topacked32(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette)
>> +{
>> +    long i;
>> +
>> +    for (i=0; i<num_pixels; i++)
>> +        ((uint32_t *) dst)[i] = ((const uint32_t *) palette)[src[i]];
>> +}
> [...]
>> Index: swscale.h
>> ===================================================================
>> --- swscale.h	(revision 31179)
>> +++ swscale.h	(working copy)
>> @@ -302,4 +302,58 @@
>>                                          int flags, SwsFilter *srcFilter,
>>                                          SwsFilter *dstFilter, const double *param);
>>  
>> +/**
>> + * Convert an 8bit paletted frame into an RGB32 frame
>> + * @param src: source frame buffer
>> + * @param dst: destination frame buffer
>> + * @param num_pixels: number of pixels to convert
>> + * @param palette: array, [256] of RGB32 entries
>> + */
>> +void sws_palette8topacked32(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette);
>
> Convert -> ConvertS, please use third person. Also add an empty line
> before the first @param, the ":" at the end of the param name is
> inconsistent with the FFmpeg doxies.
>
> Same comments apply to the other doxies in the patch.

changed

> [...]
>>  #endif /* SWSCALE_SWSCALE_H */
>> Index: rgb2rgb.c
>> ===================================================================
>> --- rgb2rgb.c	(revision 31179)
>> +++ rgb2rgb.c	(working copy)
>> @@ -207,63 +207,34 @@
>>          rgb2rgb_init_C();
>>  }
>>  
>> -/**
>> - * Convert the palette to the same packet 32-bit format as the palette
>> - */
>>  void palette8topacked32(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette)
>>  {
>> -    long i;
>> -
>> -    for (i=0; i<num_pixels; i++)
>> -        ((uint32_t *) dst)[i] = ((const uint32_t *) palette)[src[i]];
>> +    sws_palette8topacked32(src, dst, num_pixels, palette);
>>  }
>
> Why not directly use sws_palette8XXX() in the code, and get rid of
> these senseless wrapper functions?
>
> They've never been public, so it should be safe to just remove
> them. If you want to keep them for the sake of misbehaving apps using
> the libav* internal API (hi MPlayer!) please mark them with:
> #if LIBSWCALE_VERSION_MAJOR < 1
> ...
> #endif

good idea, implemented.

> But I don't think this is going to be any good, applications using
> internal API are doomed to be broken anyway, if not here somewhere
> else (see my eval.c patches).

Yes, and I think this is undesireable. I can tell you more about the
breakage as soon as I'll upload an ffmpeg 0.6 package for ubuntu/debian.

>
>>  void rgb32to24(const uint8_t *src, uint8_t *dst, long src_size)
>> Index: rgb2rgb.h
>> ===================================================================
>> --- rgb2rgb.h	(revision 31179)
>> +++ rgb2rgb.h	(working copy)
>> @@ -4,7 +4,7 @@
>>   *               Software YUV to YUV converter
>>   *               Software YUV to RGB converter
>>   *  Written by Nick Kurshev.
>> - *  palette & YUV & runtime CPU stuff by Michael (michaelni at gmx.at)
>> + *  YUV & runtime CPU stuff by Michael (michaelni at gmx.at)
>>   *
>>   * This file is part of FFmpeg.
>>   *
>> @@ -66,6 +66,7 @@
>>  void shuffle_bytes_3012(const uint8_t *src, uint8_t *dst, long src_size);
>>  void shuffle_bytes_3210(const uint8_t *src, uint8_t *dst, long src_size);
>>  
>> +/* deprecated, use the public versions in swscale.h */
>>  void palette8topacked32(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette);
>>  void palette8topacked24(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette);
>>  void palette8torgb16(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette);
>
> Regards.

Index: swscale.c
===================================================================
--- swscale.c	(revision 31179)
+++ swscale.c	(working copy)
@@ -1424,12 +1424,12 @@
 
     if (usePal(srcFormat)) {
         switch (dstFormat) {
-        case PIX_FMT_RGB32  : conv = palette8topacked32; break;
-        case PIX_FMT_BGR32  : conv = palette8topacked32; break;
-        case PIX_FMT_BGR32_1: conv = palette8topacked32; break;
-        case PIX_FMT_RGB32_1: conv = palette8topacked32; break;
-        case PIX_FMT_RGB24  : conv = palette8topacked24; break;
-        case PIX_FMT_BGR24  : conv = palette8topacked24; break;
+        case PIX_FMT_RGB32  : conv = sws_palette8topacked32; break;
+        case PIX_FMT_BGR32  : conv = sws_palette8topacked32; break;
+        case PIX_FMT_BGR32_1: conv = sws_palette8topacked32; break;
+        case PIX_FMT_RGB32_1: conv = sws_palette8topacked32; break;
+        case PIX_FMT_RGB24  : conv = sws_palette8topacked24; break;
+        case PIX_FMT_BGR24  : conv = sws_palette8topacked24; break;
         }
     }
 
@@ -1962,3 +1962,70 @@
     return sws_scale(c, src, srcStride, srcSliceY, srcSliceH, dst, dstStride);
 }
 #endif
+
+/**
+ * Convert the palette to the same packet 32-bit format as the palette
+ */
+void sws_palette8topacked32(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette)
+{
+    long i;
+
+    for (i=0; i<num_pixels; i++)
+        ((uint32_t *) dst)[i] = ((const uint32_t *) palette)[src[i]];
+}
+
+/**
+ * Palette format: ABCD -> dst format: ABC
+ */
+void sws_palette8topacked24(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette)
+{
+    long i;
+
+    for (i=0; i<num_pixels; i++) {
+        //FIXME slow?
+        dst[0]= palette[src[i]*4+0];
+        dst[1]= palette[src[i]*4+1];
+        dst[2]= palette[src[i]*4+2];
+        dst+= 3;
+    }
+}
+
+/**
+ * Palette is assumed to contain BGR16
+ */
+void sws_palette8torgb16(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette)
+{
+    long i;
+    for (i=0; i<num_pixels; i++)
+        ((uint16_t *)dst)[i] = ((const uint16_t *)palette)[src[i]];
+}
+
+/**
+ * Palette is assumed to contain BGR16
+ */
+void sws_palette8tobgr16(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette)
+{
+    long i;
+    for (i=0; i<num_pixels; i++)
+        ((uint16_t *)dst)[i] = bswap_16(((const uint16_t *)palette)[src[i]]);
+}
+
+/**
+ * Palette is assumed to contain RGB15
+ */
+void sws_palette8torgb15(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette)
+{
+    long i;
+    for (i=0; i<num_pixels; i++)
+        ((uint16_t *)dst)[i] = ((const uint16_t *)palette)[src[i]];
+}
+
+/**
+ * Palette is assumed to contain BGR15
+ */
+void sws_palette8tobgr15(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette)
+{
+    long i;
+    for (i=0; i<num_pixels; i++)
+        ((uint16_t *)dst)[i] = bswap_16(((const uint16_t *)palette)[src[i]]);
+}
Index: swscale.h
===================================================================
--- swscale.h	(revision 31179)
+++ swscale.h	(working copy)
@@ -302,4 +302,64 @@
                                         int flags, SwsFilter *srcFilter,
                                         SwsFilter *dstFilter, const double *param);
 
+/**
+ * Converts an 8bit paletted frame into an RGB32 frame
+ *
+ * @param src        source frame buffer
+ * @param dst        destination frame buffer
+ * @param num_pixels number of pixels to convert
+ * @param palette    array, [256] of RGB32 entries
+ */
+void sws_palette8topacked32(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette);
+
+/**
+ * Converts an 8bit paletted frame into an RGB24 frame
+ *
+ * @param src        source frame buffer
+ * @param dst        destination frame buffer
+ * @param num_pixels number of pixels to convert
+ * @param palette    array, [256] of RGB24 entries
+ */
+void sws_palette8topacked24(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette);
+
+/**
+ * Converts an 8bit paletted frame into an RGB16 frame
+ *
+ * @param src        source frame buffer
+ * @param dst        destination frame buffer
+ * @param num_pixels number of pixels to convert
+ * @param palette    array, [256] of RGB16 entries
+ */
+void sws_palette8torgb16(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette);
+
+/**
+ * Converts an 8bit paletted frame into an BGR16 frame
+ *
+ * @param src        source frame buffer
+ * @param dst        destination frame buffer
+ * @param num_pixels number of pixels to convert
+ * @param palette    array, [256] of BGR16 entries
+ */
+void sws_palette8tobgr16(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette);
+
+/**
+ * Converts an 8bit paletted frame into an RGB15 frame
+ *
+ * @param src        source frame buffer
+ * @param dst        destination frame buffer
+ * @param num_pixels number of pixels to convert
+ * @param palette    array, [256] of RGB15 entries
+ */
+void sws_palette8torgb15(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette);
+
+/**
+ * Converts an 8bit paletted frame into an BGR15 frame
+ *
+ * @param src        source frame buffer
+ * @param dst        destination frame buffer
+ * @param num_pixels number of pixels to convert
+ * @param palette    array, [256] of BGR15 entries
+ */
+void sws_palette8tobgr15(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette);
+
 #endif /* SWSCALE_SWSCALE_H */
Index: rgb2rgb.c
===================================================================
--- rgb2rgb.c	(revision 31179)
+++ rgb2rgb.c	(working copy)
@@ -207,64 +207,37 @@
         rgb2rgb_init_C();
 }
 
-/**
- * Convert the palette to the same packet 32-bit format as the palette
- */
+#if LIBSWCALE_VERSION_MAJOR < 1
 void palette8topacked32(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette)
 {
-    long i;
-
-    for (i=0; i<num_pixels; i++)
-        ((uint32_t *) dst)[i] = ((const uint32_t *) palette)[src[i]];
+    sws_palette8topacked32(src, dst, num_pixels, palette);
 }
 
-/**
- * Palette format: ABCD -> dst format: ABC
- */
 void palette8topacked24(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette)
 {
-    long i;
-
-    for (i=0; i<num_pixels; i++) {
-        //FIXME slow?
-        dst[0]= palette[src[i]*4+0];
-        dst[1]= palette[src[i]*4+1];
-        dst[2]= palette[src[i]*4+2];
-        dst+= 3;
-    }
+    sws_palette8topacked24(src, dst, num_pixels, palette);
 }
 
-/**
- * Palette is assumed to contain BGR16, see rgb32to16 to convert the palette.
- */
 void palette8torgb16(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette)
 {
-    long i;
-    for (i=0; i<num_pixels; i++)
-        ((uint16_t *)dst)[i] = ((const uint16_t *)palette)[src[i]];
+    sws_palette8torgb16(src, dst, num_pixels, palette);
 }
+
 void palette8tobgr16(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette)
 {
-    long i;
-    for (i=0; i<num_pixels; i++)
-        ((uint16_t *)dst)[i] = bswap_16(((const uint16_t *)palette)[src[i]]);
+    sws_palette8tobgr16(src, dst, num_pixels, palette);
 }
 
-/**
- * Palette is assumed to contain BGR15, see rgb32to15 to convert the palette.
- */
 void palette8torgb15(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette)
 {
-    long i;
-    for (i=0; i<num_pixels; i++)
-        ((uint16_t *)dst)[i] = ((const uint16_t *)palette)[src[i]];
+    sws_palette8torgb15(src, dst, num_pixels, palette);
 }
+
 void palette8tobgr15(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette)
 {
-    long i;
-    for (i=0; i<num_pixels; i++)
-        ((uint16_t *)dst)[i] = bswap_16(((const uint16_t *)palette)[src[i]]);
+    sws_palette8tobgr15(src, dst, num_pixels, palette);
 }
+#endif
 
 void rgb32to24(const uint8_t *src, uint8_t *dst, long src_size)
 {
Index: rgb2rgb.h
===================================================================
--- rgb2rgb.h	(revision 31179)
+++ rgb2rgb.h	(working copy)
@@ -4,7 +4,7 @@
  *               Software YUV to YUV converter
  *               Software YUV to RGB converter
  *  Written by Nick Kurshev.
- *  palette & YUV & runtime CPU stuff by Michael (michaelni at gmx.at)
+ *  YUV & runtime CPU stuff by Michael (michaelni at gmx.at)
  *
  * This file is part of FFmpeg.
  *
@@ -66,6 +66,7 @@
 void shuffle_bytes_3012(const uint8_t *src, uint8_t *dst, long src_size);
 void shuffle_bytes_3210(const uint8_t *src, uint8_t *dst, long src_size);
 
+/* deprecated, use the public versions in swscale.h */
 void palette8topacked32(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette);
 void palette8topacked24(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette);
 void palette8torgb16(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette);


-- 
Gruesse/greetings,
Reinhard Tartler, KeyID 945348A4




More information about the ffmpeg-devel mailing list