diff mbox

[FFmpeg-devel] lavc/psd: Interpret duotone as grayscale

Message ID 201701101751.19199.cehoyos@ag.or.at
State Accepted
Headers show

Commit Message

Carl Eugen Hoyos Jan. 10, 2017, 4:51 p.m. UTC
Hi!

Attached patch does what other applications do with duotone and fixes the 
provided sample.

Please comment, Carl Eugen
From 32f777d3a63007a91ab3c08990bface1bbf79be9 Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos <cehoyos@ag.or.at>
Date: Tue, 10 Jan 2017 17:49:31 +0100
Subject: [PATCH] lavc/psd: Interpret DUOTONE as GRAYSCALE.

Fixes a sample from ticket #6045.
---
 libavcodec/psd.c |    1 +
 1 file changed, 1 insertion(+)

Comments

Martin Vignali Jan. 10, 2017, 5:42 p.m. UTC | #1
Hello,

Seems strange, that duotone is interpreted like grayscale
(i doesn't test yet your patch)

You can find a sample here (and an rgb png of the result in photoshop), for
a duotone file with color
https://we.tl/mbvMt5bytk

I think, duotone, need to be interpreted using information in Image
Ressource

I never use Duotone before, so i can't really help to the right way to
interpret this kind of file

Martin
Carl Eugen Hoyos Jan. 10, 2017, 6:49 p.m. UTC | #2
2017-01-10 18:42 GMT+01:00 Martin Vignali <martin.vignali@gmail.com>:

> Seems strange, that duotone is interpreted like grayscale

It's what gimp, ImageMagick and FreeImage do and what Adobe recommends.

Did you see the BITMAP patch?

Carl Eugen
Martin Vignali Jan. 10, 2017, 10:23 p.m. UTC | #3
> > Seems strange, that duotone is interpreted like grayscale
>
> It's what gimp, ImageMagick and FreeImage do and what Adobe recommends.
>
>
> Seems to be also the case of Apple Preview.

Seems not the best way to manage this kind of file

Doesn't know the general rules of this project, about that :
Is it better to not support a file, or wrongly interpret it ?

Martin
Carl Eugen Hoyos Jan. 10, 2017, 10:39 p.m. UTC | #4
2017-01-10 23:23 GMT+01:00 Martin Vignali <martin.vignali@gmail.com>:
>> > Seems strange, that duotone is interpreted like grayscale
>>
>> It's what gimp, ImageMagick and FreeImage do and what Adobe recommends.
>
> Seems to be also the case of Apple Preview.
>
> Seems not the best way to manage this kind of file

Not sure if I understand:
What is the alternative? Do you know how to interpret
the duotone specification?

> Doesn't know the general rules of this project, about that :
> Is it better to not support a file

> or wrongly interpret it ?

What is wrong about following the specification?

Did you see the BITMAP patch?

Carl Eugenpeg.org/mailman/listinfo/ffmpeg-devel
Martin Vignali Jan. 10, 2017, 10:57 p.m. UTC | #5
I don't have a strong opinion about duo tone (i never use it)

> Seems not the best way to manage this kind of file
>
> Not sure if I understand:
> What is the alternative? Do you know how to interpret
> the duotone specification?
>

I think it's better to have the real color :-)
But i don't know how to interpret the DuoTone information

>
> > Doesn't know the general rules of this project, about that :
> > Is it better to not support a file
>
> > or wrongly interpret it ?
>
> What is wrong about following the specification?
>
>
> Sorry didn't see this part of the spec :
"Other applications that read Photoshop files can treat a duotone image as
a gray image, and just preserve the contents of the duotone information
when reading and writing the file."

So probably ok in that case, but maybe with a warning, so the user can
know, that the file is not correctly decode.

Martin
Carl Eugen Hoyos Jan. 10, 2017, 11:21 p.m. UTC | #6
2017-01-10 23:57 GMT+01:00 Martin Vignali <martin.vignali@gmail.com>:
> I don't have a strong opinion about duo tone (i never use it)
>
>> Seems not the best way to manage this kind of file
>>
>> Not sure if I understand:
>> What is the alternative? Do you know how to interpret
>> the duotone specification?
>>
>
> I think it's better to have the real color :-)
> But i don't know how to interpret the DuoTone information
>
>>
>> > Doesn't know the general rules of this project, about that :
>> > Is it better to not support a file
>>
>> > or wrongly interpret it ?
>>
>> What is wrong about following the specification?
>
> Sorry didn't see this part of the spec :
> "Other applications that read Photoshop files can treat a duotone image as
> a gray image, and just preserve the contents of the duotone information
> when reading and writing the file."
>
> So probably ok in that case, but maybe with a warning, so the user can
> know, that the file is not correctly decode.

Pushed with a warning.

Feel free to look at my 32bit patch attached to ticket #6045, perhaps
you know how to fix it.

Thank you, Carl Eugen
Martin Vignali Jan. 10, 2017, 11:28 p.m. UTC | #7
>
> Feel free to look at my 32bit patch attached to ticket #6045, perhaps
> you know how to fix it.
>
>
I think 32bit in psd are for float data,
maybe something like the float to 16i used inside exr.

Doesn't tested, and i will not have time to take a better look soon.

Martin
diff mbox

Patch

diff --git a/libavcodec/psd.c b/libavcodec/psd.c
index 7587ed9..a71082d 100644
--- a/libavcodec/psd.c
+++ b/libavcodec/psd.c
@@ -352,6 +352,7 @@  static int decode_frame(AVCodecContext *avctx, void *data,
             return AVERROR_PATCHWELCOME;
         }
         break;
+    case PSD_DUOTONE:
     case PSD_GRAYSCALE:
         if (s->channel_count == 1) {
             if (s->channel_depth == 8) {