On Thu, Nov 19, 2015 at 1:21 AM, Moritz Barsnick <barsn...@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.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
