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

[Michael Niedermayer]

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()


> +    }
> +
> +    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

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In fact, the RIAA has been known to suggest that students drop out
of college or go to community college in order to be able to afford
settlements. -- The RIAA
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090203/2d0ea913/attachment.pgp>