| Message ID | CAB0OVGokov_4B2vWwAT+=TnV_6oDvb5vOvkq-nVAHU8DhoSf6Q@mail.gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
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
From 401bdebba6dc8fa2253c720107a971b3fbf2c312 Mon Sep 17 00:00:00 2001 From: Carl Eugen Hoyos <ceffmpeg@gmail.com> Date: Sat, 16 Jun 2018 17:47:46 +0200 Subject: [PATCH] lavc/dpx: Support 10-bit packing method b (msbpad). --- libavcodec/dpx.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/libavcodec/dpx.c b/libavcodec/dpx.c index 69d594b..0650a20 100644 --- a/libavcodec/dpx.c +++ b/libavcodec/dpx.c @@ -51,7 +51,7 @@ static unsigned int read32(const uint8_t **ptr, int is_big) } static uint16_t read10in32(const uint8_t **ptr, uint32_t * lbuf, - int * n_datum, int is_big) + int * n_datum, int is_big, int shift) { if (*n_datum) (*n_datum)--; @@ -60,7 +60,7 @@ static uint16_t read10in32(const uint8_t **ptr, uint32_t * lbuf, *n_datum = 2; } - *lbuf = (*lbuf << 10) | (*lbuf >> 22); + *lbuf = (*lbuf << 10) | (*lbuf >> shift); return *lbuf & 0x3FF; } @@ -221,7 +221,7 @@ static int decode_frame(AVCodecContext *avctx, stride = avctx->width * elements; break; case 10: - if (!packing || packing > 1) { + if (!packing) { av_log(avctx, AV_LOG_ERROR, "Packing to 32bit required\n"); return -1; } @@ -360,17 +360,18 @@ static int decode_frame(AVCodecContext *avctx, (uint16_t*)ptr[1], (uint16_t*)ptr[2], (uint16_t*)ptr[3]}; + int shift = packing == 1 ? 22 : 20; for (y = 0; y < avctx->width; y++) { *dst[2]++ = read10in32(&buf, &rgbBuffer, - &n_datum, endian); + &n_datum, endian, shift); *dst[0]++ = read10in32(&buf, &rgbBuffer, - &n_datum, endian); + &n_datum, endian, shift); *dst[1]++ = read10in32(&buf, &rgbBuffer, - &n_datum, endian); + &n_datum, endian, shift); if (elements == 4) *dst[3]++ = read10in32(&buf, &rgbBuffer, - &n_datum, endian); + &n_datum, endian, shift); } n_datum = 0; for (i = 0; i < elements; i++) -- 1.7.10.4