Re: [FFmpeg-devel] [PATCH] avcodec/dpx: Support for RGB 12-bit packed decoding

2018-04-11 Thread Michael Niedermayer 
On Wed, Apr 11, 2018 at 11:44:47AM +0200, Jerome Martinez wrote:
> On 11/04/2018 00:16, Lou Logan wrote:
> >On Tue, Apr 10, 2018, at 2:05 PM, Michael Niedermayer wrote:
> >>what do you mean by "Git and me are not good friends" ?
> >>If git hates you and sometimes does things that you didnt intend at all then
> >>that would be a problem with direct pushes as theres no way to undo.
> >>But maybe i misunderstand.
> 
> 
> Yes, that's right :), I think you also remarked that I am an expert of
> careless mistakes :(.
> I am still at the bottom of the Git learning curve, being more used to the
> GitHub PR things (which IMO avoids well to do a mess with upstream repo).
> Additionally, I think that it does not worth it to take risks (including the
> risk of compromised machine) about giving me write rights with the only few
> patches I send. It would be another story if I become more active on FFmpeg
> code in the future.
> 
> >>
> >>Also to get git write access, post a patch that adds yourself to the
> >>MAINTAINERs file. When noone objects then ill add your key and apply
> >>the MAINTAINER patch.
> >Also, if you're uncomfortable pushing to the repository that's fine too. It 
> >is not a problem, but you may have to prod and remind us, perhaps a few 
> >times (or more), to push stuff.
> 
> I kindly request a maintainer to push one (the latest one?) of the DPX

will apply

thx

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Those who are too smart to engage in politics are punished by being
governed by those who are dumber. -- Plato 


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/dpx: Support for RGB 12-bit packed decoding

2018-04-11 Thread Jerome Martinez 

On 11/04/2018 00:16, Lou Logan wrote:

On Tue, Apr 10, 2018, at 2:05 PM, Michael Niedermayer wrote:

what do you mean by "Git and me are not good friends" ?
If git hates you and sometimes does things that you didnt intend at all then
that would be a problem with direct pushes as theres no way to undo.
But maybe i misunderstand.



Yes, that's right :), I think you also remarked that I am an expert of 
careless mistakes :(.
I am still at the bottom of the Git learning curve, being more used to 
the GitHub PR things (which IMO avoids well to do a mess with upstream 
repo).
Additionally, I think that it does not worth it to take risks (including 
the risk of compromised machine) about giving me write rights with the 
only few patches I send. It would be another story if I become more 
active on FFmpeg code in the future.




Also to get git write access, post a patch that adds yourself to the
MAINTAINERs file. When noone objects then ill add your key and apply
the MAINTAINER patch.

Also, if you're uncomfortable pushing to the repository that's fine too. It is 
not a problem, but you may have to prod and remind us, perhaps a few times (or 
more), to push stuff.


I kindly request a maintainer to push one (the latest one?) of the DPX 
patches in this thread (i.e. right I am uncomfortable pushing to FFmpeg 
repo directly, I would do it only if it is the only method for having my 
patches pushed upstream after review due to no current maintainer 
willing to push them), as well as reviewing then push (if they are fine) 
the ones related to FFV1 [1]


Jérôme

[1] https://ffmpeg.org/pipermail/ffmpeg-devel/2018-March/226192.html
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/dpx: Support for RGB 12-bit packed decoding

2018-04-10 Thread Lou Logan 
On Tue, Apr 10, 2018, at 2:05 PM, Michael Niedermayer wrote:
>
> what do you mean by "Git and me are not good friends" ?
> If git hates you and sometimes does things that you didnt intend at all then
> that would be a problem with direct pushes as theres no way to undo.
> But maybe i misunderstand.
> 
> Also to get git write access, post a patch that adds yourself to the
> MAINTAINERs file. When noone objects then ill add your key and apply
> the MAINTAINER patch.

Also, if you're uncomfortable pushing to the repository that's fine too. It is 
not a problem, but you may have to prod and remind us, perhaps a few times (or 
more), to push stuff.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/dpx: Support for RGB 12-bit packed decoding

2018-04-10 Thread Michael Niedermayer 
On Tue, Apr 10, 2018 at 09:41:05PM +0200, Jerome Martinez wrote:
> On 10/04/2018 12:34, Carl Eugen Hoyos wrote:
> >2018-04-10 12:28 GMT+02:00, Kieran O Leary :
> >>I just tested this patch non packed to 16-bit gbrp12le DPX from DaVinci
> >>Resolve.
> >Testing is good, apart
> 
> I thought the patch was "technically" OK, as I answered to all change
> requests and there was no additional feedback IIRC.
> 
> >  from more brackets
> 
> Not sure I understand, as the only "missing" brackets I see are for the 1
> line code after a "if", and I see that 1 line code has no brackets in other
> parts of the file.
> Anyway, I added more brackets, except for "if (*n_datum) (*n_datum)--;" as I
> copied/pasted it from another part of the file.
> Did I miss something else?
> 
> >  (and less comments)
> 
> I thought it would be better for someone willing to add alpha support in the
> future, as the alpha support was tested and "just" rejected for the moment.
> Anyway, I removed the commented code.
> 
> Modified patch attached.
> Note that I personally prefer to use the previous patch (or this patch
> without the additional brackets).
> 
> >  it would
> >be better if Jerome sends his public keys to Michael and pushes the patch.
> 
> If it is the only solution for having the patch pushed, I'll do that, even
> if I am not convinced that I deserve for the moment write rights on FFmpeg
> repository (especially because Git and me are not good friends :) ).

what do you mean by "Git and me are not good friends" ?
If git hates you and sometimes does things that you didnt intend at all then
that would be a problem with direct pushes as theres no way to undo.
But maybe i misunderstand.

Also to get git write access, post a patch that adds yourself to the
MAINTAINERs file. When noone objects then ill add your key and apply
the MAINTAINER patch.

thanks

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No snowflake in an avalanche ever feels responsible. -- Voltaire


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/dpx: Support for RGB 12-bit packed decoding

2018-04-10 Thread Jerome Martinez 

On 10/04/2018 12:34, Carl Eugen Hoyos wrote:

2018-04-10 12:28 GMT+02:00, Kieran O Leary :

I just tested this patch non packed to 16-bit gbrp12le DPX from DaVinci
Resolve.

Testing is good, apart


I thought the patch was "technically" OK, as I answered to all change 
requests and there was no additional feedback IIRC.



  from more brackets


Not sure I understand, as the only "missing" brackets I see are for the 
1 line code after a "if", and I see that 1 line code has no brackets in 
other parts of the file.
Anyway, I added more brackets, except for "if (*n_datum) (*n_datum)--;" 
as I copied/pasted it from another part of the file.

Did I miss something else?


  (and less comments)


I thought it would be better for someone willing to add alpha support in 
the future, as the alpha support was tested and "just" rejected for the 
moment.

Anyway, I removed the commented code.

Modified patch attached.
Note that I personally prefer to use the previous patch (or this patch 
without the additional brackets).



  it would
be better if Jerome sends his public keys to Michael and pushes the patch.


If it is the only solution for having the patch pushed, I'll do that, 
even if I am not convinced that I deserve for the moment write rights on 
FFmpeg repository (especially because Git and me are not good friends :) ).


Jérôme
From 02286a07f920b8d15bf5f031a21fc9080fc4fa32 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Martinez?= 
Date: Tue, 10 Apr 2018 18:20:23 +0200
Subject: [PATCH] avcodec/dpx: Support for RGB 12-bit packed decoding

Limited to widths multiple of 8 (RGB) due to lack of test files for such corner 
case

This partially fixes ticket #5639
---
 libavcodec/dpx.c | 67 +---
 1 file changed, 64 insertions(+), 3 deletions(-)

diff --git a/libavcodec/dpx.c b/libavcodec/dpx.c
index 1aa2cbd1c8..026fb10e90 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,27 @@ 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 (descriptor == 50 && endian && (avctx->width%8) == 0) { // 
Little endian and widths not a multiple of 8 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;
+if (stride % 8) {
+stride /= 8;
+stride++;
+stride *= 8;
+}
+stride /= 2;
 }
-stride = 2 * avctx->width * elements;
 break;
 case 16:
 stride = 2 * avctx->width * elements;
@@ -349,6 +398,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(, endian) >> 4;
 *dst[0] = read16(, endian) >> 4;
@@ -357,6 +407,17 @@ static int decode_frame(AVCodecContext *avctx,
 *dst[1]++ = read16(, endian) >> 4;
 if (elements == 4)
 *dst[3]++ = read16(, endian) >> 4;
+} else {
+*dst[2]++ = read12in32(, ,
+   _datum, endian);
+*dst[0]++ = read12in32(, ,
+   _datum, endian);
+

Re: [FFmpeg-devel] [PATCH] avcodec/dpx: Support for RGB 12-bit packed decoding

2018-04-10 Thread Kieran O Leary 
On Tue, Apr 10, 2018 at 3:16 PM, Paul B Mahol  wrote:
> On 4/10/18, Kieran O Leary  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.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/dpx: Support for RGB 12-bit packed decoding

2018-04-10 Thread Paul B Mahol 
On 4/10/18, Kieran O Leary  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.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/dpx: Support for RGB 12-bit packed decoding

2018-04-10 Thread Kieran O Leary 
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.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/dpx: Support for RGB 12-bit packed decoding

2018-04-10 Thread Kieran O Leary 
Hi

On Tue, Apr 10, 2018 at 11:34 AM, Carl Eugen Hoyos  wrote:
> 2018-04-10 12:28 GMT+02:00, Kieran O Leary :
>> 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.05s

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.15s
(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

  libavformat58. 11.101 / 58. 11.101

  libavdevice58.  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

  libavformat58. 11.101 / 58. 11.101

  libavdevice58.  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

  libavformat58. 11.101 / 58. 11.101

  libavdevice58.  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

Re: [FFmpeg-devel] [PATCH] avcodec/dpx: Support for RGB 12-bit packed decoding

2018-04-10 Thread Carl Eugen Hoyos 
2018-04-10 12:41 GMT+02:00, Paul B Mahol :
> On 4/10/18, Kieran O Leary  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(, ,
> +   _datum, endian);
> +*dst[0]++ = read12in32(, ,
> +   _datum, endian);
> +*dst[1]++ = read12in32(, ,
> +   _datum, endian);

(Above are "& 0xFFF")

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/dpx: Support for RGB 12-bit packed decoding

2018-04-10 Thread Paul B Mahol 
On 4/10/18, Kieran O Leary  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?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/dpx: Support for RGB 12-bit packed decoding

2018-04-10 Thread Carl Eugen Hoyos 
2018-04-10 12:28 GMT+02:00, Kieran O Leary :
> 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
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/dpx: Support for RGB 12-bit packed decoding

2018-04-10 Thread Kieran O Leary 
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

[FFmpeg-devel] [PATCH] avcodec/dpx: Support for RGB 12-bit packed decoding

2018-02-14 Thread Jerome Martinez 

On 08/02/2018 11:28, Jerome Martinez wrote:
Currently RGB and RGBA 12-bit are supported by DPX decoder only if 
component values are padded (packing "Filled to 32-bit words, method A").
This patch adds decoding of RGB and RGBA 12-bit with no padding 
(packing "Packed into 32-bit words").


As I have no file with line boundaries not aligned on 32-bit, I can 
not have good tests about the stride computing (so code about non 
aligned boundaries is theory) so I preferred to limit risks by 
decoding only if line boundaries are aligned on 32-bit words:
- 8 pixels for RGB (8 pixels x 3 components x 12 bits = 288 bits = 9 x 
32-bit words)
- 2 pixels for RGBA (2 pixels x 4 components x 12 bits = 3 x 32-bit 
words)


I think Little Endian parsing is fine thanks to the generic code about 
Big vs Little endian but I have no Little Endian test file so I also 
limited the decoding to Big Endian files.


Would be happy to check with cases I was not able to check if someone 
provides files.


I kept "Packing to 16bit required\n" message but interested in any 
suggestion about a better wording due to exceptions.


My test files:
https://github.com/MediaArea/RAWcooked-RegressionTestingFiles/tree/master/Formats/DPX/Flavors/RGB_12_Packed_BE 



Regression tests done on 12-bit content "Filled to 32-bit words, 
method A" in:

https://samples.ffmpeg.org/image-samples/dpx_samples.zip


Looks like the previous version of the patch is not included due to the 
debate about RGBA.
Please consider to include this modified patch, which has the RGBA part 
commented (I could remove the commented lines if you prefer).
From 28316686fed140279494d8c94e4f6bb5707e9ac0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Martinez?= 
Date: Thu, 8 Feb 2018 09:22:08 +0100
Subject: [PATCH] avcodec/dpx: Support for RGB 12-bit packed decoding

Limited to widths multiple of 8 (RGB) due to lack of test files for such corner 
case

This partially fixes ticket #5639
---
 libavcodec/dpx.c | 69 +---
 1 file changed, 66 insertions(+), 3 deletions(-)

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(, endian) >> 4;