[FFmpeg-devel] [PATCH] libopenjpeg wrapper for jpeg2k decoding

[Jai Menon]

Hi,

On Wed, Feb 4, 2009 at 2:12 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> On Wed, Feb 04, 2009 at 10:33:52AM +0530, Jai Menon wrote:
>> Hi,
>>
>> On Tue, Feb 3, 2009 at 5:51 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
>> > On Tue, Feb 03, 2009 at 11:31:23AM +0530, Jai Menon wrote:
>> >> Hi,
>> >>
>> >> On Tue, Feb 3, 2009 at 12:58 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
>> >> > On Mon, Feb 02, 2009 at 04:48:17PM +0530, Jai Menon wrote:
>> >> >> Hi,
>> >> >>
>> >> >> 2009/1/29 Michael Niedermayer <michaelni at gmx.at>:
>> >> >> > On Thu, Jan 29, 2009 at 11:35:31AM +0530, Jai Menon wrote:
>> >> >> >> Hi,
>> >> >> >>
>> >> >> >> On Thu, Jan 29, 2009 at 4:27 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
>> >> >> >> > On Wed, Jan 28, 2009 at 02:23:46PM +0530, Jai Menon wrote:
>> >> >>
>> >> >> [...]
>> >> >>
>> >> >> > dec is written to twice with no read in between, this applies to more vars
>> >> >>
>> >> >> fixed.
>> >> >>
>> >> >> [...]
>> >> >>
>> >> >> > sec hole when width/height changes
>> >> >>
>> >> >> fixed (i think).
>> >> >>
>> >> >> i also reworked most of the code to use get/release_buffer
>> >> >> revised patch attached.
>> >> > [...]
>> >> >> +static int libopenjpeg_decode_frame(AVCodecContext *avctx,
>> >> >> +                                    void *data, int *data_size,
>> >> >> +                                    const uint8_t *buf, int buf_size)
>> >> >> +{
>> >> >> +    LibOpenJPEGContext *ctx = avctx->priv_data;
>> >> >> +    AVFrame *picture = &ctx->image, *output = data;
>> >> >> +    opj_dinfo_t *dec;
>> >> >
>> >> >> +    opj_cio_t *stream = NULL;
>> >> >
>> >> > written twice before read
>> >>
>> >> this and another one fixed.
>> >>
>> >> >> +    opj_image_t *image = NULL;
>> >> >> +    int width, height, has_alpha = 0, ret = -1;
>> >> >> +    int x, y, index;
>> >> >> +    uint8_t *img_ptr;
>> >> >> +    float scale;
>> >> >> +
>> >> >> +    *data_size = 0;
>> >> >> +
>> >> >> +    // Check if input is a raw jpeg2k codestream or in jp2 wrapping
>> >> >> +    if((AV_RB32(buf) == 12) &&
>> >> >> +       (AV_RB32(buf + 4) == JP2_SIG_TYPE) &&
>> >> >> +       (AV_RB32(buf + 8) == JP2_SIG_VALUE)) {
>> >> >> +         dec = opj_create_decompress(CODEC_JP2);
>> >> >
>> >> >> +    }
>> >> >> +    else dec = opj_create_decompress(CODEC_J2K);
>> >> >
>> >> > diego likely wont like this kind of } placement style
>> >>
>> >> changed.
>> >>
>> >> > [...]
>> >> >> +    scale = 255.0f / (float)((1 << image->comps[0].prec) - 1);
>> >> >> +    for(y = 0; y < height; y++) {
>> >> >> +        index = y*width;
>> >> >> +        img_ptr = picture->data[0] + y*picture->linesize[0];
>> >> >> +        for(x = 0; x < width; x++, index++) {
>> >> >> +            *img_ptr++ = image->comps[0].data[index] * scale;
>> >> >> +            if(image->numcomps > 2 && check_image_attributes(image)) {
>> >> >> +                *img_ptr++ = image->comps[1].data[index] * scale;
>> >> >> +                *img_ptr++ = image->comps[2].data[index] * scale;
>> >> >> +                if(has_alpha)
>> >> >> +                    *img_ptr++ = image->comps[3].data[index] * scale;
>> >> >> +            }
>> >> >> +        }
>> >> >> +    }
>> >> >
>> >> > lrintf()
>> >>
>> >> hmm...i made this generic and modified it to use integer arithmetic. i
>> >> think its better.
>> >> revised patch attached.
>> >>
>> >
>> > [...]
>> >
>> >> +        if(image->comps[x].prec > 8)
>> >> +             adjust[x] = image->comps[x].prec - 8;
>> >> +        else adjust[x] = 0;
>> >
>> > there should be a {} between the if/else, it doesnt add any extra lines
>> > but allows something to be inserted without changing the if/else lines, that
>> > is it means future patches would be simpler
>> >
>> > but then i realize now this could be written simpler with FFMAX()
>>
>> changed.
>>
>> >> +    }
>> >> +
>> >> +    for(y = 0; y < height; y++) {
>> >> +        index = y*width;
>> >> +        img_ptr = picture->data[0] + y*picture->linesize[0];
>> >> +        for(x = 0; x < width; x++, index++) {
>> >> +            *img_ptr++ = (image->comps[0].data[index] >> adjust[0]) +
>> >> +                         (image->comps[0].data[index] >> (adjust[0] - 1) & 1);
>> >
>> > if adjust[x] == 0 -> A >> (-1) which is undefined
>> >
>> > also this looks like (data+(1<<(adjust-1)))>>adjust
>>
>> made it much simpler since i didn't see any difference in the test images.
>
> [...]
>
>> +    // Check if input is a raw jpeg2k codestream or in jp2 wrapping
>> +    if((AV_RB32(buf) == 12) &&
>> +       (AV_RB32(buf + 4) == JP2_SIG_TYPE) &&
>> +       (AV_RB32(buf + 8) == JP2_SIG_VALUE)) {
>> +         dec = opj_create_decompress(CODEC_JP2);
>> +    } else {
>> +        dec = opj_create_decompress(CODEC_J2K);
>
> one of these is wrongly indented

sorry, missed that. fixed.

>> +    }
>> +
>> +    if(!dec) {
>> +        av_log(avctx, AV_LOG_ERROR, "Error initializing decoder.\n");
>> +        return -1;
>> +    }
>> +    opj_set_event_mgr((opj_common_ptr)dec, NULL, NULL);
>> +    // Tie decoder with decoding parameters
>> +    opj_setup_decoder(dec, &ctx->dec_params);
>> +    stream = opj_cio_open((opj_common_ptr)dec, buf, buf_size);
>> +    if(!stream) {
>> +        av_log(avctx, AV_LOG_ERROR, "Codestream could not be opened for reading.\n");
>> +        opj_destroy_decompress(dec);
>> +        return -1;
>> +    }
>> +    // Decode the codestream
>> +    image = opj_decode_with_info(dec, stream, NULL);
>
> i think an empty line before the // comments would improve readabilty

ok. changed.

> [...]
>> +AVCodec libopenjpeg_decoder = {
>> +    "libopenjpeg",
>> +    CODEC_TYPE_VIDEO,
>> +    CODEC_ID_JPEG2000,
>> +    sizeof(LibOpenJPEGContext),
>> +    libopenjpeg_decode_init,
>> +    NULL,
>> +    libopenjpeg_decode_close,
>> +    libopenjpeg_decode_frame,
>
>> +    CODEC_CAP_DELAY,
>
> looks wrong

this one was left over from the initial copy paste. removed in new patch.

[...]


-- 
Regards,

Jai
-------------- next part --------------
A non-text attachment was scrubbed...
Name: libopenjpeg_wrapper.patch
Type: text/x-patch
Size: 11727 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090204/7e30e51e/attachment.bin>