Re: [FFmpeg-devel] [PATCH 3/4] wavpack: fully support stream parameter changes
On 4/5/20 1:32 PM, Anton Khirnov wrote:
> 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 !=
[FFmpeg-devel] [PATCH 3/4] wavpack: fully support stream parameter changes
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
