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.
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
