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

Michael Niedermayer michaelni
Fri Jan 30 10:30:01 CET 2009


On Fri, Jan 30, 2009 at 12:02:48PM +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:
> 
> [...]
> 
> >> +static int libopenjpeg_decode_frame(AVCodecContext *avctx,
> >> +                                    void *data, int *data_size,
> >> +                                    const uint8_t *buf, int buf_size)
> >> +{
> >> +    LibOpenJPEGContext *ctx = avctx->priv_data;
> >> +    AVPicture *picture = data;
> >> +    opj_dinfo_t *dec = NULL;
> >> +    opj_cio_t *stream = NULL;
> >> +    opj_image_t *image = NULL;
> >> +    int width, height, has_alpha = 0, ret = -1;
> >> +    int x, y, index;
> >> +    int picture_size;
> >> +    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);
> >
> > dec is written to twice with no read in between, this applies to more vars
> 
> I didn't quite get what this implies, other than that the
> initialization at the top can be removed.

thats what i meant, the initialization at the top is unneeded


> 
> > [...]
> >> +    picture_size = avpicture_get_size(avctx->pix_fmt, avctx->width, avctx->height);
> >> +    if(!ctx->img_buf) {
> >> +        ctx->img_buf = av_mallocz(picture_size);
> >> +        if(!ctx->img_buf) {
> >> +            av_log(avctx, AV_LOG_ERROR, "Couldn't allocate image buffer.\n");
> >> +            goto done;
> >> +        }
> >> +    }
> >> +    img_ptr = ctx->img_buf;
> >> +    picture->data[0] = ctx->img_buf;
> >> +    scale = 255.0f / (float)((1 << image->comps[0].prec) - 1);
> >> +    for(y = 0; y < height; y++) {
> >> +        index = y*width;
> >> +        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;
> >> +            }
> >> +        }
> >> +    }
> >
> > sec hole when width/height changes
> 
> The assumption here was that width/height wouldn't change. Allocating
> the buffer
> for each frame was too slow. I could change it though, I mostly care only about
> the soc code :-)

security issues must be fixed even if that means svn rm of code or that it
runs at half the speed.
but in this case here, neither is the case, you could check w/h 
even better would be to use get/release_buffer()

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

It is not what we do, but why we do it that matters.
-------------- 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/20090130/2fbfa13e/attachment.pgp>



More information about the ffmpeg-devel mailing list