[FFmpeg-devel] [PATCH]Support 32bit palette in targa

[Carl Eugen Hoyos]

On Tuesday 09 October 2012 01:14:06 am Michael Niedermayer wrote:
> On Mon, Oct 08, 2012 at 01:11:47PM +0200, Carl Eugen Hoyos wrote:
> > Hi!
> >
> > The Targa specification and gimp agree that the targa palette
> > may contain transparency information.
> > ImageMagick is broken: It allows 32bit palette entries but
> > reads them identically as 24bit entries leading to a broken
> > ("shifted") image.
> >
> > Please comment, Carl Eugen
> >
> >  targa.c    |    5 +++++
> >  targaenc.c |    6 +++---
> >  2 files changed, 8 insertions(+), 3 deletions(-)
> > 61f853c3def447bafd8dc06deaaacc9c7c204d32  patchtgapal32.diff
> > diff --git a/libavcodec/targa.c b/libavcodec/targa.c
> > index 339d7c4..5e9d2e9 100644
> > --- a/libavcodec/targa.c
> > +++ b/libavcodec/targa.c
> > @@ -181,6 +181,7 @@ static int decode_frame(AVCodecContext *avctx,
> >              return -1;
> >          }
> >          switch (csize) {
> > +        case 32: pal_sample_size = 4; break;
> >          case 24: pal_sample_size = 3; break;
> >          case 16:
> >          case 15: pal_sample_size = 2; break;
> > @@ -201,6 +202,10 @@ static int decode_frame(AVCodecContext *avctx,
> >                  return AVERROR_INVALIDDATA;
> >              }
> >              switch (pal_sample_size) {
> > +            case 4:
> > +                for (t = 0; t < colors; t++)
> > +                    *pal++ = bytestream2_get_le32u(&s->gb);
> > +                break;
> >              case 3:
> >                  /* RGB24 */
> >                  for (t = 0; t < colors; t++)
>
> this is probably ok

Hunk applied.

> > diff --git a/libavcodec/targaenc.c b/libavcodec/targaenc.c
> > index 2f22e94..cc082aa 100644
> > --- a/libavcodec/targaenc.c
> > +++ b/libavcodec/targaenc.c
> > @@ -107,11 +107,11 @@ static int targa_encode_frame(AVCodecContext
> > *avctx, AVPacket *pkt, pkt->data[1]  = 1;          /* palette present */
> >          pkt->data[2]  = TGA_PAL;    /* uncompressed palettised image */
> >          pkt->data[6]  = 1;          /* palette contains 256 entries */
> > -        pkt->data[7]  = 24;         /* palette contains 24 bit entries
> > */ +        pkt->data[7]  = 32;         /* palette contains 32 bit
> > entries */ pkt->data[16] = 8;          /* bpp */
> >          for (i = 0; i < 256; i++)
> > -            AV_WL24(pkt->data + 18 + 3 * i, *(uint32_t *)(p->data[1] + i
> > * 4)); -        out += 256 * 3;             /* skip past the palette we
> > just output */ +            AV_WL32(pkt->data + 18 + 4 * i, *(uint32_t
> > *)(p->data[1] + i * 4)); +        out += 256 * 4;             /* skip
> > past the palette we just output */
>
> if i understand correctly this would break imagemagik reading tgas
> from ffmpeg if so

(Only pal8 tga's.)

> i think the palette should be checked and only stored as 32bit when
> its needed for some entry

As in attached?

> also please add a regression test once encoder and decoder support it

Wouldn't this need transparency in vsynth1 or vsynth2?

Thank you, Carl Eugen
-------------- next part --------------
diff --git a/libavcodec/targaenc.c b/libavcodec/targaenc.c
index 35e3ea5..4a71f27 100644
--- a/libavcodec/targaenc.c
+++ b/libavcodec/targaenc.c
@@ -103,16 +103,27 @@ static int targa_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
 
     avctx->bits_per_coded_sample = av_get_bits_per_pixel(&av_pix_fmt_descriptors[avctx->pix_fmt]);
     switch(avctx->pix_fmt) {
-    case AV_PIX_FMT_PAL8:
+    case AV_PIX_FMT_PAL8: {
+        int pal_bpp = 24; /* Only write 32bit palette if there is transparency information */
+        for (i = 0; i < 256; i++)
+            if (AV_RN32(p->data[1] + 4 * i) >> 24 != 0xFF) {
+                pal_bpp = 32;
+                break;
+            }
         pkt->data[1]  = 1;          /* palette present */
         pkt->data[2]  = TGA_PAL;    /* uncompressed palettised image */
         pkt->data[6]  = 1;          /* palette contains 256 entries */
-        pkt->data[7]  = 24;         /* palette contains 24 bit entries */
+        pkt->data[7]  = pal_bpp;    /* palette contains pal_bpp bit entries */
         pkt->data[16] = 8;          /* bpp */
         for (i = 0; i < 256; i++)
+            if (pal_bpp == 32) {
+                AV_WL32(pkt->data + 18 + 4 * i, *(uint32_t *)(p->data[1] + i * 4));
+            } else {
             AV_WL24(pkt->data + 18 + 3 * i, *(uint32_t *)(p->data[1] + i * 4));
+            }
-        out += 256 * 3;             /* skip past the palette we just output */
+        out += 32 * pal_bpp;        /* skip past the palette we just output */
         break;
+        }
     case AV_PIX_FMT_GRAY8:
         pkt->data[2]  = TGA_BW;     /* uncompressed grayscale image */
         avctx->bits_per_coded_sample = 0x28;