[FFmpeg-devel] [PATCH]Fix pnm encoding with bpc set

Christophe Gisquet christophe.gisquet at gmail.com
Sun Aug 24 14:02:05 CEST 2014


Hi,

I think a first point must be cleared, seeing your reaction to my
pnmdec comment. Here' are the opinions:
- Mine: bits_per_raw_sample should indicate the dynamics of the signal
when it is different from the colorspace bitdepth
- ffmpeg's and yours (if I'm not mistaken): the signal bitdepth should
always match the colorspace one, and bits_per_raw_sample should
indicate the original bitdepth

I believe rescaling in the decoder, like pnmdec does, is wrong. Like
CMYK support, doing that is the job of a rescaler independent of the
codec (yes that would be a speed hit). Otherwise, several "codecs"
(dpx comes to mind) would have to implement it, up to getting it
slightly wrong.

2014-08-24 13:25 GMT+02:00 Carl Eugen Hoyos <cehoyos at ag.or.at>:
> Christophe Gisquet <christophe.gisquet <at> gmail.com> writes:
>> ljpeg 36bits from #2966 and having the actual
>> bits_per_raw_sample value
>
> The sample is broken, you cannot use it to test
> anything else but ticket #2966.

It isn't, it decodes fine, the pixels are just marked differently. It
is the very basis for our argumentation. You probably think it should
be rescaled to reach a bitdepth of 16, and I disagree. That
potentially removes any opportunity for absolutely lossless
processing. Or force to do round-trips like your patch proposes.

> I believe it is very useful and works as expected,
> the only issue I found today is that it gets lost
> on endian conversion in libswscale (that should be
> lossless).

AFAIK, swscale never has read or write access to bits_per_raw_sample.
I actually traced that, and that's what patch 4/5 was fixing in my
dropped pachset.

> And this is working as expected for the samples
> from ticket #2952, it is broken for the sample
> in ticket #2966.

OK, potentially any of our solutions should have 2966 and 2952 working.

> Yes, this is to make sure that you can use the same
> frame for encoders (displays) that only support 16bpc
> (as most do) while it allows to know how many bpc
> are actually coded.

cf. my representation of your opinion above. As told, I disagree,
because this fixed by doing the scaling outside the codec. If the
scaling is unneeded (eg I'm outputting to a codec that has some
support for [9-15] bitdepth like dpx, pnm, prores, dnxhd or ...),
we've just forced a round-trip for nothing.

>> We have 2 cases resulting in the same info but
>> needing different processing.
>
> Because of ticket #2966.

To fix according to your view, it needs rescaling. Are you going to
implement pnmdec's one?

> The values have the right dynamics except for the
> sample from ticket #2966.

There are several codecs that could output values not ok.

>> A potential "fix" would be to remove
>> bits_per_raw_sample setting from pnmdec.c
>
> Why??

That's where I understood we had diverging views on the meaning of
bits_per_raw_sample.

>> But I expect some other decoders to be potentially
>> wrong/doing yet another thing at this point, so
>> bits_per_raw_sample is not completely reliable.
>
> Bugs in FFmpeg do exist.

Yes, until bits_per_raw_sample meaning has its semantics clarified
(and the codecs fixed according to it) or a proper signaling done,
there will be.

-- 
Christophe


More information about the ffmpeg-devel mailing list