> > Subject: [PATCH 1/2] libavcodec : add decoder for Photoshop PSD image > file. > > ^ > image file*s* and no dot at the end > > > Decode the Image Data Section (who contain merge picture). > ^ > (which contains merged pictures) > > > Support RGB/A and Grayscale/A in 8bits and 16 bits by channel. > ^ > per channel >
Correct locally > > > Support uncompress and rle compression in Image Data Section. > ^ > decompression > Not sure i understand, do you mean : Support uncompress and rle decompression in Image Data Section. ? Because currently, the psd reader, can manage uncompress or rle. > > > --- > > Changelog | 1 + > > doc/general.texi | 2 + > > libavcodec/Makefile | 1 + > > libavcodec/allcodecs.c | 1 + > > libavcodec/avcodec.h | 1 + > > libavcodec/psd.c | 428 ++++++++++++++++++++++++++++++ > +++++++++++++++++++ > > 6 files changed, 434 insertions(+) > > create mode 100644 libavcodec/psd.c > > Have you read doc/developers.texi, section "New codecs or formats > checklist"? > * needs minor version bump > * codec should be added to libavcodec/codec_desc.c > Correct locally. > > Also a fate test would be nice. > Il would add a fate test, with the samples (in the previous link, when the path will be apply (to not mix, path and fate test). > > > diff --git a/libavcodec/psd.c b/libavcodec/psd.c > > new file mode 100644 > > index 0000000..90ee337 > > --- /dev/null > > +++ b/libavcodec/psd.c > > @@ -0,0 +1,428 @@ > [...] > > + s->height = bytestream2_get_be32(&s->gb); > > + > > + if ((s->height < 1) || (s->height > 30000)) { > > Why 30000? > ff_set_dimensions already checks for sane dimensions. > Following adobe specs : http://www.adobe.com/devnet-apps/photoshop/fileformatashtml/#50577409_pgfId-1055726 in a psd file, the width or height, can't be > to 30 000 pixels. > > > + av_log(s->avctx, AV_LOG_ERROR, "Invalid height %d.\n", > s->height); > > + return AVERROR_INVALIDDATA; > > + } > > + > > + s->width = bytestream2_get_be32(&s->gb); > > + if ((s->width < 1) || (s->width > 30000)) { > > + av_log(s->avctx, AV_LOG_ERROR, "Invalid width %d.\n", s->width); > > + return AVERROR_INVALIDDATA; > > + } > > + > > + if ((ret = ff_set_dimensions(s->avctx, s->width, s->height)) < 0) > > + return ret; > > + > > + s->channel_depth = bytestream2_get_be16(&s->gb); > > + > > + color_mode = bytestream2_get_be16(&s->gb); > > + switch (color_mode) { > > + case 0: > > + s->color_mode = PSD_BITMAP; > > + break; > > + case 1: > > + s->color_mode = PSD_GRAYSCALE; > > + break; > > + case 2: > > + s->color_mode = PSD_INDEXED; > > + break; > > + case 3: > > + s->color_mode = PSD_RGB; > > + break; > > + case 4: > > + s->color_mode = PSD_CMYK; > > + break; > > + case 7: > > + s->color_mode = PSD_MULTICHANNEL; > > + break; > > + case 8: > > + s->color_mode = PSD_DUOTONE; > > + break; > > + case 9: > > + s->color_mode = PSD_LAB; > > + break; > > + default: > > + av_log(s->avctx, AV_LOG_ERROR, "Unknown color mode %d.\n", > color_mode); > > + return AVERROR_INVALIDDATA; > > + } > > + > > + /* color map data */ > > + len_section = bytestream2_get_be32(&s->gb); > > + if (len_section < 0) > > Please add an error message. > Correct locally > > > + return AVERROR_INVALIDDATA; > > + > > + if (bytestream2_get_bytes_left(&s->gb) < (len_section + 4)) { /* > section and len next section */ > > + av_log(s->avctx, AV_LOG_ERROR, "Incomplete file.\n"); > > + return AVERROR_INVALIDDATA; > > + } > > + bytestream2_skip(&s->gb, len_section); > > + > > + /* image ressources */ > > + len_section = bytestream2_get_be32(&s->gb); > > + if (len_section < 0) > > Here too. > Correct locally > > > + return AVERROR_INVALIDDATA; > > + > > + if (bytestream2_get_bytes_left(&s->gb) < (len_section + 4)) { /* > section and len next section */ > > + av_log(s->avctx, AV_LOG_ERROR, "Incomplete file.\n"); > > + return AVERROR_INVALIDDATA; > > + } > > + bytestream2_skip(&s->gb, len_section); > > + > > + /* layers and masks */ > > + len_section = bytestream2_get_be32(&s->gb); > > + if (len_section < 0) > > And here. > Correct locally > > [...] > > +static int decode_frame(AVCodecContext *avctx, void *data, > > + int *got_frame, AVPacket *avpkt) > > +{ > > + int ret; > > + uint8_t *ptr; > > + const uint8_t * ptr_data; > ^ > nit: no space between * and variable name. (Also elsewhere.) > Correct locally > > > + int index_out, c, y, x, p; > > + uint8_t eq_channel[4] = {2,0,1,3};;/* RGBA -> GBRA channel order */ > ^ > One ';' too much, use a space instead. > Correct locally > > > + uint8_t plane_number; > > + > > + 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_GBRP; > > + } else if (s->channel_depth == 16) { > > + avctx->pix_fmt = AV_PIX_FMT_GBRP16BE; > > + } else { > > + avpriv_report_missing_feature(avctx, "channel depth %d > unsupported for rgb", s->channel_depth); > > This should just say "Channel depth %d for rgb", because this function will > continue the text with " is not implemented.". Same for the similar cases > below. > Correct locally > > > From 1b85107d53ab4e5748156e8e84cb8fbe26317700 Mon Sep 17 00:00:00 2001 > > From: Martin Vignali <martin.vign...@gmail.com> > > Date: Fri, 18 Nov 2016 18:18:36 +0100 > > Subject: [PATCH 2/2] libavformat : add Photoshop PSD file. > ^ > demuxer > > And this also needs a minor version bump. > Correct locally > > On 18.11.2016 23:41, Martin Vignali wrote: > > You can find sample here : > > https://we.tl/BHKIQCDZZm > > Thanks. I fuzz-tested the demuxer/decoder and didn't find any problems. > However, all the rle samples seem to not decode entirely correct, instead > containing blue/white rectangular areas. > > > Thanks for testing Martin _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel