Message ID | CAB0OVGq6V_EgBz2OUd66ZMXM_er=JpM6gf4wgwOaR3rKGMw0LQ@mail.gmail.com |
---|---|
State | Accepted |
Headers | show |
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
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
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