[FFmpeg-devel] [PATCH]lavc/dpx: Support 10-bit packing method b (msbpad)

[Jerome Martinez]

On 16/06/2018 17:49, Carl Eugen Hoyos wrote:
> Hi!
>
> Attached patch allows to decode 10-bit dpx files with packing method
> b, needs the 12-bit patch.

Great to see the support of Method B.

>
> Please comment, Carl Eugen
> [...]
>
> +    *lbuf = (*lbuf << 10) | (*lbuf >> shift);
>

Padding bits are 0 in all DPX files I have seen up to now but in theory 
padding bits are not defined (they are set to 0 in an informative part 
of the spec, the one containing diagrams, which does not mean that they 
must be 0 as it is only an informative part; I don't see elsewhere in 
the spec that padding bits must be 0, I understand that value of padding 
bits are not defined so could be any)

Example of impact on the suggested patch if padding bits are 1:

Method A (shift is 22), hat is currently in FFmpeg:
CCCCCCCCCCBBBBBBBBBBAAAAAAAAAA11 initial after read32 then
BBBBBBBBBBAAAAAAAAAA11CCCCCCCCCC so CCCCCCCCCC with 0x3FF mask then
AAAAAAAAAA11CCCCCCCCCCBBBBBBBBBB so BBBBBBBBBB with 0x3FF mask then
11CCCCCCCCCCBBBBBBBBBBAAAAAAAAAA so AAAAAAAAAA with 0x3FF mask
Padding bits are never used, so they can have any value without impact 
on the pixels

Method B (shift is 20)
11CCCCCCCCCCBBBBBBBBBBAAAAAAAAAA initial after read32 then
CCBBBBBBBBBBAAAAAAAA11CCCCCCCCCC so CCCCCCCCCC with 0x3FF mask then
BBAAAAAAAA11CCCCCCCC??BBBBBBBBBB so BBBBBBBBBB with 0x3FF mask then
11CCCCCCCC??BBBBBBBB??AAAAAAAA11 so AAAAAAAA11 with 0x3FF mask
the last component is "corrupted" by the padding bits if they are not 0.

I suggest to not expect that padding bits are 0 (to not depend on their 
value for filling the pixel content).
For example, by using:
int shift = packing == 1 ? 0 : 2;
then in read10in32() add "<< shift" after read32 call:
*lbuf = read32(ptr, is_big) << shift;
in order to transform Method B in Method A.

Similar issue with 12-bit Method B support (No 0xFFF mask so MSB bits 
may contain 1, which is maybe not expected in other parts of FFmpeg and 
may lead to weird output if padding bits are not 0, by having pixel 
content greater than 1^12-1)

Jérôme