From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 476B040CC0 for ; Sat, 9 Apr 2022 15:45:06 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 72E1668B2F6; Sat, 9 Apr 2022 18:45:04 +0300 (EEST) Received: from mail-yw1-f175.google.com (mail-yw1-f175.google.com [209.85.128.175]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id B686168B0E4 for ; Sat, 9 Apr 2022 18:44:58 +0300 (EEST) Received: by mail-yw1-f175.google.com with SMTP id 00721157ae682-2ebdf6ebd29so54012987b3.2 for ; Sat, 09 Apr 2022 08:44:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=sG5+BGki/7nSBnh72NN3nMcgXrNeV+xWXda8SQoPGFI=; b=m9ROsO6gJpBuk7Xmnx8WYevosBzCUsYTWntlEmT61V4AYfuw86QXdjQcByKrrfsaUy NBrwsAMd4q972SjAg4B3lc9t0aCo5ZMbTD3Kz9Dy9waHgNFH0TTd0coj28Fjo4mK221K GqOZr7ysiC5K+rrp31qpnuKcgoU8ukM0JmFCp24x9oywTgs4NggkYTgHHqlgpFs2wTbq iKVie3B4fkxBF1/3Zlzk4J6jq2/fL0Dmky07WEEYvnPruLvRxIcWxx0jeSC4A6RZVhcY KB7CAkXXtXOi8jBeyBTLR2kTTVsOJMVUp7Uw0tjAJyxXtNWKBywWy/M6qpv1poT+5Y/6 Q54A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=sG5+BGki/7nSBnh72NN3nMcgXrNeV+xWXda8SQoPGFI=; b=4U1eHxTlviGKAApA5eT6T/fwN/ol2+wQJwkKW9NkXjYHpEylWrnWLhdzCNpfSbg+cw nKLxoGQgdeRT8sjvUudc6oyJIzOf242KPvBcL9U62PDpeo1cSC4ncd3ID8Q1aCkdprMK zxoDeCy4w4fu5+AnF5mx0om2iTxo4a6X7diJj6GsURy3IqXNCJJFqptOJ1fNyD+HI5K+ 37OFd9HFfw3o/hqk3BvrWTWcIR3Gpqq9yuOLF14oo8pRyeufJInk5MkQE6xQgcQB/y0V jLJT8qCpiP+8iwNxbYP6JjDza/KCIGPymRHNXDuP2833eV97QSVRdXwce8N9Y9d+zDUn cZEA== X-Gm-Message-State: AOAM533WRj7fHZIGEvtNH8xDAt2HUjklUBQM/HC+oATZtg1mtgri2wzx Gx/ZUUs711s5oQNnUI2e/ksdep8f4eOk0hTRNPaqnZv4l2o= X-Google-Smtp-Source: ABdhPJwfWPf/aDlpRor0AA9edB8wgzRsQp2fQ2R0l0imxou/Q8/e45V8Gex9soBpbQiV9lkLt8YCx67k/UMyyKqFuIQ= X-Received: by 2002:a0d:e8c2:0:b0:2eb:36b2:a1c3 with SMTP id r185-20020a0de8c2000000b002eb36b2a1c3mr20172986ywe.293.1649519097082; Sat, 09 Apr 2022 08:44:57 -0700 (PDT) MIME-Version: 1.0 References: <20220405165504.29261-1-leo.izen@gmail.com> <20220405165504.29261-4-leo.izen@gmail.com> <164951025246.21047.6570158843618793411@lain.red.khirnov.net> <1df72419-967b-df7b-e819-e4cb9b9bf08e@gmail.com> In-Reply-To: <1df72419-967b-df7b-e819-e4cb9b9bf08e@gmail.com> From: Hendrik Leppkes Date: Sat, 9 Apr 2022 17:44:45 +0200 Message-ID: To: FFmpeg development discussions and patches Subject: Re: [FFmpeg-devel] [PATCH v13 3/4] avcodec/libjxl: add Jpeg XL encoding via libjxl X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 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 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On Sat, Apr 9, 2022 at 5:38 PM James Almer wrote: > > > > On 4/9/2022 10:17 AM, Anton Khirnov wrote: > > Quoting Leo Izen (2022-04-05 18:55:03) > >> +static int libjxl_init_jxl_encoder(AVCodecContext *avctx) > >> +{ > >> + LibJxlEncodeContext *ctx = avctx->priv_data; > >> + > >> + /* reset the encoder every frame for image2 muxer */ > >> + JxlEncoderReset(ctx->encoder); > >> + > >> + ctx->options = JxlEncoderFrameSettingsCreate(ctx->encoder, NULL); > >> + if (!ctx->options) { > >> + av_log(avctx, AV_LOG_ERROR, "Failed to create JxlEncoderOptions\n"); > >> + return AVERROR_EXTERNAL; > >> + } > >> + > >> + /* This needs to be set each time the decoder is reset */ > >> + if (JxlEncoderSetParallelRunner(ctx->encoder, JxlThreadParallelRunner, ctx->runner) > >> + != JXL_ENC_SUCCESS) { > >> + av_log(avctx, AV_LOG_ERROR, "Failed to set JxlThreadParallelRunner\n"); > >> + return AVERROR_EXTERNAL; > >> + } > >> + > >> + /* these shouldn't fail, libjxl bug notwithstanding */ > >> + if (JxlEncoderFrameSettingsSetOption(ctx->options, JXL_ENC_FRAME_SETTING_EFFORT, ctx->effort) > >> + != JXL_ENC_SUCCESS) { > >> + av_log(avctx, AV_LOG_ERROR, "Failed to set effort to: %d\n", ctx->effort); > >> + return AVERROR_EXTERNAL; > >> + } > >> + > >> + /* check for negative zero, our default */ > >> + if (1.0f / ctx->distance == 1.0f / -0.0f) { > > > > IIRC division by zero is UB. Why not make the default -1.0 and then just > > check whether the number is negative? > > > >> + /* > >> + * This buffer will be copied when the generic > >> + * code makes this packet refcounted, > >> + * so we can use the buffer again. > >> + */ > >> + pkt->data = ctx->buffer; > >> + pkt->size = bytes_written; > > > > This is very evil. Encoders should return refcounted packets and not > > rely on the generic code fixing stuff up for them. > > Agree. Call avcodec_default_get_encode_buffer() then memcpy the data to > the returned buffer. > Don't use ff_alloc_packet() as that function does basically the same > thing you're doing here with a growable non refcounted buffer. > > > > > Also, pointers from av_malloc() cannot be passed to av_realloc(). You > > need to allocate it with av_realloc() in the first place. > > Is this documented? afaik realloc() can be used with malloc'd pointers. > It will, i assume, also realloc it the first time you call it even if > you request the exact same amount of memory malloc already allocated. > But in any case it's hardly a problem if he can just use av_realloc the > first time. realloc can work with malloc, but thats not guaranteed with every form of aligned memory allocator. That said, I don't think any platform where this is currently a problem is known, and we likely have this issue in a few areas of the code. The documentation of this limitation was removed in 2016 (21f70940ae106bfffa07e73057cdb4b5e81a767a) - Hendrik _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".