On 18.11.2016 18:27, Martin Vignali wrote:
> From c41e6c79ed42478a1658c8922d19c832d3a89424 Mon Sep 17 00:00:00 2001
> From: Martin Vignali <martin.vign...@gmail.com>
> Date: Fri, 18 Nov 2016 18:18:20 +0100
> 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
> Support uncompress and rle compression in Image Data Section.
^
decompression
> ---
> 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
Also a fate test would be nice.
> 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.
> + 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.
> + 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.
> + 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.
[...]
> +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.)
> + 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.
> + 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.
> 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.
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.
Best regards,
Andreas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
