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 BB50742589 for ; Wed, 5 Jan 2022 03:57:55 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 6888B68A84E; Wed, 5 Jan 2022 05:57:53 +0200 (EET) Received: from w4.tutanota.de (w4.tutanota.de [81.3.6.165]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 3D5B6689988 for ; Wed, 5 Jan 2022 05:57:47 +0200 (EET) Received: from w3.tutanota.de (unknown [192.168.1.164]) by w4.tutanota.de (Postfix) with ESMTP id E08E5106026C for ; Wed, 5 Jan 2022 03:57:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; t=1641355066; s=s1; d=lynne.ee; h=From:From:To:To:Subject:Subject:Content-Description:Content-ID:Content-Type:Content-Type:Content-Transfer-Encoding:Content-Transfer-Encoding:Cc:Date:Date:In-Reply-To:In-Reply-To:MIME-Version:MIME-Version:Message-ID:Message-ID:Reply-To:References:References:Sender; bh=XwHE27pXeroVV+awYPS4Vxdu6WThwHdgt7JFLEKGKzM=; b=01L0+mT+eJuiMZIX8BlXbmqvcNsux2BDlDdUqBqbq8hTrJh68kgz+EEix0g27s+l NOQWqVM0cfZ6hchLJT/5tv35VjSOLqqxdS5JhGKp74Qw3tHGoNqCOU/MwNE8zibOhvm 8duEI2SSvW9e0obfzPNGW1bZddfcPqQU7ilRJeIm1E8Vn5V632fjyZ2jTdbK/KmCMfW I9jYUCMuH0WX8kV1nIflKSn9qsTAjxUrdsFrnhTKrSrKtOtwIO7SiV+dPLPYVh+X7PZ i32cs3mS2yjQQwutLoo7kOqE+dxjCWEQ7Yi0d5pmWNiXZYUG0d+Eub4qTVBifkJTKdb vBIfLlbedg== Date: Wed, 5 Jan 2022 04:57:46 +0100 (CET) From: Lynne To: FFmpeg development discussions and patches Message-ID: In-Reply-To: <20220103202608.56364-1-mvanb1@gmail.com> References: <20211219205318.9362-1-mvanb1@gmail.com> <20211220081156.12515-1-mvanb1@gmail.com> <20220103202608.56364-1-mvanb1@gmail.com> MIME-Version: 1.0 Subject: Re: [FFmpeg-devel] [PATCH v3] libavcodec/flacenc: add backward-compatible 32 bit-per-sample capability 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: 3 Jan 2022, 21:26 by mvanb1@gmail.com: > + if (pmax > 0) { > + if (lpc_reduction_tries >= 2) > + return 0; > + lpc_reduction_tries++; > + for (int i = 0; i < order; i++) > + coefs[i] = ((int64_t)coefs[i] * INT32_MAX) / pmax; > + } > + } while (pmax > 0); > + return 1; > +} > Could you invert the return value of the function? Return 0 if there's no overflow and 1 if there is? That's usually how we use return values throughout the code too, and it saves you from having to invert it when you use it. > static void flac_lpc_16_c(int32_t *decoded, const int coeffs[32], > int pred_order, int qlevel, int len) > { > diff --git a/libavcodec/flacdsp.h b/libavcodec/flacdsp.h > index 7bb0dd0e9a..5978a4722a 100644 > --- a/libavcodec/flacdsp.h > +++ b/libavcodec/flacdsp.h > @@ -40,4 +40,7 @@ void ff_flacdsp_init(FLACDSPContext *c, enum AVSampleFormat fmt, int channels, i > void ff_flacdsp_init_arm(FLACDSPContext *c, enum AVSampleFormat fmt, int channels, int bps); > void ff_flacdsp_init_x86(FLACDSPContext *c, enum AVSampleFormat fmt, int channels, int bps); > > +int ff_flacdsp_lpc_encode_c_32_overflow_detect(int32_t *res, const int32_t *smp, int len, > + int order, int32_t *coefs, int shift); > Could you also move the function to flacenc.c? The flacdsp.c file gets compiled if either is enabled, but if flacenc.c isn't enabled, then this code shouldn't be enabled, to save space. The function isn't likely to get SIMD, and the compiler could do more aggressive optimizations if it's in the same file. > put_sbits(&s->pb, sub->obits, res[0]); > } else if (sub->type == FLAC_SUBFRAME_VERBATIM) { > - while (res < frame_end) > - put_sbits(&s->pb, sub->obits, *res++); > + if (sub->obits < 32) { > + while (res < frame_end) > + put_sbits(&s->pb, sub->obits, *res++); > + } else { > + while (res < frame_end) > + put_bits32(&s->pb, *res++); > + } > } else { > /* warm-up samples */ > - for (i = 0; i < sub->order; i++) > - put_sbits(&s->pb, sub->obits, *res++); > + if (sub->obits < 32) { > + for (i = 0; i < sub->order; i++) > + put_sbits(&s->pb, sub->obits, *res++); > + }else{ > + for (i = 0; i < sub->order; i++) > + put_bits32(&s->pb, *res++); > + } > Small nit, but could you put spaces in the }else{ Apart from that, looks good. _______________________________________________ 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".