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

[Jai Menon]

Hi,

On Tue, Jan 27, 2009 at 4:52 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> On Tue, Jan 27, 2009 at 10:26:19AM +0530, Jai Menon wrote:
>> Hi,
>>
>> This has proved useful for debugging the soc code and hopefully for
>> other purposes as well.
>>
>> The code itself is meant for review but the buildsystem part is mostly
>> rfc. I don't know if building ffmpeg/lavc with
>> openjpeg support makes it unredistributable.
>>
>> Also, I have tested this only with jpeg2k conformance suite since my
>> last attempt to download a R3d file failed
>> miserably :-) It would be interesting to know if those work as well.
> [...]
>
>> +static int check_image_attributes(opj_image_t *image)
>> +{
>> +    return(image->comps[0].dx == image->comps[1].dx
>> +           && image->comps[1].dx == image->comps[2].dx
>> +                && image->comps[0].dy == image->comps[1].dy
>> +                && image->comps[1].dy == image->comps[2].dy
>> +           && image->comps[0].prec == image->comps[1].prec
>> +                && image->comps[1].prec == image->comps[2].prec);
>> +}
>
> tabs

very sorry, I should probably use a better text editor.
fixed.

> [...]
>> +    switch(image->numcomps)
>> +    {
>> +        case 1:  avctx->pix_fmt = PIX_FMT_GRAY8;
>> +                 break;
>
> the  { should likely be on the same line as the switch and case and
> switch should not be indented relative to each other.
> Anyway this is just a suggestion, not important if you dislike it

can i keep it this way? :-)
its easier on my eyes.

>> +        case 3:  if(check_image_attributes(image))
>> +                     avctx->pix_fmt = PIX_FMT_RGB24;
>> +                 else {
>
> these should have a {} so future changes have smaller diffs

done.

>> +                     avctx->pix_fmt = PIX_FMT_GRAY8;
>> +                     av_log(avctx, AV_LOG_ERROR, "Only first component will be used\n");
>> +                 }
>> +                 break;
>> +        case 4:  has_alpha = 1;
>> +                 avctx->pix_fmt = PIX_FMT_RGB32;
>> +                 break;
>> +        default: av_log(avctx, AV_LOG_ERROR, "%d components unsupported \n", image->numcomps);
>> +                 ret = -1;
>> +                 goto done;
>> +    }
>> +
>> +    avpicture_fill(picture, NULL, avctx->pix_fmt, avctx->width, avctx->height);
>> +    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");
>
>> +            ret = -1;
>
> you can avoid this by setting ret to -1 at the top

yes, changed.

revised patch attached.

> [...]
> Observe your enemies, for they first find out your faults. -- Antisthenes
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
>
> iD8DBQFJfu5lYR7HhwQLD6sRAicuAKCKqDkybJGf5tsjA7iXvFztHipb/wCePStt
> lMCknauDU3uOcc3V7p0dPRg=
> =7LxZ
> -----END PGP SIGNATURE-----


-- 
Regards,

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