diff mbox

[FFmpeg-devel] lavc/libopenjpeg: Support GRAY10, GRAY12 and GRAY14

Message ID CAB0OVGq6V_EgBz2OUd66ZMXM_er=JpM6gf4wgwOaR3rKGMw0LQ@mail.gmail.com
State Accepted
Headers show

Commit Message

Carl Eugen Hoyos June 21, 2018, 10:12 a.m. UTC 
Hi!

Attached patch allows to create (and read) gray1x samples with libopenjpeg.

Please comment, Carl Eugen

Comments

Michael Bradshaw June 22, 2018, 1:34 p.m. UTC | #1 
Hey, Carl Eugen! Thanks for the patch.

On Thu, Jun 21, 2018 at 3:12 AM, Carl Eugen Hoyos <ceffmpeg@gmail.com>
wrote:
>
> Attached patch allows to create (and read) gray1x samples with libopenjpeg.


Looks good to me. I have a few very small nits below. I won't block the
patch based on these small nits, so feel free to submit without fixing them
if you don't have the time/patience.

> From 25c4a1ea0afae9fb3561fd928552133ddcf70d05 Mon Sep 17 00:00:00 2001
> From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
> Date: Thu, 21 Jun 2018 12:05:40 +0200
> Subject: [PATCH] lavc/libopenjpeg: Support GRAY10, GRAY12 and GRAY14.
>
> ---
>  libavcodec/libopenjpegdec.c |    1 +
>  libavcodec/libopenjpegenc.c |    7 +++++++
>  2 files changed, 8 insertions(+)
>
> diff --git a/libavcodec/libopenjpegdec.c b/libavcodec/libopenjpegdec.c
> index 5e66cd9..344c5ba 100644
> --- a/libavcodec/libopenjpegdec.c
> +++ b/libavcodec/libopenjpegdec.c
> @@ -45,6 +45,7 @@
>                             AV_PIX_FMT_RGB48, AV_PIX_FMT_RGBA64
>
>  #define GRAY_PIXEL_FORMATS AV_PIX_FMT_GRAY8, AV_PIX_FMT_YA8,
      \
> +                           AV_PIX_FMT_GRAY10, AV_PIX_FMT_GRAY12,
AV_PIX_FMT_GRAY14, \
>                             AV_PIX_FMT_GRAY16, AV_PIX_FMT_YA16
>
>  #define YUV_PIXEL_FORMATS  AV_PIX_FMT_YUV410P, AV_PIX_FMT_YUV411P,
AV_PIX_FMT_YUVA420P, \
> diff --git a/libavcodec/libopenjpegenc.c b/libavcodec/libopenjpegenc.c
> index 7c7d0aa..8627d02 100644
> --- a/libavcodec/libopenjpegenc.c
> +++ b/libavcodec/libopenjpegenc.c
> @@ -191,6 +191,9 @@ static opj_image_t *mj2_create_image(AVCodecContext
*avctx, opj_cparameters_t *p
>      case AV_PIX_FMT_YA8:
>      case AV_PIX_FMT_GRAY16:
>      case AV_PIX_FMT_YA16:
> +    case AV_PIX_FMT_GRAY10:
> +    case AV_PIX_FMT_GRAY12:
> +    case AV_PIX_FMT_GRAY14:

nit: It would be nice if these new GRAY enums were grouped with the
previous GRAY16 case.

>          color_space = OPJ_CLRSPC_GRAY;
>          break;
>      case AV_PIX_FMT_RGB24:
> @@ -613,6 +616,9 @@ static int libopenjpeg_encode_frame(AVCodecContext
*avctx, AVPacket *pkt,
>          cpyresult = libopenjpeg_copy_unpacked8(avctx, frame, image);
>          break;
>      case AV_PIX_FMT_GRAY16:
> +    case AV_PIX_FMT_GRAY14:
> +    case AV_PIX_FMT_GRAY12:
> +    case AV_PIX_FMT_GRAY10:

nit: I don't have a strong preference on ordering, but it would be nice if
it were consistent. Above you have 10 -> 12 -> 14 bit ordering, whereas
here you have 14 -> 12 -> 10.

>      case AV_PIX_FMT_YUV420P9:
>      case AV_PIX_FMT_YUV422P9:
>      case AV_PIX_FMT_YUV444P9:
> @@ -763,6 +769,7 @@ AVCodec ff_libopenjpeg_encoder = {
>          AV_PIX_FMT_RGBA64, AV_PIX_FMT_GBR24P,
>          AV_PIX_FMT_GBRP9, AV_PIX_FMT_GBRP10, AV_PIX_FMT_GBRP12,
AV_PIX_FMT_GBRP14, AV_PIX_FMT_GBRP16,
>          AV_PIX_FMT_GRAY8, AV_PIX_FMT_YA8, AV_PIX_FMT_GRAY16,
AV_PIX_FMT_YA16,
> +        AV_PIX_FMT_GRAY10, AV_PIX_FMT_GRAY12, AV_PIX_FMT_GRAY14,

nit: I know this slightly inflates the diff, but I think it would be nice
to group the 10, 12, and 14 bit GRAY enums together with GRAY16.

>          AV_PIX_FMT_YUV420P, AV_PIX_FMT_YUV422P, AV_PIX_FMT_YUVA420P,
>          AV_PIX_FMT_YUV440P, AV_PIX_FMT_YUV444P, AV_PIX_FMT_YUVA422P,
>          AV_PIX_FMT_YUV411P, AV_PIX_FMT_YUV410P, AV_PIX_FMT_YUVA444P,
> --
> 1.7.10.4
Carl Eugen Hoyos June 25, 2018, 10:25 p.m. UTC | #2 
2018-06-22 15:34 GMT+02:00, Michael Bradshaw <mjbshaw@gmail.com>:
> Hey, Carl Eugen! Thanks for the patch.
>
> On Thu, Jun 21, 2018 at 3:12 AM, Carl Eugen Hoyos <ceffmpeg@gmail.com>
> wrote:
>>
>> Attached patch allows to create (and read) gray1x samples with
>> libopenjpeg.
>
>
> Looks good to me. I have a few very small nits below. I won't block the
> patch based on these small nits, so feel free to submit without fixing them
> if you don't have the time/patience.
>
>> From 25c4a1ea0afae9fb3561fd928552133ddcf70d05 Mon Sep 17 00:00:00 2001
>> From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
>> Date: Thu, 21 Jun 2018 12:05:40 +0200
>> Subject: [PATCH] lavc/libopenjpeg: Support GRAY10, GRAY12 and GRAY14.
>>
>> ---
>>  libavcodec/libopenjpegdec.c |    1 +
>>  libavcodec/libopenjpegenc.c |    7 +++++++
>>  2 files changed, 8 insertions(+)
>>
>> diff --git a/libavcodec/libopenjpegdec.c b/libavcodec/libopenjpegdec.c
>> index 5e66cd9..344c5ba 100644
>> --- a/libavcodec/libopenjpegdec.c
>> +++ b/libavcodec/libopenjpegdec.c
>> @@ -45,6 +45,7 @@
>>                             AV_PIX_FMT_RGB48, AV_PIX_FMT_RGBA64
>>
>>  #define GRAY_PIXEL_FORMATS AV_PIX_FMT_GRAY8, AV_PIX_FMT_YA8,
>       \
>> +                           AV_PIX_FMT_GRAY10, AV_PIX_FMT_GRAY12,
> AV_PIX_FMT_GRAY14, \
>>                             AV_PIX_FMT_GRAY16, AV_PIX_FMT_YA16
>>
>>  #define YUV_PIXEL_FORMATS  AV_PIX_FMT_YUV410P, AV_PIX_FMT_YUV411P,
> AV_PIX_FMT_YUVA420P, \
>> diff --git a/libavcodec/libopenjpegenc.c b/libavcodec/libopenjpegenc.c
>> index 7c7d0aa..8627d02 100644
>> --- a/libavcodec/libopenjpegenc.c
>> +++ b/libavcodec/libopenjpegenc.c
>> @@ -191,6 +191,9 @@ static opj_image_t *mj2_create_image(AVCodecContext
> *avctx, opj_cparameters_t *p
>>      case AV_PIX_FMT_YA8:
>>      case AV_PIX_FMT_GRAY16:
>>      case AV_PIX_FMT_YA16:
>> +    case AV_PIX_FMT_GRAY10:
>> +    case AV_PIX_FMT_GRAY12:
>> +    case AV_PIX_FMT_GRAY14:
>
> nit: It would be nice if these new GRAY enums were grouped with the
> previous GRAY16 case.

Done.

>>          color_space = OPJ_CLRSPC_GRAY;
>>          break;
>>      case AV_PIX_FMT_RGB24:
>> @@ -613,6 +616,9 @@ static int libopenjpeg_encode_frame(AVCodecContext
> *avctx, AVPacket *pkt,
>>          cpyresult = libopenjpeg_copy_unpacked8(avctx, frame, image);
>>          break;
>>      case AV_PIX_FMT_GRAY16:
>> +    case AV_PIX_FMT_GRAY14:
>> +    case AV_PIX_FMT_GRAY12:
>> +    case AV_PIX_FMT_GRAY10:
>
> nit: I don't have a strong preference on ordering, but it would be nice if
> it were consistent. Above you have 10 -> 12 -> 14 bit ordering, whereas
> here you have 14 -> 12 -> 10.

Done.

>>      case AV_PIX_FMT_YUV420P9:
>>      case AV_PIX_FMT_YUV422P9:
>>      case AV_PIX_FMT_YUV444P9:
>> @@ -763,6 +769,7 @@ AVCodec ff_libopenjpeg_encoder = {
>>          AV_PIX_FMT_RGBA64, AV_PIX_FMT_GBR24P,
>>          AV_PIX_FMT_GBRP9, AV_PIX_FMT_GBRP10, AV_PIX_FMT_GBRP12,
> AV_PIX_FMT_GBRP14, AV_PIX_FMT_GBRP16,
>>          AV_PIX_FMT_GRAY8, AV_PIX_FMT_YA8, AV_PIX_FMT_GRAY16,
> AV_PIX_FMT_YA16,
>> +        AV_PIX_FMT_GRAY10, AV_PIX_FMT_GRAY12, AV_PIX_FMT_GRAY14,
>
> nit: I know this slightly inflates the diff, but I think it would be nice
> to group the 10, 12, and 14 bit GRAY enums together with GRAY16.

I prefer the smaller diff, sorry...

Patch applied, Carl Eugen
diff mbox

Patch

From 25c4a1ea0afae9fb3561fd928552133ddcf70d05 Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
Date: Thu, 21 Jun 2018 12:05:40 +0200
Subject: [PATCH] lavc/libopenjpeg: Support GRAY10, GRAY12 and GRAY14.

---
 libavcodec/libopenjpegdec.c |    1 +
 libavcodec/libopenjpegenc.c |    7 +++++++
 2 files changed, 8 insertions(+)

diff --git a/libavcodec/libopenjpegdec.c b/libavcodec/libopenjpegdec.c
index 5e66cd9..344c5ba 100644
--- a/libavcodec/libopenjpegdec.c
+++ b/libavcodec/libopenjpegdec.c
@@ -45,6 +45,7 @@ 
                            AV_PIX_FMT_RGB48, AV_PIX_FMT_RGBA64
 
 #define GRAY_PIXEL_FORMATS AV_PIX_FMT_GRAY8, AV_PIX_FMT_YA8,                  \
+                           AV_PIX_FMT_GRAY10, AV_PIX_FMT_GRAY12, AV_PIX_FMT_GRAY14, \
                            AV_PIX_FMT_GRAY16, AV_PIX_FMT_YA16
 
 #define YUV_PIXEL_FORMATS  AV_PIX_FMT_YUV410P, AV_PIX_FMT_YUV411P, AV_PIX_FMT_YUVA420P, \
diff --git a/libavcodec/libopenjpegenc.c b/libavcodec/libopenjpegenc.c
index 7c7d0aa..8627d02 100644
--- a/libavcodec/libopenjpegenc.c
+++ b/libavcodec/libopenjpegenc.c
@@ -191,6 +191,9 @@  static opj_image_t *mj2_create_image(AVCodecContext *avctx, opj_cparameters_t *p
     case AV_PIX_FMT_YA8:
     case AV_PIX_FMT_GRAY16:
     case AV_PIX_FMT_YA16:
+    case AV_PIX_FMT_GRAY10:
+    case AV_PIX_FMT_GRAY12:
+    case AV_PIX_FMT_GRAY14:
         color_space = OPJ_CLRSPC_GRAY;
         break;
     case AV_PIX_FMT_RGB24:
@@ -613,6 +616,9 @@  static int libopenjpeg_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
         cpyresult = libopenjpeg_copy_unpacked8(avctx, frame, image);
         break;
     case AV_PIX_FMT_GRAY16:
+    case AV_PIX_FMT_GRAY14:
+    case AV_PIX_FMT_GRAY12:
+    case AV_PIX_FMT_GRAY10:
     case AV_PIX_FMT_YUV420P9:
     case AV_PIX_FMT_YUV422P9:
     case AV_PIX_FMT_YUV444P9:
@@ -763,6 +769,7 @@  AVCodec ff_libopenjpeg_encoder = {
         AV_PIX_FMT_RGBA64, AV_PIX_FMT_GBR24P,
         AV_PIX_FMT_GBRP9, AV_PIX_FMT_GBRP10, AV_PIX_FMT_GBRP12, AV_PIX_FMT_GBRP14, AV_PIX_FMT_GBRP16,
         AV_PIX_FMT_GRAY8, AV_PIX_FMT_YA8, AV_PIX_FMT_GRAY16, AV_PIX_FMT_YA16,
+        AV_PIX_FMT_GRAY10, AV_PIX_FMT_GRAY12, AV_PIX_FMT_GRAY14,
         AV_PIX_FMT_YUV420P, AV_PIX_FMT_YUV422P, AV_PIX_FMT_YUVA420P,
         AV_PIX_FMT_YUV440P, AV_PIX_FMT_YUV444P, AV_PIX_FMT_YUVA422P,
         AV_PIX_FMT_YUV411P, AV_PIX_FMT_YUV410P, AV_PIX_FMT_YUVA444P,
-- 
1.7.10.4