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

[Martin Vignali]

>
> > I doesn't find a planar, for YA, so there is now two way to store data
> > one for YA, and one for Gray and GBR.
>
> Sorry, I don't understand this sentence...
>

Not sure i understand what is your question.
I use planar for RGB and Gray, bu for YA, i doesn't find a planar something
like YAP and YAP_16BE.
So i use the "interleaved" pixel format (YA and YA16BE).
These two mode explain, the if/else part in the storage data part of
decode_frame.


>
> >> And the more I think of it, the more likely it seems that (GBR) 9, 10,
> 12
> >> and 14
> >> could also be of use.
> >>
> > Don't think GBR 9, 10, 12, 14, can be use now for psd file.
>
> Then why do you return AVERROR_PATCHWELCOME for the non-8,
> non-16 case?
>
Not sure i understand.
In this version, there is no support for 9, 10, 12, 14 bitdepth. (and i
doesn't think psd file can store these kinds of pixel depth).
But PSD support other pixel depth than 8, and 16 (like float for example).


>
> > +            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);
> > +                return AVERROR_PATCHWELCOME;
> > +            }
>
> > +    color_mode = bytestream2_get_be16(&s->gb);
> > +    switch (color_mode) {
>
> Why is this not "s->color_mode = bytestream2_get_be16(&s->gb);" instead?
>
Seems more clean for me, considering color mode are not "continuous".
so the switch case assign the value after checking it, using the enum.

>
> > +    compression = bytestream2_get_be16(&s->gb);
> > +    switch (compression) {
>
> Same here: Why not "s->compression = bytestream2_get_be16(&s->gb);"?
>
> Correct locally


> From the demuxer patch:
> > +    if (p->buf_size < 26)
> > +        return 0;
>
> Padding alone is 32, so this check is not necessary.
>
>
> Correct locally

Martin