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

Reimar Döffinger Reimar.Doeffinger at gmx.de
Tue Aug 26 09:29:59 CEST 2014

On 25.08.2014, at 19:33, Christophe Gisquet <christophe.gisquet at gmail.com> wrote:
> Hi,
> 2014-08-25 6:41 GMT+02:00 Reimar Döffinger <Reimar.Doeffinger at gmx.de>:
>> The colorspace is supposed to be authoritative.
>> If it says 16 bit, then showing it as 16 bit must work properly.
>> The only intention of bits_per_raw_sample is to indicate that there is no point in storing some of the lower bits if you have the option.
> ie, actually writing a smaller bitdepth, like what was achieved for
> ppm by my patch and Carl Eugen's ?
> I find this very inconvenient for 2 things:
> - you force an additional roundtrip to 16bps when going eg 12bps to
> 12bps, while forcing 16bps just moves the scaling from the decoder to
> outside (ok that's one additional copy)

There is no "round trip" really just an additional shift on reading and writing.

> - you need to maintain different buffers for predictive codecs (ie
> both the encoder and the decoder), one being what is actually coded,
> and the other for the output of the decoder/input of the encoder
> (though it is not an actual full copy and can just be done in coding
> units eg MB)

Currently no relevant codecs use 12 bits, which is the reason why 9 and 10 bits exist as separate formats while 12 bits does not.
For simple formats, doing a val = (val << 4) | (val >> 12) before each store is completely acceptable.
Amd for reference frames you certainly do not need an actual full copy, that would be silly, you just need to add a shift after each load.

> I don't think I can have time both for convincing you and CE of this,
> then implement fixes everywhere needed, then introduce incompatible
> APIs, so just read this as a statement of my opinion, and let's close
> the subject. I have understood this line of thought is futile.

I don't have a particular opinion on what the design should be, I am just describing how it was intended.

>> That can be solved by a common function if necessary, alternatively a 12 bit colorspace could be added, but the cost/benefit is questionable.
> I think at this point they all have in common "sample storage rounded
> to the upper bytecount" and "raw_bits is indicative of what they
> hold". Adding them is just going to make the scalers more tentacular.
> But my opinion of moving the scaling from the decoder to a specialized
> function also does this.

There is the additional issue that it (just like changing the meaning of raw bits) will require each application to add support for it.

More information about the ffmpeg-devel mailing list