From patchwork Thu Oct 13 19:21:31 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Cadhalpun X-Patchwork-Id: 993 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.103.140.66 with SMTP id o63csp321413vsd; Thu, 13 Oct 2016 12:21:42 -0700 (PDT) X-Received: by 10.194.86.201 with SMTP id r9mr7988478wjz.5.1476386502144; Thu, 13 Oct 2016 12:21:42 -0700 (PDT) Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id ji3si19521050wjb.137.2016.10.13.12.21.41; Thu, 13 Oct 2016 12:21:42 -0700 (PDT) Received-SPF: pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) client-ip=79.124.17.100; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@googlemail.com; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org; dmarc=fail (p=QUARANTINE dis=NONE) header.from=googlemail.com Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id C3442689219; Thu, 13 Oct 2016 22:21:38 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-lf0-f48.google.com (mail-lf0-f48.google.com [209.85.215.48]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 1C0B1680D56 for ; Thu, 13 Oct 2016 22:21:33 +0300 (EEST) Received: by mail-lf0-f48.google.com with SMTP id b75so150682110lfg.3 for ; Thu, 13 Oct 2016 12:21:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=googlemail.com; s=20120113; h=from:subject:to:references:message-id:date:user-agent:mime-version :in-reply-to; bh=ja3tPgs4lGncejTPinVTabye7BdYzXBmYNEEUQJXYEU=; b=xsPoxyIzu8JI+GaXiNGhQYSeD+kWW2Yb+y8tmaZHt+7gLYIpA33gQREAFdvTVG5tGo /xVWP40I/D7sVrWjZ0SyGpnduvKzbvz3uyzTszzrDjYC6vfEmlMd3X8gO0wWI/MBN5NT gy/KznGtkZ7Fu5TAlm2H2vQqJ6FR/84V2h9WUubpuz+QTFDel7iL1GIO7BNcodwJgtMk pu0mbI/DDuwuoGtPICjkfcvTOozinaskZGslVa0ie+uES57QkD+Hb02c/Xlrp1ZU4r+u xJSsVMcTUpXqr6KtE/hQtf2cq254fBH31hC1HDZzti0A0y+i+CW72wARdbNV+SUd3DR9 78Vw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:subject:to:references:message-id:date :user-agent:mime-version:in-reply-to; bh=ja3tPgs4lGncejTPinVTabye7BdYzXBmYNEEUQJXYEU=; b=c/vs+i2ve3u1iN0w+eQXbviBWNt4drwkc7HWzY7aEtWGjYXd8X5vJ5ypdCHXQfDlDA XlikWZU4uxXQTt8xRgIj8lY7aXCSSmwiMCwlG4YBSUanNQoQi9DlTopWZ0aioKoOp6Tj Q/J2K/mW/P4ZRyOpOUHLeQGtf4vsf32rmKNpnEUj8IDODePdvwtgythVswwDR2E6CfcI jtIhYfvkOozcBQbQPUUmd3wO3XmIT6e+I8jddEzVvY1wdsNpe+OQQoxt8fBQshekSzoq ynO3dy9RDwRk00H5INc/rNnGpqA39NHyN2t2ygYzBjd2MFavEAYYUVZ5oDgur8KrshGB cmtQ== X-Gm-Message-State: AA6/9Rnlc96mHT6o/EY/EFZZP7q4BHxefuBVuPGRdWnwqoi/qcI2DJ1py+t0YXZkV0l7NQ== X-Received: by 10.25.7.201 with SMTP id 192mr12124022lfh.170.1476386493137; Thu, 13 Oct 2016 12:21:33 -0700 (PDT) Received: from [192.168.2.21] (p5B09552C.dip0.t-ipconnect.de. [91.9.85.44]) by smtp.googlemail.com with ESMTPSA id u3sm1120112lja.8.2016.10.13.12.21.31 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 13 Oct 2016 12:21:32 -0700 (PDT) From: Andreas Cadhalpun X-Google-Original-From: Andreas Cadhalpun To: ffmpeg-devel@ffmpeg.org References: Message-ID: Date: Thu, 13 Oct 2016 21:21:31 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.4.0 MIME-Version: 1.0 In-Reply-To: Subject: Re: [FFmpeg-devel] [PATCH] libopenjpegenc: recreate image data buffer after encoding frame X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" On 13.10.2016 03:36, Michael Bradshaw wrote: > Thanks for that (and the link back to the OpenJPEG source). Well dang. I > think it would be better to change the patch to completely remove the image > member from LibOpenJPEGContext, and instead just create a local image (and > destroy it) for every call to libopenjpeg_encode_frame. OK. Attached patch does that for openjpeg 2. I didn't change the behavior for openjpeg 1, as reusing the image works there. Best regards, Andreas From 350ceab27db0346d89698070e40b1ae19ff1d559 Mon Sep 17 00:00:00 2001 From: Andreas Cadhalpun Date: Thu, 13 Oct 2016 21:16:35 +0200 Subject: [PATCH] libopenjpegenc: stop reusing image data buffer for openjpeg 2 openjpeg 2 sets the data pointers of the image components to NULL, causing segfaults if the image is reused. Signed-off-by: Andreas Cadhalpun --- libavcodec/libopenjpegenc.c | 41 ++++++++++++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/libavcodec/libopenjpegenc.c b/libavcodec/libopenjpegenc.c index 5042507..857ee1a 100644 --- a/libavcodec/libopenjpegenc.c +++ b/libavcodec/libopenjpegenc.c @@ -52,7 +52,9 @@ typedef struct LibOpenJPEGContext { AVClass *avclass; +#if OPENJPEG_MAJOR_VERSION == 1 opj_image_t *image; +#endif // OPENJPEG_MAJOR_VERSION == 1 opj_cparameters_t enc_params; #if OPENJPEG_MAJOR_VERSION == 1 opj_event_mgr_t event_mgr; @@ -369,18 +371,22 @@ static av_cold int libopenjpeg_encode_init(AVCodecContext *avctx) cinema_parameters(&ctx->enc_params); } +#if OPENJPEG_MAJOR_VERSION == 1 ctx->image = mj2_create_image(avctx, &ctx->enc_params); if (!ctx->image) { av_log(avctx, AV_LOG_ERROR, "Error creating the mj2 image\n"); err = AVERROR(EINVAL); goto fail; } +#endif // OPENJPEG_MAJOR_VERSION == 1 return 0; fail: +#if OPENJPEG_MAJOR_VERSION == 1 opj_image_destroy(ctx->image); ctx->image = NULL; +#endif // OPENJPEG_MAJOR_VERSION == 1 return err; } @@ -591,19 +597,25 @@ static int libopenjpeg_encode_frame(AVCodecContext *avctx, AVPacket *pkt, const AVFrame *frame, int *got_packet) { LibOpenJPEGContext *ctx = avctx->priv_data; - opj_image_t *image = ctx->image; + int ret; + AVFrame *gbrframe; + int cpyresult = 0; #if OPENJPEG_MAJOR_VERSION == 1 + opj_image_t *image = ctx->image; opj_cinfo_t *compress = NULL; opj_cio_t *stream = NULL; int len; #else // OPENJPEG_MAJOR_VERSION == 2 + PacketWriter writer = { 0 }; opj_codec_t *compress = NULL; opj_stream_t *stream = NULL; - PacketWriter writer = { 0 }; + opj_image_t *image = mj2_create_image(avctx, &ctx->enc_params); + if (!image) { + av_log(avctx, AV_LOG_ERROR, "Error creating the mj2 image\n"); + ret = AVERROR(EINVAL); + goto done; + } #endif // OPENJPEG_MAJOR_VERSION == 1 - int cpyresult = 0; - int ret; - AVFrame *gbrframe; switch (avctx->pix_fmt) { case AV_PIX_FMT_RGB24: @@ -626,8 +638,10 @@ static int libopenjpeg_encode_frame(AVCodecContext *avctx, AVPacket *pkt, case AV_PIX_FMT_GBRP14: case AV_PIX_FMT_GBRP16: gbrframe = av_frame_clone(frame); - if (!gbrframe) - return AVERROR(ENOMEM); + if (!gbrframe) { + ret = AVERROR(ENOMEM); + goto done; + } gbrframe->data[0] = frame->data[2]; // swap to be rgb gbrframe->data[1] = frame->data[0]; gbrframe->data[2] = frame->data[1]; @@ -684,19 +698,21 @@ static int libopenjpeg_encode_frame(AVCodecContext *avctx, AVPacket *pkt, av_log(avctx, AV_LOG_ERROR, "The frame's pixel format '%s' is not supported\n", av_get_pix_fmt_name(avctx->pix_fmt)); - return AVERROR(EINVAL); + ret = AVERROR(EINVAL); + goto done; break; } if (!cpyresult) { av_log(avctx, AV_LOG_ERROR, "Could not copy the frame data to the internal image buffer\n"); - return -1; + ret = -1; + goto done; } #if OPENJPEG_MAJOR_VERSION == 2 if ((ret = ff_alloc_packet2(avctx, pkt, 1024, 0)) < 0) { - return ret; + goto done; } #endif // OPENJPEG_MAJOR_VERSION == 2 @@ -763,7 +779,7 @@ static int libopenjpeg_encode_frame(AVCodecContext *avctx, AVPacket *pkt, #error Missing call to opj_stream_set_user_data #endif - if (!opj_start_compress(compress, ctx->image, stream) || + if (!opj_start_compress(compress, image, stream) || !opj_encode(compress, stream) || !opj_end_compress(compress, stream)) { av_log(avctx, AV_LOG_ERROR, "Error during the opj encode\n"); @@ -782,6 +798,7 @@ done: #if OPENJPEG_MAJOR_VERSION == 2 opj_stream_destroy(stream); opj_destroy_codec(compress); + opj_image_destroy(image); #else opj_cio_close(stream); opj_destroy_compress(compress); @@ -791,10 +808,12 @@ done: static av_cold int libopenjpeg_encode_close(AVCodecContext *avctx) { +#if OPENJPEG_MAJOR_VERSION == 1 LibOpenJPEGContext *ctx = avctx->priv_data; opj_image_destroy(ctx->image); ctx->image = NULL; +#endif // OPENJPEG_MAJOR_VERSION == 1 return 0; } -- 2.9.3