Message ID | 133b22e5-2849-4cde-8da7-8332b4ae8a77@mediaarea.net |
---|---|
State | Superseded |
Headers | show |
I just tested this patch non packed to 16-bit gbrp12le DPX from DaVinci Resolve. Encoding to FFV1 and back again to DPX produced the same framemd5 values. Does any more testing need to happen before this can be merged? Best, Kieran
2018-04-10 12:28 GMT+02:00, Kieran O Leary <kieran.o.leary@gmail.com>: > I just tested this patch non packed to 16-bit gbrp12le DPX from DaVinci > Resolve. Testing is good, apart from more brackets (and less comments) it would be better if Jerome sends his public keys to Michael and pushes the patch. > Encoding to FFV1 and back again to DPX produced the same > framemd5 values. (I believe we already discussed in public that testing FFmpeg's bugs with FFmpeg makes limited sense.) Carl Eugen
On 4/10/18, Kieran O Leary <kieran.o.leary@gmail.com> wrote: > I just tested this patch non packed to 16-bit gbrp12le DPX from DaVinci > Resolve. Encoding to FFV1 and back again to DPX produced the same framemd5 > values. > > Does any more testing need to happen before this can be merged? > > Best, > > Kieran > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > Does this patch decodes to gbrp12 pixel format?
2018-04-10 12:41 GMT+02:00, Paul B Mahol <onemda@gmail.com>: > On 4/10/18, Kieran O Leary <kieran.o.leary@gmail.com> wrote: >> I just tested this patch non packed to 16-bit gbrp12le DPX from DaVinci >> Resolve. Encoding to FFV1 and back again to DPX produced the same framemd5 >> values. >> >> Does any more testing need to happen before this can be merged? > Does this patch decodes to gbrp12 pixel format? Looking at this block of the patch, it appears so: > + *dst[2]++ = read12in32(&buf, &rgbBuffer, > + &n_datum, endian); > + *dst[0]++ = read12in32(&buf, &rgbBuffer, > + &n_datum, endian); > + *dst[1]++ = read12in32(&buf, &rgbBuffer, > + &n_datum, endian); (Above are "& 0xFFF") Carl Eugen
Hi On Tue, Apr 10, 2018 at 11:34 AM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > 2018-04-10 12:28 GMT+02:00, Kieran O Leary <kieran.o.leary@gmail.com>: >> I just tested this patch non packed to 16-bit gbrp12le DPX from DaVinci >> Resolve. > > Testing is good, apart from more brackets (and less comments) it would > be better if Jerome sends his public keys to Michael and pushes the patch. > >> Encoding to FFV1 and back again to DPX produced the same >> framemd5 values. > > (I believe we already discussed in public that testing FFmpeg's bugs > with FFmpeg makes limited sense.) > Yes, and at the risk of making a fool of myself, here's some slightly different testing sing the graphicsmagick dpx encoder: The following console outputs have 4 steps: 1. Use gm convert to decode 12-bit DPX to 12-bit DPX (padded to 16-bit DPX) 2. Use ffmpeg convert to decode 12-bit DPX to 12-bit DPX (padded to 16-bit DPX) 3. Use ffmpeg to decode the graphicsmagick file with the md5 muxer 4. Use ffmpeg to decode the ffmpeg file with the md5 muxer The same md5 values are produced in steps 3 and 4- not sure what it proves exactly, maybe that even if the ffmpeg decoder is buggy, at least it decodes the files produced by Jerome's patch and the existing GM decoder in the same way? $ gm convert -verbose 6e0edc3b-7d89-4710-8b9a-66c8c990f9dc_1_00094787.dpx gm_dpx_to_dpx.dpx 6e0edc3b-7d89-4710-8b9a-66c8c990f9dc_1_00094787.dpx DPX 1600x1168+0+0 DirectClass 12-bit 8.0Mi 0.030u 0m:0.050000s 6e0edc3b-7d89-4710-8b9a-66c8c990f9dc_1_00094787.dpx=>gm_dpx_to_dpx.dpx DPX 1600x1168+0+0 DirectClass 12-bit 10.7Mi 0.040u 0m:0.150000s (11.9Mi pixels/s) $ ./ffmpeg -i 6e0edc3b-7d89-4710-8b9a-66c8c990f9dc_1_00094787.dpx ffmpeg_dpx_to_dpx.dpx ffmpeg version N-90642-gd64183e Copyright (c) 2000-2018 the FFmpeg developers built with Apple LLVM version 8.0.0 (clang-800.0.42.1) configuration: libavutil 56. 13.100 / 56. 13.100 libavcodec 58. 17.100 / 58. 17.100 libavformat 58. 11.101 / 58. 11.101 libavdevice 58. 2.100 / 58. 2.100 libavfilter 7. 14.100 / 7. 14.100 libswscale 5. 0.102 / 5. 0.102 libswresample 3. 0.101 / 3. 0.101 [dpx_pipe @ 0x7f80db809000] Stream #0: not enough frames to estimate rate; consider increasing probesize Input #0, dpx_pipe, from '6e0edc3b-7d89-4710-8b9a-66c8c990f9dc_1_00094787.dpx': Duration: N/A, bitrate: N/A Stream #0:0: Video: dpx, gbrp12le, 1600x1168 [SAR 1:1 DAR 100:73], 24 tbr, 25 tbn, 24 tbc Stream mapping: Stream #0:0 -> #0:0 (dpx (native) -> dpx (native)) Press [q] to stop, [?] for help Output #0, image2, to 'ffmpeg_dpx_to_dpx.dpx': Metadata: encoder : Lavf58.11.101 Stream #0:0: Video: dpx, gbrp12le, 1600x1168 [SAR 1:1 DAR 100:73], q=2-31, 200 kb/s, 24 fps, 24 tbn, 24 tbc Metadata: encoder : Lavc58.17.100 dpx frame= 1 fps=0.0 q=-0.0 Lsize=N/A time=00:00:00.04 bitrate=N/A speed=0.286x video:10952kB audio:0kB subtitle:0kB other streams:0kB global headers:0kB muxing overhead: unknown $ ./ffmpeg -i ffmpeg_dpx_to_dpx.dpx -f md5 - ffmpeg version N-90642-gd64183e Copyright (c) 2000-2018 the FFmpeg developers built with Apple LLVM version 8.0.0 (clang-800.0.42.1) configuration: libavutil 56. 13.100 / 56. 13.100 libavcodec 58. 17.100 / 58. 17.100 libavformat 58. 11.101 / 58. 11.101 libavdevice 58. 2.100 / 58. 2.100 libavfilter 7. 14.100 / 7. 14.100 libswscale 5. 0.102 / 5. 0.102 libswresample 3. 0.101 / 3. 0.101 [dpx_pipe @ 0x7ff76b009000] Stream #0: not enough frames to estimate rate; consider increasing probesize Input #0, dpx_pipe, from 'ffmpeg_dpx_to_dpx.dpx': Duration: N/A, bitrate: N/A Stream #0:0: Video: dpx, gbrp12le, 1600x1168 [SAR 1:1 DAR 100:73], 25 tbr, 25 tbn, 25 tbc Stream mapping: Stream #0:0 -> #0:0 (dpx (native) -> rawvideo (native)) Press [q] to stop, [?] for help Output #0, md5, to 'pipe:': Metadata: encoder : Lavf58.11.101 Stream #0:0: Video: rawvideo (G3[0][12] / 0xC003347), gbrp12le, 1600x1168 [SAR 1:1 DAR 100:73], q=2-31, 1681920 kb/s, 25 fps, 25 tbn, 25 tbc Metadata: encoder : Lavc58.17.100 rawvideo MD5=aa43e3ec1c2a8e4499b3bc796a35185b frame= 1 fps=0.0 q=-0.0 Lsize= 0kB time=00:00:00.04 bitrate= 7.4kbits/s speed=1.25x video:10950kB audio:0kB subtitle:0kB other streams:0kB global headers:0kB muxing overhead: unknown $ ./ffmpeg -i gm_dpx_to_dpx.dpx -f md5 - ffmpeg version N-90642-gd64183e Copyright (c) 2000-2018 the FFmpeg developers built with Apple LLVM version 8.0.0 (clang-800.0.42.1) configuration: libavutil 56. 13.100 / 56. 13.100 libavcodec 58. 17.100 / 58. 17.100 libavformat 58. 11.101 / 58. 11.101 libavdevice 58. 2.100 / 58. 2.100 libavfilter 7. 14.100 / 7. 14.100 libswscale 5. 0.102 / 5. 0.102 libswresample 3. 0.101 / 3. 0.101 [dpx_pipe @ 0x7fe9d2809000] Stream #0: not enough frames to estimate rate; consider increasing probesize Input #0, dpx_pipe, from 'gm_dpx_to_dpx.dpx': Duration: N/A, bitrate: N/A Stream #0:0: Video: dpx, gbrp12le, 1600x1168 [SAR 1:1 DAR 100:73], 24 tbr, 25 tbn, 24 tbc Stream mapping: Stream #0:0 -> #0:0 (dpx (native) -> rawvideo (native)) Press [q] to stop, [?] for help Output #0, md5, to 'pipe:': Metadata: encoder : Lavf58.11.101 Stream #0:0: Video: rawvideo (G3[0][12] / 0xC003347), gbrp12le, 1600x1168 [SAR 1:1 DAR 100:73], q=2-31, 1614643 kb/s, 24 fps, 24 tbn, 24 tbc Metadata: encoder : Lavc58.17.100 rawvideo MD5=aa43e3ec1c2a8e4499b3bc796a35185b frame= 1 fps=0.0 q=-0.0 Lsize= 0kB time=00:00:00.04 bitrate= 7.1kbits/s speed=1.32x video:10950kB audio:0kB subtitle:0kB other streams:0kB global headers:0kB muxing overhead: unknown
Actually, here's another test that might be a bit more meaningful. This is using graphicksmagic and the identify/signature command in order to generate sha256 hashes for the image data only. It is producing identical hashes for: 1. The original 12-bit DPX from Da Vinci Resolve (not packed to 16-bit) 2. The graphicsmagick 12-bit DPX to 12-bit DPX (padded to 16-bit DPX) file 3. The ffmpeg convert 12-bit DPX to 12-bit DPX (padded to 16-bit DPX) which uses the ffmpeg decoder with Jerome's patch. $ gm identify -verbose 6e0edc3b-7d89-4710-8b9a-66c8c990f9dc_1_00094787.dpx |grep -i signature Signature: 3fb67059dca860c483b1deac912973b76d48fa9f0114d63e8116cff69d55bb4c $ gm identify -verbose gm_dpx_to_dpx.dpx |grep -i signature Signature: 3fb67059dca860c483b1deac912973b76d48fa9f0114d63e8116cff69d55bb4c $ gm identify -verbose ffmpeg_dpx_to_dpx.dpx |grep -i signature Signature: 3fb67059dca860c483b1deac912973b76d48fa9f0114d63e8116cff69d55bb4c Hopefully this is a bit better than just using ffmpeg? Best, Kieran.
On 4/10/18, Kieran O Leary <kieran.o.leary@gmail.com> wrote: > Actually, here's another test that might be a bit more meaningful. > This is using graphicksmagic and the identify/signature command in > order to generate sha256 hashes for the image data only. > > It is producing identical hashes for: > 1. The original 12-bit DPX from Da Vinci Resolve (not packed to 16-bit) > 2. The graphicsmagick 12-bit DPX to 12-bit DPX (padded to 16-bit DPX) file > 3. The ffmpeg convert 12-bit DPX to 12-bit DPX (padded to 16-bit DPX) > which uses the ffmpeg decoder with Jerome's patch. > > $ gm identify -verbose > 6e0edc3b-7d89-4710-8b9a-66c8c990f9dc_1_00094787.dpx |grep -i > signature > > Signature: > 3fb67059dca860c483b1deac912973b76d48fa9f0114d63e8116cff69d55bb4c > > $ gm identify -verbose gm_dpx_to_dpx.dpx |grep -i signature > > Signature: > 3fb67059dca860c483b1deac912973b76d48fa9f0114d63e8116cff69d55bb4c > > > $ gm identify -verbose ffmpeg_dpx_to_dpx.dpx |grep -i signature > > Signature: > 3fb67059dca860c483b1deac912973b76d48fa9f0114d63e8116cff69d55bb4c > > Hopefully this is a bit better than just using ffmpeg? No, you must not use tragic software ever. Besides ffmpeg also can give you checksums for image only. See framehash muxer, and it can give you sha512 hash.
On Tue, Apr 10, 2018 at 3:16 PM, Paul B Mahol <onemda@gmail.com> wrote: > On 4/10/18, Kieran O Leary <kieran.o.leary@gmail.com> wrote: >> >> Hopefully this is a bit better than just using ffmpeg? > > No, you must not use tragic software ever. > > Besides ffmpeg also can give you checksums for image only. > See framehash muxer, and it can give you sha512 hash. Yes, I've used framehash but we use framemd5 internally in the Irish Film Institute. I was just using another DPX decoder as Carl is sceptical about using framemd5, or ffmpeg in general to determine losslessness of files created by ffmpeg. This was one of the only times I've ever used Image/GraphicsMagick to be honest.
diff --git a/libavcodec/dpx.c b/libavcodec/dpx.c index 1aa2cbd1c8..7e2a6fb779 100644 --- a/libavcodec/dpx.c +++ b/libavcodec/dpx.c @@ -65,6 +65,38 @@ static uint16_t read10in32(const uint8_t **ptr, uint32_t * lbuf, return *lbuf & 0x3FF; } +static uint16_t read12in32(const uint8_t **ptr, uint32_t * lbuf, + int * n_datum, int is_big) +{ + if (*n_datum) + (*n_datum)--; + else { + *lbuf = read32(ptr, is_big); + *n_datum = 7; + } + + switch (*n_datum){ + case 7: return *lbuf & 0xFFF; + case 6: return (*lbuf >> 12) & 0xFFF; + case 5: { + uint32_t c = *lbuf >> 24; + *lbuf = read32(ptr, is_big); + c |= *lbuf << 8; + return c & 0xFFF; + } + case 4: return (*lbuf >> 4) & 0xFFF; + case 3: return (*lbuf >> 16) & 0xFFF; + case 2: { + uint32_t c = *lbuf >> 28; + *lbuf = read32(ptr, is_big); + c |= *lbuf << 4; + return c & 0xFFF; + } + case 1: return (*lbuf >> 8) & 0xFFF; + default: return *lbuf >> 20; + } +} + static int decode_frame(AVCodecContext *avctx, void *data, int *got_frame, @@ -201,10 +233,29 @@ static int decode_frame(AVCodecContext *avctx, break; case 12: if (!packing) { - av_log(avctx, AV_LOG_ERROR, "Packing to 16bit required\n"); - return -1; + int tested = 0; + if (endian && descriptor == 50 && (avctx->width%8) == 0) // Little endian and widths not a multiple of 8 need tests + tested = 1; + //if (endian && descriptor == 51 && (avctx->width%2) == 0) // Little endian and widths not a multiple of 2 need tests + // tested = 1; + if (!tested) { + av_log(avctx, AV_LOG_ERROR, "Packing to 16bit required\n"); + return -1; + } + } + stride = avctx->width * elements; + if (packing) + stride *= 2; + else { + stride *= 3; // 12 bits are 1.5 byte so multiplied by 3 then divided by 2 + if (stride % 8) { + // Align to 32-bit boundaries (not tested) + stride /= 8; + stride++; + stride *= 8; + } + stride /= 2; } - stride = 2 * avctx->width * elements; break; case 16: stride = 2 * avctx->width * elements; @@ -349,6 +400,7 @@ static int decode_frame(AVCodecContext *avctx, (uint16_t*)ptr[2], (uint16_t*)ptr[3]}; for (y = 0; y < avctx->width; y++) { + if (packing) { if (elements >= 3) *dst[2]++ = read16(&buf, endian) >> 4; *dst[0] = read16(&buf, endian) >> 4; @@ -357,6 +409,17 @@ static int decode_frame(AVCodecContext *avctx, *dst[1]++ = read16(&buf, endian) >> 4; if (elements == 4) *dst[3]++ = read16(&buf, endian) >> 4; + } else { + *dst[2]++ = read12in32(&buf, &rgbBuffer, + &n_datum, endian); + *dst[0]++ = read12in32(&buf, &rgbBuffer, + &n_datum, endian); + *dst[1]++ = read12in32(&buf, &rgbBuffer, + &n_datum, endian); + if (elements == 4) + *dst[3]++ = read12in32(&buf, &rgbBuffer, + &n_datum, endian); + } } for (i = 0; i < elements; i++) ptr[i] += p->linesize[i];