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 09B1842566 for ; Fri, 18 Mar 2022 15:21:54 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 59D7D68B0C3; Fri, 18 Mar 2022 17:21:52 +0200 (EET) Received: from mail-yb1-f182.google.com (mail-yb1-f182.google.com [209.85.219.182]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id A101468AAD7 for ; Fri, 18 Mar 2022 17:21:46 +0200 (EET) Received: by mail-yb1-f182.google.com with SMTP id u3so16429794ybh.5 for ; Fri, 18 Mar 2022 08:21:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:in-reply-to:references:from:date:message-id:subject:to; bh=nVCnc9ZzxYlIlKWZd9tP744wVXt2fcSPXpduzz73Tu0=; b=cVoujkhQ2uU1UDHymdEFk4XlivuDlQgXoBNQQ6KHovHshKs/07u9RBI0yC49n/QL6v TP8QGm4rclWqvdw7n6nYw8gKaI/F6LgTbI9388q7AgGbB9JbA/9I2T8zlNdJG70WZ0AG OA6yizvMjnLHZ2jvbaUIKCe5yBK6ALNVqRvLRKF8MtB1T1C84a+ReGJ3JxPrHqL0iThJ cAejfcUYkXkA+pKv6aiNp9B41+3tfiQYpf3vZnrvkkClp+WS8j+D47TS+5Cc29lGxrwV EVnuc6AnMlurx8HuhBbrp00Bg7Qe1KrJFsXziV5O/YdnNTjpOQSxr/cpoTsCuQdQbimM 6bkQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to; bh=nVCnc9ZzxYlIlKWZd9tP744wVXt2fcSPXpduzz73Tu0=; b=ygbtxbxWrJ038zF6SWJKAHTAA95F1kiqqr9p5x5xJg03mzqMzgiye4rVMvdgMDkUky 46qXoSGjbUrQkyW+oxJvXdMsVgKZa4pSg2U/0Eo65eAi/EkWg+e4QkcbbfZZ4wzldITj 53LtXkw88vWQ7WBm/jFyip711ZvgTXP1yzWU/FZBPE6THwcfCMLUOVwOyShTn16OiIe3 p+Lvhe5aZ9u4ccXJpmCU6nISqg88DvCE4sEiTsFOTVw2LLQZXu8iu6yzZhasmR3Zw0IH Xz26SufMru/TEjmjkIslB4r7sxky/rvXFbtg7Tnh3cRdABck2nYP1V2JdJHhduij6k4+ K+bw== X-Gm-Message-State: AOAM5306kFFJwbdUZayWLb4FreojkZjRIWsBhYRJroidS0FAyjG47KN3 VkYeSlXV1JyDZspHz6ndOOk4OqYRfT3MNTyZNNidvlPO X-Google-Smtp-Source: ABdhPJxP12/20WUp2JM9xK9Sa13ALYSJbc3JnPdsftAuV8g3me9xTsFBrn7fD+DaV8iAb3p7hJP6DON+OL9dVKuOAp8= X-Received: by 2002:a25:c60c:0:b0:633:7a2b:f78c with SMTP id k12-20020a25c60c000000b006337a2bf78cmr10591432ybf.250.1647616905018; Fri, 18 Mar 2022 08:21:45 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a05:690c:12:0:0:0:0 with HTTP; Fri, 18 Mar 2022 08:21:44 -0700 (PDT) In-Reply-To: References: <20220318130417.47935-1-onemda@gmail.com> From: Paul B Mahol Date: Fri, 18 Mar 2022 16:21:44 +0100 Message-ID: To: FFmpeg development discussions and patches Subject: Re: [FFmpeg-devel] [PATCH] avcodec/binkaudio: add support for >2 channels dct codec 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 3/18/22, Andreas Rheinhardt wrote: > Paul B Mahol: >> As presented in .binka files. >> >> Signed-off-by: Paul B Mahol >> --- >> libavcodec/binkaudio.c | 50 +++++++++++++++++++++++++++--------------- >> 1 file changed, 32 insertions(+), 18 deletions(-) >> >> diff --git a/libavcodec/binkaudio.c b/libavcodec/binkaudio.c >> index b4ff15beeb..54b7e22854 100644 >> --- a/libavcodec/binkaudio.c >> +++ b/libavcodec/binkaudio.c >> @@ -51,13 +51,14 @@ typedef struct BinkAudioContext { >> int version_b; ///< Bink version 'b' >> int first; >> int channels; >> + int ch_offset; >> int frame_len; ///< transform size (samples) >> int overlap_len; ///< overlap size (samples) >> int block_size; >> int num_bands; >> float root; >> unsigned int bands[26]; >> - float previous[MAX_CHANNELS][BINK_BLOCK_MAX_SIZE / 16]; ///< coeffs >> from previous audio block >> + float previous[6][BINK_BLOCK_MAX_SIZE / 16]; ///< coeffs from >> previous audio block >> float quant_table[96]; >> AVPacket *pkt; >> union { >> @@ -74,6 +75,7 @@ static av_cold int decode_init(AVCodecContext *avctx) >> int sample_rate_half; >> int i, ret; >> int frame_len_bits; >> + int max_channels = avctx->codec->id == AV_CODEC_ID_BINKAUDIO_RDFT ? >> MAX_CHANNELS : 6; > > If you allow up to six channels, then MAX_CHANNELS (i.e. two) needs to > be renamed. > >> int channels = avctx->ch_layout.nb_channels; >> >> /* determine frame length */ >> @@ -85,7 +87,7 @@ static av_cold int decode_init(AVCodecContext *avctx) >> frame_len_bits = 11; >> } >> >> - if (channels < 1 || channels > MAX_CHANNELS) { >> + if (channels < 1 || channels > max_channels) { >> av_log(avctx, AV_LOG_ERROR, "invalid number of channels: %d\n", >> channels); >> return AVERROR_INVALIDDATA; >> } >> @@ -110,7 +112,7 @@ static av_cold int decode_init(AVCodecContext *avctx) >> >> s->frame_len = 1 << frame_len_bits; >> s->overlap_len = s->frame_len / 16; >> - s->block_size = (s->frame_len - s->overlap_len) * s->channels; >> + s->block_size = (s->frame_len - s->overlap_len) * >> FFMIN(MAX_CHANNELS, s->channels); >> sample_rate_half = (sample_rate + 1LL) / 2; >> if (avctx->codec->id == AV_CODEC_ID_BINKAUDIO_RDFT) >> s->root = 2.0 / (sqrt(s->frame_len) * 32768.0); >> @@ -166,7 +168,8 @@ static const uint8_t rle_length_tab[16] = { >> * @param[out] out Output buffer (must contain s->block_size elements) >> * @return 0 on success, negative error code on failure >> */ >> -static int decode_block(BinkAudioContext *s, float **out, int use_dct) >> +static int decode_block(BinkAudioContext *s, float **out, int use_dct, >> + int channels, int ch_offset) >> { >> int ch, i, j, k; >> float q, quant[25]; >> @@ -176,8 +179,8 @@ static int decode_block(BinkAudioContext *s, float >> **out, int use_dct) >> if (use_dct) >> skip_bits(gb, 2); >> >> - for (ch = 0; ch < s->channels; ch++) { >> - FFTSample *coeffs = out[ch]; >> + for (ch = 0; ch < channels; ch++) { >> + FFTSample *coeffs = out[ch + ch_offset]; >> >> if (s->version_b) { >> if (get_bits_left(gb) < 64) >> @@ -252,17 +255,17 @@ static int decode_block(BinkAudioContext *s, float >> **out, int use_dct) >> s->trans.rdft.rdft_calc(&s->trans.rdft, coeffs); >> } >> >> - for (ch = 0; ch < s->channels; ch++) { >> + for (ch = 0; ch < channels; ch++) { >> int j; >> - int count = s->overlap_len * s->channels; >> + int count = s->overlap_len * channels; >> if (!s->first) { >> j = ch; >> - for (i = 0; i < s->overlap_len; i++, j += s->channels) >> - out[ch][i] = (s->previous[ch][i] * (count - j) + >> - out[ch][i] * j) / count; >> + for (i = 0; i < s->overlap_len; i++, j += channels) >> + out[ch + ch_offset][i] = (s->previous[ch + ch_offset][i] >> * (count - j) + >> + out[ch + ch_offset][i] * >> j) / count; >> } >> - memcpy(s->previous[ch], &out[ch][s->frame_len - s->overlap_len], >> - s->overlap_len * sizeof(*s->previous[ch])); >> + memcpy(s->previous[ch + ch_offset], &out[ch + >> ch_offset][s->frame_len - s->overlap_len], >> + s->overlap_len * sizeof(*s->previous[ch + ch_offset])); >> } >> >> s->first = 0; >> @@ -293,6 +296,7 @@ static int binkaudio_receive_frame(AVCodecContext >> *avctx, AVFrame *frame) >> GetBitContext *gb = &s->gb; >> int ret; >> >> +again: >> if (!s->pkt->data) { >> ret = ff_decode_get_packet(avctx, s->pkt); >> if (ret < 0) >> @@ -313,22 +317,31 @@ static int binkaudio_receive_frame(AVCodecContext >> *avctx, AVFrame *frame) >> } >> >> /* get output buffer */ >> - frame->nb_samples = s->frame_len; >> - if ((ret = ff_get_buffer(avctx, frame, 0)) < 0) >> - return ret; >> + if (s->ch_offset == 0) { >> + frame->nb_samples = s->frame_len; >> + if ((ret = ff_get_buffer(avctx, frame, 0)) < 0) >> + return ret; >> + } >> >> if (decode_block(s, (float **)frame->extended_data, >> - avctx->codec->id == AV_CODEC_ID_BINKAUDIO_DCT)) { >> + avctx->codec->id == AV_CODEC_ID_BINKAUDIO_DCT, >> + FFMIN(MAX_CHANNELS, s->channels), s->ch_offset)) { >> av_log(avctx, AV_LOG_ERROR, "Incomplete packet\n"); >> return AVERROR_INVALIDDATA; >> } >> + s->ch_offset += MAX_CHANNELS; >> get_bits_align32(gb); >> if (!get_bits_left(gb)) { >> memset(gb, 0, sizeof(*gb)); >> av_packet_unref(s->pkt); >> } >> + if (s->ch_offset >= s->channels) { >> + s->ch_offset = 0; >> + } else { >> + goto again; >> + } > > Is it really intended that the data for one multi-channel frame is > divided into several input packets? You are missing big picture here, >2 files have channels in different packets interleaved. Something like in XMA. (And nothing signals how are they interleaved. so its worse than in XMA) So it is working fine. I just need another look for possible regressions and security implications. Renaming MAX_CHANNELS is not useful as that is not property of both codecs. > >> >> - frame->nb_samples = s->block_size / avctx->ch_layout.nb_channels; >> + frame->nb_samples = s->block_size / >> FFMIN(avctx->ch_layout.nb_channels, MAX_CHANNELS); >> >> return 0; >> fail: >> @@ -343,6 +356,7 @@ static void decode_flush(AVCodecContext *avctx) >> /* s->pkt coincides with avctx->internal->in_pkt >> * and is unreferenced generically when flushing. */ >> s->first = 1; >> + s->ch_offset = 0; >> } >> >> const AVCodec ff_binkaudio_rdft_decoder = { > > _______________________________________________ > 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". > _______________________________________________ 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".