[FFmpeg-devel] [PATCH] avcodec: add OpenJPEG 2.x compatibility

[Michael Bradshaw]

On Thu, Nov 19, 2015 at 1:21 AM, Moritz Barsnick <barsnick at gmx.net> wrote:

> On Wed, Nov 18, 2015 at 21:22:57 -0800, Michael Bradshaw wrote:
>
> >      if (!dec) {
> >          av_log(avctx, AV_LOG_ERROR, "Error initializing decoder.\n");
> > -        return AVERROR_UNKNOWN;
> > +        ret = AVERROR_EXTERNAL;
> > +        goto done;
> >      }
> [...]
> >      if (!stream) {
> >          av_log(avctx, AV_LOG_ERROR,
> >                 "Codestream could not be opened for reading.\n");
> > -        opj_destroy_decompress(dec);
> > -        return AVERROR_UNKNOWN;
> > +        ret = AVERROR_EXTERNAL;
> > +        goto done;
> >      }
> [...]
> > -    if (!image) {
> > -        av_log(avctx, AV_LOG_ERROR, "Error decoding codestream.\n");
> > -        opj_destroy_decompress(dec);
> > -        return AVERROR_UNKNOWN;
> > +    if (ret) {
> > +        av_log(avctx, AV_LOG_ERROR, "Error decoding codestream
> header.\n");
> > +        ret = AVERROR_EXTERNAL;
> > +        goto done;
> >      }
> [...]
> >      if (avctx->pix_fmt == AV_PIX_FMT_NONE) {
> > -        av_log(avctx, AV_LOG_ERROR, "Unable to determine pixel
> format\n");
> > +        av_log(avctx, AV_LOG_ERROR, "Unable to determine pixel
> format.\n");
> > +        ret = AVERROR_UNKNOWN;
> >          goto done;
> >      }
> [...]
> >      if (!stream) {
> >          av_log(avctx, AV_LOG_ERROR,
> >                 "Codestream could not be opened for reading.\n");
> > -        ret = AVERROR_UNKNOWN;
> > +        ret = AVERROR_EXTERNAL;
> >          goto done;
> >      }
> [...]
> >          av_log(avctx, AV_LOG_ERROR, "Error decoding codestream.\n");
> > -        ret = AVERROR_UNKNOWN;
> > +        ret = AVERROR_EXTERNAL;
> [...]
> >      LibOpenJPEGContext *ctx = avctx->priv_data;
> > -    int err = AVERROR(ENOMEM);
> > +    int err = 0;
> [...]
> >      if (!compress) {
> >          av_log(avctx, AV_LOG_ERROR, "Error creating the compressor\n");
> > -        return AVERROR(ENOMEM);
> > +        ret = AVERROR(ENOMEM);
> > +        goto done;
> >      }
> [...]
> >          av_log(avctx, AV_LOG_ERROR, "Error creating the cio stream\n");
> > -        return AVERROR(ENOMEM);
> > +        ret = AVERROR(ENOMEM);
> > +        goto done;
> [...]
> >      if (!opj_encode(compress, stream, image, NULL)) {
> >          av_log(avctx, AV_LOG_ERROR, "Error during the opj encode\n");
> > -        return -1;
> > +        ret = AVERROR_EXTERNAL;
> > +        goto done;
> >      }
> [...]
> >      len = cio_tell(stream);
> >      if ((ret = ff_alloc_packet2(avctx, pkt, len, 0)) < 0) {
> > -        return ret;
> > +        goto done;
> >      }
>
> Are these not unrelated changes (i.e. valid even without integration of
> openjpeg2), which would belong into a separate patch? Just wondering.
>
> (IOW, you're changing the openjpeg1 codepath/behavior as well? Or are
> these changes for compatibility?)
>

It makes it so v1 and v2 can share the cleanup/error/logging code (for the
most part). Removing those changes would require more #ifdefs and duplicate
logging messages so v2 could handle errors, so I don't really consider them
superfluous changes for this patch. The intent was not to change the v1
code path or behavior.

Except a couple changes that are for consistency (i.e. changing
AVERROR_UNKNOWN to AVERROR_EXTERNAL in a couple spots) or correctness (the
change in if (avctx->pix_fmt == AV_PIX_FMT_NONE) that sets ret =
AVERROR_UNKNOWN;). I could separate those changes out into a different
patch, but I don't think either patch would really be complete without the
other, which is why I did it in one patch.