From patchwork Sun Apr 5 20:32:40 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Anton Khirnov X-Patchwork-Id: 18687 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 78F92448725 for ; Sun, 5 Apr 2020 23:33:00 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 6451368B16A; Sun, 5 Apr 2020 23:33:00 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail.red.khirnov.net (red.khirnov.net [176.97.15.12]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 42F5668B132 for ; Sun, 5 Apr 2020 23:32:52 +0300 (EEST) Received: from localhost (localhost [IPv6:::1]) by mail.red.khirnov.net (Postfix) with ESMTP id EFA57286899 for ; Sun, 5 Apr 2020 22:32:51 +0200 (CEST) Received: from mail.red.khirnov.net ([IPv6:::1]) by localhost (mail.red.khirnov.net [IPv6:::1]) (amavisd-new, port 10024) with ESMTP id PA-xtitdu1ov for ; Sun, 5 Apr 2020 22:32:50 +0200 (CEST) Received: from quelana.khirnov.net (unknown [IPv6:2a00:c500:61:23b::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "quelana.khirnov.net", Issuer "smtp.khirnov.net SMTP CA" (verified OK)) by mail.red.khirnov.net (Postfix) with ESMTPS id D71DD28666E for ; Sun, 5 Apr 2020 22:32:50 +0200 (CEST) Received: from localhost (quelana.khirnov.net [IPv6:::1]) by quelana.khirnov.net (Postfix) with ESMTP id 1A9BB213CC for ; Sun, 5 Apr 2020 22:32:49 +0200 (CEST) Received: from quelana.khirnov.net ([IPv6:::1]) by localhost (quelana.khirnov.net [IPv6:::1]) (amavisd-new, port 10024) with ESMTP id xU_DI5FHNp46 for ; Sun, 5 Apr 2020 22:32:46 +0200 (CEST) Received: from libav.daenerys.khirnov.net (libav.daenerys.khirnov.net [IPv6:2a00:c500:561:201::7]) by quelana.khirnov.net (Postfix) with ESMTP id BD630214ED for ; Sun, 5 Apr 2020 22:32:46 +0200 (CEST) Received: by libav.daenerys.khirnov.net (Postfix, from userid 1000) id DBCFC20E0012; Sun, 5 Apr 2020 22:32:44 +0200 (CEST) From: Anton Khirnov To: ffmpeg-devel@ffmpeg.org Date: Sun, 5 Apr 2020 22:32:40 +0200 Message-Id: <20200405203241.13033-3-anton@khirnov.net> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200405203241.13033-1-anton@khirnov.net> References: <20200405203241.13033-1-anton@khirnov.net> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 3/4] wavpack: fully support stream parameter changes 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" Fix invalid memory access on DSD streams with changing channel count. --- libavcodec/wavpack.c | 122 +++++++++++++++++++++++++++++++------------ 1 file changed, 90 insertions(+), 32 deletions(-) diff --git a/libavcodec/wavpack.c b/libavcodec/wavpack.c index b27262b94e..9cc4104dd0 100644 --- a/libavcodec/wavpack.c +++ b/libavcodec/wavpack.c @@ -20,6 +20,7 @@ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA */ +#include "libavutil/buffer.h" #include "libavutil/channel_layout.h" #define BITSTREAM_READER_LE @@ -109,7 +110,10 @@ typedef struct WavpackContext { AVFrame *frame; ThreadFrame curr_frame, prev_frame; Modulation modulation; + + AVBufferRef *dsd_ref; DSDContext *dsdctx; + int dsd_channels; } WavpackContext; #define LEVEL_DECAY(a) (((a) + 0x80) >> 8) @@ -978,6 +982,32 @@ static av_cold int wv_alloc_frame_context(WavpackContext *c) return 0; } +static int wv_dsd_reset(WavpackContext *s, int channels) +{ + int i; + + s->dsdctx = NULL; + s->dsd_channels = 0; + av_buffer_unref(&s->dsd_ref); + + if (!channels) + return 0; + + if (channels > INT_MAX / sizeof(*s->dsdctx)) + return AVERROR(EINVAL); + + s->dsd_ref = av_buffer_allocz(channels * sizeof(*s->dsdctx)); + if (!s->dsd_ref) + return AVERROR(ENOMEM); + s->dsdctx = (DSDContext*)s->dsd_ref->data; + s->dsd_channels = channels; + + for (i = 0; i < channels; i++) + memset(s->dsdctx[i].buf, 0x69, sizeof(s->dsdctx[i].buf)); + + return 0; +} + #if HAVE_THREADS static int init_thread_copy(AVCodecContext *avctx) { @@ -1008,6 +1038,17 @@ static int update_thread_context(AVCodecContext *dst, const AVCodecContext *src) return ret; } + av_buffer_unref(&fdst->dsd_ref); + fdst->dsdctx = NULL; + fdst->dsd_channels = 0; + if (fsrc->dsd_ref) { + fdst->dsd_ref = av_buffer_ref(fsrc->dsd_ref); + if (!fdst->dsd_ref) + return AVERROR(ENOMEM); + fdst->dsdctx = (DSDContext*)fdst->dsd_ref->data; + fdst->dsd_channels = fsrc->dsd_channels; + } + return 0; } #endif @@ -1025,15 +1066,9 @@ static av_cold int wavpack_decode_init(AVCodecContext *avctx) s->curr_frame.f = av_frame_alloc(); s->prev_frame.f = av_frame_alloc(); - // the DSD to PCM context is shared (and used serially) between all decoding threads - s->dsdctx = av_calloc(avctx->channels, sizeof(DSDContext)); - - if (!s->curr_frame.f || !s->prev_frame.f || !s->dsdctx) + if (!s->curr_frame.f || !s->prev_frame.f) return AVERROR(ENOMEM); - for (int i = 0; i < avctx->channels; i++) - memset(s->dsdctx[i].buf, 0x69, sizeof(s->dsdctx[i].buf)); - ff_init_dsd_data(); return 0; @@ -1053,8 +1088,7 @@ static av_cold int wavpack_decode_end(AVCodecContext *avctx) ff_thread_release_buffer(avctx, &s->prev_frame); av_frame_free(&s->prev_frame.f); - if (!avctx->internal->is_copy) - av_freep(&s->dsdctx); + av_buffer_unref(&s->dsd_ref); return 0; } @@ -1065,6 +1099,7 @@ static int wavpack_decode_block(AVCodecContext *avctx, int block_no, WavpackContext *wc = avctx->priv_data; WavpackFrameContext *s; GetByteContext gb; + enum AVSampleFormat sample_fmt; void *samples_l = NULL, *samples_r = NULL; int ret; int got_terms = 0, got_weights = 0, got_samples = 0, @@ -1102,7 +1137,15 @@ static int wavpack_decode_block(AVCodecContext *avctx, int block_no, return AVERROR_INVALIDDATA; } s->frame_flags = bytestream2_get_le32(&gb); - bpp = av_get_bytes_per_sample(avctx->sample_fmt); + + if (s->frame_flags & (WV_FLOAT_DATA | WV_DSD_DATA)) + sample_fmt = AV_SAMPLE_FMT_FLTP; + else if ((s->frame_flags & 0x03) <= 1) + sample_fmt = AV_SAMPLE_FMT_S16P; + else + sample_fmt = AV_SAMPLE_FMT_S32P; + + bpp = av_get_bytes_per_sample(sample_fmt); orig_bpp = ((s->frame_flags & 0x03) + 1) << 3; multiblock = (s->frame_flags & WV_SINGLE_BLOCK) != WV_SINGLE_BLOCK; @@ -1436,11 +1479,11 @@ static int wavpack_decode_block(AVCodecContext *avctx, int block_no, av_log(avctx, AV_LOG_ERROR, "Hybrid config not found\n"); return AVERROR_INVALIDDATA; } - if (!got_float && avctx->sample_fmt == AV_SAMPLE_FMT_FLTP) { + if (!got_float && sample_fmt == AV_SAMPLE_FMT_FLTP) { av_log(avctx, AV_LOG_ERROR, "Float information not found\n"); return AVERROR_INVALIDDATA; } - if (s->got_extra_bits && avctx->sample_fmt != AV_SAMPLE_FMT_FLTP) { + if (s->got_extra_bits && sample_fmt != AV_SAMPLE_FMT_FLTP) { const int size = get_bits_left(&s->gb_extra_bits); const int wanted = s->samples * s->extra_bits << s->stereo_in; if (size < wanted) { @@ -1462,27 +1505,54 @@ static int wavpack_decode_block(AVCodecContext *avctx, int block_no, } if (!wc->ch_offset) { + int new_channels = avctx->channels; + uint64_t new_chmask = avctx->channel_layout; + int new_samplerate; int sr = (s->frame_flags >> 23) & 0xf; if (sr == 0xf) { if (!sample_rate) { av_log(avctx, AV_LOG_ERROR, "Custom sample rate missing.\n"); return AVERROR_INVALIDDATA; } - avctx->sample_rate = sample_rate * rate_x; + new_samplerate = sample_rate * rate_x; } else - avctx->sample_rate = wv_rates[sr] * rate_x; + new_samplerate = wv_rates[sr] * rate_x; if (multiblock) { if (chan) - avctx->channels = chan; + new_channels = chan; if (chmask) - avctx->channel_layout = chmask; + new_chmask = chmask; } else { - avctx->channels = s->stereo ? 2 : 1; - avctx->channel_layout = s->stereo ? AV_CH_LAYOUT_STEREO : - AV_CH_LAYOUT_MONO; + new_channels = s->stereo ? 2 : 1; + new_chmask = s->stereo ? AV_CH_LAYOUT_STEREO : + AV_CH_LAYOUT_MONO; + } + + if (new_chmask && + av_get_channel_layout_nb_channels(new_chmask) != new_channels) { + av_log(avctx, AV_LOG_ERROR, "Channel mask does not match the channel count\n"); + return AVERROR_INVALIDDATA; } + /* clear DSD state if stream properties change */ + if (new_channels != wc->dsd_channels || + new_chmask != avctx->channel_layout || + new_samplerate != avctx->sample_rate || + !!got_dsd != !!wc->dsdctx) { + ret = wv_dsd_reset(wc, got_dsd ? new_channels : 0); + if (ret < 0) { + av_log(avctx, AV_LOG_ERROR, "Error reinitializing the DSD context\n"); + return ret; + } + ff_thread_release_buffer(avctx, &wc->curr_frame); + } + avctx->channels = new_channels; + avctx->channel_layout = new_chmask; + avctx->sample_rate = new_samplerate; + avctx->sample_fmt = sample_fmt; + avctx->bits_per_raw_sample = orig_bpp; + ff_thread_release_buffer(avctx, &wc->prev_frame); FFSWAP(ThreadFrame, wc->curr_frame, wc->prev_frame); @@ -1546,10 +1616,7 @@ static void wavpack_decode_flush(AVCodecContext *avctx) { WavpackContext *s = avctx->priv_data; - if (!avctx->internal->is_copy) { - for (int i = 0; i < avctx->channels; i++) - memset(s->dsdctx[i].buf, 0x69, sizeof(s->dsdctx[i].buf)); - } + wv_dsd_reset(s, 0); } static int dsd_channel(AVCodecContext *avctx, void *frmptr, int jobnr, int threadnr) @@ -1590,15 +1657,6 @@ static int wavpack_decode_frame(AVCodecContext *avctx, void *data, s->modulation = (frame_flags & WV_DSD_DATA) ? MODULATION_DSD : MODULATION_PCM; - if (frame_flags & (WV_FLOAT_DATA | WV_DSD_DATA)) { - avctx->sample_fmt = AV_SAMPLE_FMT_FLTP; - } else if ((frame_flags & 0x03) <= 1) { - avctx->sample_fmt = AV_SAMPLE_FMT_S16P; - } else { - avctx->sample_fmt = AV_SAMPLE_FMT_S32P; - avctx->bits_per_raw_sample = ((frame_flags & 0x03) + 1) << 3; - } - while (buf_size > WV_HEADER_SIZE) { frame_size = AV_RL32(buf + 4) - 12; buf += 20;