[FFmpeg-devel] [PATCH] libopenjpegenc: add 9-15 bit RGB output

[Paul B Mahol]

On 1/23/13, Michael Bradshaw <mjbshaw at gmail.com> wrote:
> On Wed, Jan 23, 2013 at 6:53 AM, Paul B Mahol <onemda at gmail.com> wrote:
>
>> Patch is silly. It adds option to shift pixel values by some random
>> unlimited number.
>>
>
> It's not unlimited. The range [0, 7] is specified.
>
>
>> Even feature is far from being useful at all and it is not documented at
>> all.
>>
>
> I felt naming and documenting awkward for this feature, so I left it undone
> (rather than done poorly). Suggestions are welcome.
>
>
>> IMHO it is much better to support planar RGB stuff, you just need to
>> copy plane data
>> in right order. (Instead of manually deinterleaving (slow) packed RGB).
>>
>> So you should add support for planar rgb pixel formats in
>> decoder/encoder instead.
>
>
> Alright, attached is a version that does that (needs reviewing). Now people
> can decide which they prefer. One downside is that it doesn't support
> alpha, and there aren't any conversion in swscale (as far as I know) to
> allow people to go rgb48 -> bgrp10 etc. But it does seems like a more
> "proper" solution.

It is completly legit to support this pix fmts. So this patch should not depend
on questionable acceptability of previous patch.
And conversion can be added at any time.

>
> On a side note, why is the planar format bgr while the packed format
> (typically) rgb? Why wasn't the planar format made rgb as well?
>
> Thanks,
>
> Michael
>

> From 95a113e3031c2b5b2392e47921ab8f7d8e2b6e14 Mon Sep 17 00:00:00 2001
> From: Michael Bradshaw <mjbshaw at gmail.com>
> Date: Wed, 23 Jan 2013 10:44:33 -0700
> Subject: [PATCH] libopenjpegenc: add support for pix fmt gbrp(8-16 bit)
>
> Signed-off-by: Michael Bradshaw <mjbshaw at gmail.com>
> ---
>  libavcodec/libopenjpegenc.c |   28 +++++++++++++++++++++++++++-
>  1 file changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/libavcodec/libopenjpegenc.c b/libavcodec/libopenjpegenc.c
> index 13e8ef9..1e9d67a 100644
> --- a/libavcodec/libopenjpegenc.c
> +++ b/libavcodec/libopenjpegenc.c
> @@ -100,6 +100,12 @@ static opj_image_t *mj2_create_image(AVCodecContext *avctx, opj_cparameters_t *p
>      case AV_PIX_FMT_RGBA:
>      case AV_PIX_FMT_RGB48:
>      case AV_PIX_FMT_RGBA64:
> +    case AV_PIX_FMT_GBR24P:
> +    case AV_PIX_FMT_GBRP9:
> +    case AV_PIX_FMT_GBRP10:
> +    case AV_PIX_FMT_GBRP12:
> +    case AV_PIX_FMT_GBRP14:
> +    case AV_PIX_FMT_GBRP16:
>          color_space = CLRSPC_SRGB;
>          break;
>      case AV_PIX_FMT_YUV410P:
> @@ -351,6 +357,7 @@ static int libopenjpeg_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
>      opj_cio_t *stream;
>      int cpyresult = 0;
>      int ret, len;
> +    AVFrame bgrframe;
>
>      // x0, y0 is the top left corner of the image
>      // x1, y1 is the width, height of the reference grid
> @@ -369,6 +376,24 @@ static int libopenjpeg_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
>      case AV_PIX_FMT_RGBA64:
>          cpyresult = libopenjpeg_copy_packed16(avctx, frame, image);
>          break;
> +    case AV_PIX_FMT_GBR24P:
> +        bgrframe = *frame;
> +        bgrframe.data[0] = frame->data[2]; // swap to be rgb
> +        bgrframe.data[1] = frame->data[0];
> +        bgrframe.data[2] = frame->data[1];
> +        cpyresult = libopenjpeg_copy_unpacked8(avctx, &bgrframe, image);
> +        break;
> +    case AV_PIX_FMT_GBRP9:
> +    case AV_PIX_FMT_GBRP10:
> +    case AV_PIX_FMT_GBRP12:
> +    case AV_PIX_FMT_GBRP14:
> +    case AV_PIX_FMT_GBRP16:
> +        bgrframe = *frame;
> +        bgrframe.data[0] = frame->data[2]; // swap to be rgb
> +        bgrframe.data[1] = frame->data[0];
> +        bgrframe.data[2] = frame->data[1];
> +        cpyresult = libopenjpeg_copy_unpacked16(avctx, &bgrframe, image);

You could merge this two groups ...

> +        break;
>      case AV_PIX_FMT_GRAY8:
>      case AV_PIX_FMT_YUV410P:
>      case AV_PIX_FMT_YUV411P:
> @@ -378,7 +403,6 @@ static int libopenjpeg_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
>      case AV_PIX_FMT_YUV444P:
>      case AV_PIX_FMT_YUVA420P:
>      case AV_PIX_FMT_YUVA422P:
> -    case AV_PIX_FMT_YUVA444P:

stray change.

>          cpyresult = libopenjpeg_copy_unpacked8(avctx, frame, image);
>          break;
>      case AV_PIX_FMT_GRAY16:
> @@ -505,6 +529,8 @@ AVCodec ff_libopenjpeg_encoder = {
>      .capabilities   = 0,
>      .pix_fmts       = (const enum AVPixelFormat[]) {
>          AV_PIX_FMT_RGB24, AV_PIX_FMT_RGBA, AV_PIX_FMT_RGB48, 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_GRAY8A, AV_PIX_FMT_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,
> --
> 1.7.10.2 (Apple Git-33)
>

Missing decoder changes.