[FFmpeg-devel] libavcodec : add psd image file decoder

Michael Niedermayer michael at niedermayer.cc
Mon Jul 25 03:46:47 EEST 2016


On Sun, Jul 24, 2016 at 03:21:00PM +0200, Martin Vignali wrote:
> > breaks fate
> >
> > make: *** [fate-filter-metadata-ebur128] Error 1
> > --- -   2016-07-23 18:50:11.633752058 +0200
> > +++ tests/data/fate/probe-format-roundup1414    2016-07-23
> > 18:50:10.913635588 +0200
> > @@ -1 +1 @@
> > -mpeg
> > +psd_pipe
> > Test probe-format-roundup1414 failed. Look at
> > tests/data/fate/probe-format-roundup1414.err for details.
> > make: *** [fate-probe-format-roundup1414] Error 1
> > --- -   2016-07-23 18:50:11.657384876 +0200
> > +++ tests/data/fate/probe-format-roundup997     2016-07-23
> > 18:50:10.917635588 +0200
> > @@ -1 +1 @@
> > -mpeg
> > +psd_pipe
> > Test probe-format-roundup997 failed. Look at
> > tests/data/fate/probe-format-roundup997.err for details.
> > make: *** [fate-probe-format-roundup997] Error 1
> > --- -   2016-07-23 18:50:11.667958765 +0200
> > +++ tests/data/fate/probe-format-roundup1383    2016-07-23
> > 18:50:10.917635588 +0200
> > @@ -1 +1 @@
> > -mp3
> > +psd_pipe
> > Test probe-format-roundup1383 failed. Look at
> > tests/data/fate/probe-format-roundup1383.err for details.
> > make: *** [fate-probe-format-roundup1383] Error 1
> > make: Target `fate' not remade because of errors.
> >
> >
> Hello,
> 
> Corrected patch in attach who fix the psd_probe function.
> 
> Comments welcome
> 
> Martin

>  Changelog                |    1 
>  doc/general.texi         |    2 
>  libavcodec/Makefile      |    1 
>  libavcodec/allcodecs.c   |    1 
>  libavcodec/avcodec.h     |    1 
>  libavcodec/codec_desc.c  |    7 
>  libavcodec/psd.c         |  395 +++++++++++++++++++++++++++++++++++++++++++++++
>  libavformat/Makefile     |    1 
>  libavformat/allformats.c |    1 
>  libavformat/img2.c       |    1 
>  libavformat/img2dec.c    |   35 ++++
>  11 files changed, 446 insertions(+)

this should be split into 2 patches one for libavcodec and libavformat


[...]
> +static int decode_rle(PSDContext * s){
> +    unsigned int scanline_count;
> +    unsigned int sl, count;
> +    unsigned long target_index = 0;
> +
> +    scanline_count = s->height * s->channel_count;
> +
> +    /* scanline table */
> +    if (bytestream2_get_bytes_left(&s->gb) < scanline_count * 2) {
> +        av_log(s->avctx, AV_LOG_ERROR, "Not enough data for rle scanline table.\n");
> +        return AVERROR_INVALIDDATA;
> +    }
> +    bytestream2_skip(&s->gb, scanline_count * 2);/* size of each scanline */
> +
> +    /* decode rle data scanline by scanline */
> +    for (sl = 0; sl < scanline_count; sl++) {
> +        count = 0;
> +
> +        while (count < s->line_size) {

> +            char rle_char = bytestream2_get_byte(&s->gb);

please use intXY_t or int, char can be unsigned


> +
> +            if (rle_char <= 0) {/* byte repeat */

> +                rle_char *= -1;

this is not safe
rle_char is just 8 bit -128 cannot be represented as positive value


> +
> +                if (bytestream2_get_bytes_left(&s->gb) < 1) {
> +                    av_log(s->avctx, AV_LOG_ERROR, "Not enough data for rle scanline.\n");
> +                    return AVERROR_INVALIDDATA;
> +                }
> +
> +                if (target_index + rle_char > s->uncompressed_size) {
> +                    av_log(s->avctx, AV_LOG_ERROR, "Invalid rle char.\n");
> +                    return AVERROR_INVALIDDATA;
> +                }
> +

> +                uint8_t v = bytestream2_get_byte(&s->gb);

mixed declarations and code, please move the uint8_t v up


> +                for (unsigned char p = 0; p <= rle_char; p++) {
> +                    s->tmp[target_index++] = v;
> +                }
> +            } else {
> +                if (bytestream2_get_bytes_left(&s->gb) < rle_char) {
> +                    av_log(s->avctx, AV_LOG_ERROR, "Not enough data for rle scanline.\n");
> +                    return AVERROR_INVALIDDATA;
> +                }
> +

> +                if (target_index + rle_char > s->uncompressed_size) {
> +                    av_log(s->avctx, AV_LOG_ERROR, "Invalid rle char.\n");
> +                    return AVERROR_INVALIDDATA;
> +                }

isnt this off by 1?
rle_char = 0
target_index == uncompressed_size would pass but write at
uncompressed_size whch would be after the array if iam not mistaken


> +
> +                for (int p = 0; p <= rle_char; p++) {
> +                    uint8_t v = bytestream2_get_byte(&s->gb);
> +                    s->tmp[target_index++] = v;
> +                }
> +            }
> +            count += rle_char + 1;
> +        }
> +    }
> +
> +    return 0;
> +}
> +
> +static int decode_frame(AVCodecContext *avctx, void *data,
> +                        int *got_frame, AVPacket *avpkt)
> +{
> +    int ret;
> +    uint8_t *ptr;
> +    const uint8_t * ptr_data;
> +    int index_out, c, y, x, p;
> +
> +    AVFrame *picture = data;
> +
> +    PSDContext *s = avctx->priv_data;
> +    s->avctx     = avctx;
> +    s->channel_count = 0;
> +    s->channel_depth = 0;
> +    s->tmp           = NULL;
> +    s->line_size     = 0;
> +
> +    bytestream2_init(&s->gb, avpkt->data, avpkt->size);
> +
> +    if ((ret = decode_header(s)) < 0)
> +        return ret;
> +
> +    s->pixel_size = s->channel_depth >> 3;/* in byte */
> +    s->line_size = s->width * s->pixel_size;
> +    s->uncompressed_size = s->line_size * s->height * s->channel_count;
> +
> +    switch (s->color_mode) {
> +    case PSD_RGB:
> +        if (s->channel_count == 3) {
> +            if (s->channel_depth == 8) {
> +                avctx->pix_fmt = AV_PIX_FMT_RGB24;
> +            } else if (s->channel_depth == 16) {
> +                avctx->pix_fmt = AV_PIX_FMT_RGB48BE;
> +            } else {
> +                avpriv_report_missing_feature(avctx, "channel depth unsupported for rgb %d", s->channel_depth);
> +                return AVERROR_PATCHWELCOME;
> +            }
> +        } else if (s->channel_count == 4) {
> +            if (s->channel_depth == 8) {
> +                avctx->pix_fmt = AV_PIX_FMT_RGBA;
> +            } else if (s->channel_depth == 16) {
> +                avctx->pix_fmt = AV_PIX_FMT_RGBA64BE;
> +            } else {
> +                avpriv_report_missing_feature(avctx, "channel depth unsupported for rgb %d", s->channel_depth);
> +                return AVERROR_PATCHWELCOME;
> +            }
> +        } else {
> +            avpriv_report_missing_feature(avctx, "channel count unsupported for rgb %d", s->channel_count);
> +            return AVERROR_PATCHWELCOME;
> +        }
> +        break;
> +    case PSD_GRAYSCALE:
> +        if (s->channel_count == 1) {
> +            if (s->channel_depth == 8) {
> +                avctx->pix_fmt = AV_PIX_FMT_GRAY8;
> +            } else if (s->channel_depth == 16) {
> +                avctx->pix_fmt = AV_PIX_FMT_GRAY16BE;
> +            } else {
> +                avpriv_report_missing_feature(avctx, "channel depth unsupported for grayscale %d", s->channel_depth);
> +                return AVERROR_PATCHWELCOME;
> +            }
> +        } else if (s->channel_count == 2) {
> +            if (s->channel_depth == 8) {
> +                avctx->pix_fmt = AV_PIX_FMT_YA8;
> +            } else if (s->channel_depth == 16) {
> +                avctx->pix_fmt = AV_PIX_FMT_YA16BE;
> +            } else {
> +                avpriv_report_missing_feature(avctx, "channel depth unsupported for grayscale %d", s->channel_depth);
> +                return AVERROR_PATCHWELCOME;
> +            }
> +        } else {
> +            avpriv_report_missing_feature(avctx, "channel count unsupported for grayscale %d", s->channel_count);
> +            return AVERROR_PATCHWELCOME;
> +        }
> +        break;
> +    default:
> +        avpriv_report_missing_feature(avctx, "color mode unsupported %d", s->color_mode);
> +        return AVERROR_PATCHWELCOME;
> +    }
> +
> +    if ((ret = ff_get_buffer(avctx, picture, 0)) < 0)
> +        return ret;
> +

> +    /* decode picture if need */
> +    if (s->compression == PSD_RLE) {
> +        s->tmp = av_malloc(s->uncompressed_size);

missing malloc failure check

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Avoid a single point of failure, be that a person or equipment.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160725/ac9e1cb4/attachment.sig>


More information about the ffmpeg-devel mailing list