From patchwork Fri Sep 4 23:17:10 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 22104 Return-Path: X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id 048A644B56A for ; Sat, 5 Sep 2020 02:17:55 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id D35E568B32D; Sat, 5 Sep 2020 02:17:54 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wr1-f67.google.com (mail-wr1-f67.google.com [209.85.221.67]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id D9809687ED6 for ; Sat, 5 Sep 2020 02:17:47 +0300 (EEST) Received: by mail-wr1-f67.google.com with SMTP id j2so8945791wrx.7 for ; Fri, 04 Sep 2020 16:17:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=0Wul8fNjDuTPPvGwZQwWUrEBK5Hg+9cL+/VFuiCF1gQ=; b=kyl3vT72AOwumvN/ndJGHGrpVvz6hU6TB5N9qxBhG8LHYGyk8h4pjInyLNoKe56SPo qYJzZoz1wTYjkVV0RcnVCF9+qkNOXau7GzLg5IE96JXopf2ClLGh5ZC3rd8L8CxR4VD7 7YJQF3o/JDOE05naO7brvLidx7XCgWrCjZrxs5+wdYcoy01IxYXn1JxsQ94h3Blp3py3 1dYsm0dhsoAA1ujlSfK7B4KMgdQSGMU0+nGRwehPO35t92fZd4e0Viiq0W9K5aKvK43q UtaqqJuK5YfiGv0xNnxu9sqGWCtehEXcpfXJphI2gCacIehbeS9QesklNQRak2Akvp7H lU6w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=0Wul8fNjDuTPPvGwZQwWUrEBK5Hg+9cL+/VFuiCF1gQ=; b=TZM9o/QNi+j4LfQttiYg4vJwXr006gtGtKWHsOr6Db0XIuHIi8p/nsTBRgC9ns3pB6 piOspRRoSZ7vrtc+lupdjoRMFWYGAu+698t3Fsq/bQdKSc2GL8AOeu4q/slyL8acWEOU ayrnBp3J0bc7T2648lEAgH2SLNBEt/SvmpX9Upzxm2DlEq+sY9FmUICixf5M+Mz/4N1R PxdFViP4fBaldCSj/xns6EkqkA9cRbMTO8HfTFmKzjShnOCmtDpBT2T8Zx1ct1CGCzX7 h3HyniMma1K2XoH/mJkMNlEL0MzD3QOpP2PFvek0JOz2ug/nM+snoGv6cE9mbwXEZdRs 0WHw== X-Gm-Message-State: AOAM5332kfwLAZcbUBjzSB9E+9+3zeGKaA7L7qXQKxjfMiA0kg/7KRNg 5imNiZrM6CBxUDqKwDNmOIoKn7AhSNM= X-Google-Smtp-Source: ABdhPJzYBN0WgnayPFPugUbm+TzL26cJnBCcJTATRTi+aSeC1WzIMShl0AzPvuOfnv6dYIRDYL6dmA== X-Received: by 2002:adf:e6cf:: with SMTP id y15mr9733648wrm.346.1599261466784; Fri, 04 Sep 2020 16:17:46 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc1fb0f.dynamic.kabel-deutschland.de. [188.193.251.15]) by smtp.gmail.com with ESMTPSA id f14sm14598098wrv.72.2020.09.04.16.17.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 04 Sep 2020 16:17:46 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Sat, 5 Sep 2020 01:17:10 +0200 Message-Id: <20200904231716.16182-2-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200904231716.16182-1-andreas.rheinhardt@gmail.com> References: <20200904231716.16182-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 2/8] avcodec/jpeglsenc: Don't modify frame we don't own 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 Cc: Andreas Rheinhardt Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" The jpegls encoder uses three buffers (as well as its state) to perform its function: A copy of the last encoded line as a decoder would decode it, the part of the current line that has been encoded (again, as a decoder would decode it) and the part of the current line that is not yet encoded. The encoder solves this by modifying the input frame as it encodes the output (it also zero-allocates a line to serve as last line for the first line where no preceding line exists); yet this is wrong as said frame is not owned by the encoder, so it must not be modified (and it is given to the encoder as const AVFrame *) without making it writable. This patch solves this bug by allocating two lines, one for the last and one for the currently encoded line of output (as a decoder would decode it). Notice that the frame is only modified if the encoder is in the non-default non-lossless mode. Signed-off-by: Andreas Rheinhardt --- Example command line to trigger this behaviour: ffmpeg -i fate-suite/png1/lena-rgb24.png -an -map 0 -map 0 -map 0 -c:v:0 jpegls -c:v:1 jpegls -pred:v:1 median -c:v:2 jpegls -f streamhash - The output of the first and third stream differ with git master. (For some reason "pred" is used to set how lossy the mode is.) libavcodec/jpeglsenc.c | 50 +++++++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/libavcodec/jpeglsenc.c b/libavcodec/jpeglsenc.c index feea6fdde7..67897ce8ae 100644 --- a/libavcodec/jpeglsenc.c +++ b/libavcodec/jpeglsenc.c @@ -141,7 +141,7 @@ static inline void ls_encode_run(JLSState *state, PutBitContext *pb, int run, * Encode one line of image */ static inline void ls_encode_line(JLSState *state, PutBitContext *pb, - void *last, void *cur, int last2, int w, + void *last, void *cur, const void *in, int last2, int w, int stride, int comp, int bits) { int x = 0; @@ -168,7 +168,7 @@ static inline void ls_encode_line(JLSState *state, PutBitContext *pb, run = 0; RUNval = Ra; - while (x < w && (FFABS(R(cur, x) - RUNval) <= state->near)) { + while (x < w && (FFABS(R(in, x) - RUNval) <= state->near)) { run++; W(cur, x, Ra); x += stride; @@ -179,7 +179,7 @@ static inline void ls_encode_line(JLSState *state, PutBitContext *pb, Rb = R(last, x); RItype = FFABS(Ra - Rb) <= state->near; pred = RItype ? Ra : Rb; - err = R(cur, x) - pred; + err = R(in, x) - pred; if (!RItype && Ra > Rb) err = -err; @@ -195,7 +195,9 @@ static inline void ls_encode_line(JLSState *state, PutBitContext *pb, else Ra = av_clip(pred - err * state->twonear, 0, state->maxval); W(cur, x, Ra); - } + } else + W(cur, x, R(in, x)); + if (err < 0) err += state->range; if (err >= state->range + 1 >> 1) @@ -218,11 +220,11 @@ static inline void ls_encode_line(JLSState *state, PutBitContext *pb, context = -context; sign = 1; pred = av_clip(pred - state->C[context], 0, state->maxval); - err = pred - R(cur, x); + err = pred - R(in, x); } else { sign = 0; pred = av_clip(pred + state->C[context], 0, state->maxval); - err = R(cur, x) - pred; + err = R(in, x) - pred; } if (state->near) { @@ -235,7 +237,8 @@ static inline void ls_encode_line(JLSState *state, PutBitContext *pb, else Ra = av_clip(pred - err * state->twonear, 0, state->maxval); W(cur, x, Ra); - } + } else + W(cur, x, R(in, x)); ls_encode_regular(state, pb, context, err); } @@ -276,8 +279,8 @@ static int encode_picture_ls(AVCodecContext *avctx, AVPacket *pkt, GetBitContext gb; uint8_t *buf2 = NULL; uint8_t *zero = NULL; - uint8_t *cur = NULL; - uint8_t *last = NULL; + const uint8_t *in; + uint8_t *last, *cur; JLSState *state = NULL; int i, size, ret; int comps; @@ -343,28 +346,29 @@ FF_ENABLE_DEPRECATION_WARNINGS ls_store_lse(state, &pb); - zero = last = av_mallocz(FFABS(p->linesize[0])); + zero = last = av_calloc(FFABS(p->linesize[0]), 2); if (!zero) goto memfail; + cur = zero + FFABS(p->linesize[0]); - cur = p->data[0]; + in = p->data[0]; if (avctx->pix_fmt == AV_PIX_FMT_GRAY8) { int t = 0; for (i = 0; i < avctx->height; i++) { - ls_encode_line(state, &pb2, last, cur, t, avctx->width, 1, 0, 8); + ls_encode_line(state, &pb2, last, cur, in, t, avctx->width, 1, 0, 8); t = last[0]; - last = cur; - cur += p->linesize[0]; + FFSWAP(void *, last, cur); + in += p->linesize[0]; } } else if (avctx->pix_fmt == AV_PIX_FMT_GRAY16) { int t = 0; for (i = 0; i < avctx->height; i++) { - ls_encode_line(state, &pb2, last, cur, t, avctx->width, 1, 0, 16); + ls_encode_line(state, &pb2, last, cur, in, t, avctx->width, 1, 0, 16); t = *((uint16_t *)last); - last = cur; - cur += p->linesize[0]; + FFSWAP(void *, last, cur); + in += p->linesize[0]; } } else if (avctx->pix_fmt == AV_PIX_FMT_RGB24) { int j, width; @@ -373,12 +377,12 @@ FF_ENABLE_DEPRECATION_WARNINGS width = avctx->width * 3; for (i = 0; i < avctx->height; i++) { for (j = 0; j < 3; j++) { - ls_encode_line(state, &pb2, last + j, cur + j, Rc[j], + ls_encode_line(state, &pb2, last + j, cur + j, in + j, Rc[j], width, 3, j, 8); Rc[j] = last[j]; } - last = cur; - cur += p->linesize[0]; + FFSWAP(void *, last, cur); + in += p->linesize[0]; } } else if (avctx->pix_fmt == AV_PIX_FMT_BGR24) { int j, width; @@ -387,12 +391,12 @@ FF_ENABLE_DEPRECATION_WARNINGS width = avctx->width * 3; for (i = 0; i < avctx->height; i++) { for (j = 2; j >= 0; j--) { - ls_encode_line(state, &pb2, last + j, cur + j, Rc[j], + ls_encode_line(state, &pb2, last + j, cur + j, in + j, Rc[j], width, 3, j, 8); Rc[j] = last[j]; } - last = cur; - cur += p->linesize[0]; + FFSWAP(void *, last, cur); + in += p->linesize[0]; } }