[FFmpeg-devel] [PATCH] J2K encoder: fix bpp and add support for 9, 10, and 16 bit YUV

[Michael Bradshaw]

On Mon, Nov 28, 2011 at 2:20 PM, Jean First <jeanfirst at gmail.com> wrote:

> On Mon Nov 28 2011 21:33:39 GMT+0100 (CET), Michael Bradshaw wrote:
>
>> The other patch is for encoding 9, 10, and 16 bit YUV video.  One
>> thing: right now I'm using PIX_FMT_YUV420P9 and the like, rather than
>> the specific big endian and little endian versions.  If this is a
>> problem, or if it would be better to support both big and little
>> endian formats (I don't know what the industry "standard" is), let me
>> know and I can add support.  It's pretty slow at encoding, just so you
>> know.
>>
>
>  From f3b8d60ecbb745fe3d5c614fa3e599**03459a0e85 Mon Sep 17 00:00:00 2001
>> From: Michael Bradshaw <mbradshaw at sorensonmedia.com>
>> Date: Mon, 28 Nov 2011 13:19:55 -0700
>> Subject: [PATCH 103/103] Added support for encoding 9, 10, and 16 bit YUV
>> J2K
>>  video
>>
>>
>> Signed-off-by: Michael Bradshaw <mbradshaw at sorensonmedia.com>
>> ---
>>  libavcodec/libopenjpegenc.c |   67 ++++++++++++++++++++++++++++++**
>> +++++++++++--
>>  1 files changed, 64 insertions(+), 3 deletions(-)
>>
>> diff --git a/libavcodec/libopenjpegenc.c b/libavcodec/libopenjpegenc.c
>> index 8403ca1..dffd6bf 100644
>> --- a/libavcodec/libopenjpegenc.c
>> +++ b/libavcodec/libopenjpegenc.c
>> @@ -93,6 +93,27 @@ static opj_image_t *mj2_create_image(**AVCodecContext
>> *avctx, opj_cparameters_t *p
>>         color_space = CLRSPC_SYCC;
>>         numcomps = 3;
>>         break;
>> +    case PIX_FMT_YUV420P9:
>> +    case PIX_FMT_YUV422P9:
>> +    case PIX_FMT_YUV444P9:
>> +        color_space = CLRSPC_SYCC;
>> +        numcomps = 3;
>> +        bpp = 9;
>> +        break;
>> +    case PIX_FMT_YUV420P10:
>> +    case PIX_FMT_YUV422P10:
>> +    case PIX_FMT_YUV444P10:
>> +    color_space = CLRSPC_SYCC;
>>
>
> indent.


Fixed.  Good catch.

 +        numcomps = 3;
>> +        bpp = 10;
>> +        break;
>> +    case PIX_FMT_YUV420P16:
>> +    case PIX_FMT_YUV422P16:
>> +    case PIX_FMT_YUV444P16:
>> +        color_space = CLRSPC_SYCC;
>> +        numcomps = 3;
>> +        bpp = 16;
>> +        break;
>>     default:
>>         av_log(avctx, AV_LOG_ERROR, "The requested pixel format '%s' is
>> not supported\n", av_get_pix_fmt_name(avctx->**pix_fmt));
>>         return NULL;
>> @@ -182,7 +203,7 @@ static int libopenjpeg_copy_rgba(**AVCodecContext
>> *avctx, AVFrame *frame, opj_imag
>>     return 1;
>>  }
>>
>> -static int libopenjpeg_copy_yuv(**AVCodecContext *avctx, AVFrame
>> *frame, opj_image_t *image)
>> +static int libopenjpeg_copy_yuv8(**AVCodecContext *avctx, AVFrame
>> *frame, opj_image_t *image)
>>  {
>>     int compno;
>>     int x;
>> @@ -210,6 +231,36 @@ static int libopenjpeg_copy_yuv(**AVCodecContext
>> *avctx, AVFrame *frame, opj_image
>>     return 1;
>>  }
>>
>> +static int libopenjpeg_copy_yuv16(**AVCodecContext *avctx, AVFrame
>> *frame, opj_image_t *image)
>>
>
> libopenjpeg_copy_yuv8 and libopenjpeg_copy_yuv16 look quite similar. maybe
> it's possible to merge them ?
>

They are very similar.  I don't know how to merge them any better though.
 yuv8 iterates over a uint8_t* buffer, whereas yuv16 iterates over a
uint16_t*.  I suppose I could merge parts of them, but the core part of the
functions (the for loops that actually copy the buffer) can't be merged,
AFAIK.  Feedback is appreciated though if you see a good way to do this.


>  @@ -300,6 +362,5 @@ AVCodec ff_libopenjpeg_encoder = {
>>     .close = libopenjpeg_encode_close,
>>     .decode = NULL,
>>     .capabilities = 0,
>> -    .pix_fmts = (const enum PixelFormat[]){PIX_FMT_GRAY8,**
>> PIX_FMT_RGB24,PIX_FMT_RGBA,**PIX_FMT_YUV420P,PIX_FMT_**
>> YUV422P,PIX_FMT_YUV440P,PIX_**FMT_YUV444P},
>>
>
> why do you remove .pix_fmts ? I'd rather update and reformat it.
>

It was just getting long and it meant maintaining the list of supported
pixel formats in 3 places rather than just two (right now it's just in two
switches).  I'll add it back in though.


>      .long_name = NULL_IF_CONFIG_SMALL("OpenJPEG based JPEG 2000 encoder"),
>>  } ;
>> --
>> 1.7.7
>>
>>
> by the way - there are some quite long lines that could be wraped.
>

I'll try and keep them shorter and possibly fix some of the currently
longer ones in a future patch.  Attached is a patch that fixes the two
issues discussed.

Thanks,

Michael
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0104-Fixed-a-formatting-issue-on-one-line-and-added-the-..patch
Type: application/octet-stream
Size: 1467 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20111128/b3659467/attachment.obj>