[FFmpeg-devel] [PATCH] png: Fully support the tRNS chunk

[Paul B Mahol]

On 8/30/15, Donny Yang <work at kota.moe> wrote:
> On 31 August 2015 at 00:58, Paul B Mahol <onemda at gmail.com> wrote:
>
>> On 8/28/15, Donny Yang <work at kota.moe> wrote:
>> > Signed-off-by: Donny Yang <work at kota.moe>
>> > ---
>> >  libavcodec/pngdec.c | 98
>> > ++++++++++++++++++++++++++++++++++++++++++++++++-----
>> >  1 file changed, 89 insertions(+), 9 deletions(-)
>> >
>> > diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
>> > index 6e7eae0..1153d65 100644
>> > --- a/libavcodec/pngdec.c
>> > +++ b/libavcodec/pngdec.c
>> > @@ -21,6 +21,7 @@
>> >
>> >  //#define DEBUG
>> >
>> > +#include "libavutil/avassert.h"
>> >  #include "libavutil/bprint.h"
>> >  #include "libavutil/imgutils.h"
>> >  #include "avcodec.h"
>> > @@ -59,6 +60,7 @@ typedef struct PNGDecContext {
>> >      int bits_per_pixel;
>> >      int bpp;
>> >      int has_trns;
>> > +    uint8_t transparent_colour_be[6];
>> >
>> >      uint8_t *image_buf;
>> >      int image_linesize;
>> > @@ -590,6 +592,7 @@ static int decode_idat_chunk(AVCodecContext *avctx,
>> > PNGDecContext *s,
>> >                               uint32_t length, AVFrame *p)
>> >  {
>> >      int ret;
>> > +    size_t byte_depth = s->bit_depth > 8 ? 2 : 1;
>> >
>> >      if (!(s->state & PNG_IHDR)) {
>> >          av_log(avctx, AV_LOG_ERROR, "IDAT without IHDR\n");
>> > @@ -641,6 +644,31 @@ static int decode_idat_chunk(AVCodecContext
>> > *avctx,
>> > PNGDecContext *s,
>> >              return AVERROR_INVALIDDATA;
>> >          }
>> >
>> > +        if (s->has_trns && s->color_type != PNG_COLOR_TYPE_PALETTE) {
>> > +            switch (avctx->pix_fmt) {
>> > +            case AV_PIX_FMT_RGB24:
>> > +                avctx->pix_fmt = AV_PIX_FMT_RGBA;
>> > +                break;
>> > +
>> > +            case AV_PIX_FMT_RGB48BE:
>> > +                avctx->pix_fmt = AV_PIX_FMT_RGBA64BE;
>> > +                break;
>> > +
>> > +            case AV_PIX_FMT_GRAY8:
>> > +                avctx->pix_fmt = AV_PIX_FMT_YA8;
>> > +                break;
>> > +
>> > +            case AV_PIX_FMT_GRAY16BE:
>> > +                avctx->pix_fmt = AV_PIX_FMT_YA16BE;
>> > +                break;
>> > +
>> > +            default:
>> > +                av_assert0(0);
>> > +            }
>> > +
>> > +            s->bpp += byte_depth;
>> > +        }
>> > +
>> >          if ((ret = ff_thread_get_buffer(avctx, &s->picture,
>> > AV_GET_BUFFER_FLAG_REF)) < 0)
>> >              return ret;
>> >          if (avctx->codec_id == AV_CODEC_ID_APNG && s->last_dispose_op
>> > !=
>> > APNG_DISPOSE_OP_PREVIOUS) {
>> > @@ -691,9 +719,21 @@ static int decode_idat_chunk(AVCodecContext
>> > *avctx,
>> > PNGDecContext *s,
>> >          s->zstream.avail_out = s->crow_size;
>> >          s->zstream.next_out  = s->crow_buf;
>> >      }
>> > +
>> >      s->state |= PNG_IDAT;
>> > -    if ((ret = png_decode_idat(s, length)) < 0)
>> > +
>> > +    /* set image to non-transparent bpp while decompressing */
>> > +    if (s->has_trns && s->color_type != PNG_COLOR_TYPE_PALETTE)
>> > +        s->bpp -= byte_depth;
>> > +
>> > +    ret = png_decode_idat(s, length);
>> > +
>> > +    if (s->has_trns && s->color_type != PNG_COLOR_TYPE_PALETTE)
>> > +        s->bpp += byte_depth;
>> > +
>> > +    if (ret < 0)
>> >          return ret;
>> > +
>> >      bytestream2_skip(&s->gb, 4); /* crc */
>> >
>> >      return 0;
>> > @@ -727,17 +767,33 @@ static int decode_trns_chunk(AVCodecContext
>> > *avctx,
>> > PNGDecContext *s,
>> >  {
>> >      int v, i;
>> >
>> > -    /* read the transparency. XXX: Only palette mode supported */
>> > -    if (s->color_type != PNG_COLOR_TYPE_PALETTE ||
>> > -            length > 256 ||
>> > -            !(s->state & PNG_PLTE))
>> > +    if (s->color_type == PNG_COLOR_TYPE_PALETTE) {
>> > +        if (length > 256 || !(s->state & PNG_PLTE))
>> > +            return AVERROR_INVALIDDATA;
>> > +
>> > +        for (i = 0; i < length; i++) {
>> > +            v = bytestream2_get_byte(&s->gb);
>> > +            s->palette[i] = (s->palette[i] & 0x00ffffff) | (v << 24);
>> > +        }
>> > +    } else if (s->color_type == PNG_COLOR_TYPE_GRAY || s->color_type
>> > ==
>> > PNG_COLOR_TYPE_RGB) {
>> > +        if ((s->color_type == PNG_COLOR_TYPE_GRAY && length != 2) ||
>> > +            (s->color_type == PNG_COLOR_TYPE_RGB && length != 6))
>> > +            return AVERROR_INVALIDDATA;
>> > +
>> > +        for (i = 0; i < length / 2; i++) {
>> > +            /* only use the least significant bits */
>> > +            v = bytestream2_get_be16(&s->gb) & ((1 << s->bit_depth) -
>> 1);
>>
>> This can crash if length of trns chunk is > 6.
>>
>
> It shouldn't reach this code if the chunk isn't 2 or 6 bytes, since it is
> filtered out by the above check.

Indeed, please ignore.