On Wednesday 06 May 2015 09:27:22 am Carl Eugen Hoyos wrote: > Hi! > > Attached is my attempt to fix ticket #4421 based on the > analysis by trac user lvqcl. If the patch is ok,
> I will add the encoder check to the decoder and > the version bump. I failed to implement an autodetection. Implementation of the option to force buggy decoding was less straightforward than expected, two patches attached. Please review, Carl Eugen
From b48b02f32538272b20abf99daf2fc5ce9fc26a48 Mon Sep 17 00:00:00 2001 From: Carl Eugen Hoyos <ceho...@ag.or.at> Date: Tue, 12 May 2015 12:47:57 +0200 Subject: [PATCH 1/2] lavc/flacdec: Sanitize FLACSTREAMINFO usage. --- libavcodec/flacdec.c | 83 ++++++++++++++++++++++++++++---------------------- 1 file changed, 46 insertions(+), 37 deletions(-) diff --git a/libavcodec/flacdec.c b/libavcodec/flacdec.c index 34a0a70..00b4726 100644 --- a/libavcodec/flacdec.c +++ b/libavcodec/flacdec.c @@ -48,7 +48,7 @@ typedef struct FLACContext { - FLACSTREAMINFO + struct FLACStreaminfo flac_stream_info; AVCodecContext *avctx; ///< parent AVCodecContext GetBitContext gb; ///< GetBitContext initialized to start at the current frame @@ -70,7 +70,7 @@ static int allocate_buffers(FLACContext *s); static void flac_set_bps(FLACContext *s) { enum AVSampleFormat req = s->avctx->request_sample_fmt; - int need32 = s->bps > 16; + int need32 = s->flac_stream_info.bps > 16; int want32 = av_get_bytes_per_sample(req) > 2; int planar = av_sample_fmt_is_planar(req); @@ -79,13 +79,13 @@ static void flac_set_bps(FLACContext *s) s->avctx->sample_fmt = AV_SAMPLE_FMT_S32P; else s->avctx->sample_fmt = AV_SAMPLE_FMT_S32; - s->sample_shift = 32 - s->bps; + s->sample_shift = 32 - s->flac_stream_info.bps; } else { if (planar) s->avctx->sample_fmt = AV_SAMPLE_FMT_S16P; else s->avctx->sample_fmt = AV_SAMPLE_FMT_S16; - s->sample_shift = 16 - s->bps; + s->sample_shift = 16 - s->flac_stream_info.bps; } } @@ -106,12 +106,13 @@ static av_cold int flac_decode_init(AVCodecContext *avctx) return AVERROR_INVALIDDATA; /* initialize based on the demuxer-supplied streamdata header */ - ff_flac_parse_streaminfo(avctx, (FLACStreaminfo *)s, streaminfo); + ff_flac_parse_streaminfo(avctx, &s->flac_stream_info, streaminfo); ret = allocate_buffers(s); if (ret < 0) return ret; flac_set_bps(s); - ff_flacdsp_init(&s->dsp, avctx->sample_fmt, s->channels, s->bps); + ff_flacdsp_init(&s->dsp, avctx->sample_fmt, + s->flac_stream_info.channels, s->flac_stream_info.bps); s->got_streaminfo = 1; return 0; @@ -131,9 +132,10 @@ static int allocate_buffers(FLACContext *s) int buf_size; int ret; - av_assert0(s->max_blocksize); + av_assert0(s->flac_stream_info.max_blocksize); - buf_size = av_samples_get_buffer_size(NULL, s->channels, s->max_blocksize, + buf_size = av_samples_get_buffer_size(NULL, s->flac_stream_info.channels, + s->flac_stream_info.max_blocksize, AV_SAMPLE_FMT_S32P, 0); if (buf_size < 0) return buf_size; @@ -143,8 +145,10 @@ static int allocate_buffers(FLACContext *s) return AVERROR(ENOMEM); ret = av_samples_fill_arrays((uint8_t **)s->decoded, NULL, - s->decoded_buffer, s->channels, - s->max_blocksize, AV_SAMPLE_FMT_S32P, 0); + s->decoded_buffer, + s->flac_stream_info.channels, + s->flac_stream_info.max_blocksize, + AV_SAMPLE_FMT_S32P, 0); return ret < 0 ? ret : 0; } @@ -168,12 +172,13 @@ static int parse_streaminfo(FLACContext *s, const uint8_t *buf, int buf_size) metadata_size != FLAC_STREAMINFO_SIZE) { return AVERROR_INVALIDDATA; } - ff_flac_parse_streaminfo(s->avctx, (FLACStreaminfo *)s, &buf[8]); + ff_flac_parse_streaminfo(s->avctx, &s->flac_stream_info, &buf[8]); ret = allocate_buffers(s); if (ret < 0) return ret; flac_set_bps(s); - ff_flacdsp_init(&s->dsp, s->avctx->sample_fmt, s->channels, s->bps); + ff_flacdsp_init(&s->dsp, s->avctx->sample_fmt, + s->flac_stream_info.channels, s->flac_stream_info.bps); s->got_streaminfo = 1; return 0; @@ -347,7 +352,7 @@ static inline int decode_subframe(FLACContext *s, int channel) { int32_t *decoded = s->decoded[channel]; int type, wasted = 0; - int bps = s->bps; + int bps = s->flac_stream_info.bps; int i, tmp, ret; if (channel == 0) { @@ -421,66 +426,69 @@ static int decode_frame(FLACContext *s) return ret; } - if (s->channels && fi.channels != s->channels && s->got_streaminfo) { - s->channels = s->avctx->channels = fi.channels; + if ( s->flac_stream_info.channels + && fi.channels != s->flac_stream_info.channels + && s->got_streaminfo) { + s->flac_stream_info.channels = s->avctx->channels = fi.channels; ff_flac_set_channel_layout(s->avctx); ret = allocate_buffers(s); if (ret < 0) return ret; } - s->channels = s->avctx->channels = fi.channels; + s->flac_stream_info.channels = s->avctx->channels = fi.channels; if (!s->avctx->channel_layout) ff_flac_set_channel_layout(s->avctx); s->ch_mode = fi.ch_mode; - if (!s->bps && !fi.bps) { + if (!s->flac_stream_info.bps && !fi.bps) { av_log(s->avctx, AV_LOG_ERROR, "bps not found in STREAMINFO or frame header\n"); return AVERROR_INVALIDDATA; } if (!fi.bps) { - fi.bps = s->bps; - } else if (s->bps && fi.bps != s->bps) { + fi.bps = s->flac_stream_info.bps; + } else if (s->flac_stream_info.bps && fi.bps != s->flac_stream_info.bps) { av_log(s->avctx, AV_LOG_ERROR, "switching bps mid-stream is not " "supported\n"); return AVERROR_INVALIDDATA; } - if (!s->bps) { - s->bps = s->avctx->bits_per_raw_sample = fi.bps; + if (!s->flac_stream_info.bps) { + s->flac_stream_info.bps = s->avctx->bits_per_raw_sample = fi.bps; flac_set_bps(s); } - if (!s->max_blocksize) - s->max_blocksize = FLAC_MAX_BLOCKSIZE; - if (fi.blocksize > s->max_blocksize) { + if (!s->flac_stream_info.max_blocksize) + s->flac_stream_info.max_blocksize = FLAC_MAX_BLOCKSIZE; + if (fi.blocksize > s->flac_stream_info.max_blocksize) { av_log(s->avctx, AV_LOG_ERROR, "blocksize %d > %d\n", fi.blocksize, - s->max_blocksize); + s->flac_stream_info.max_blocksize); return AVERROR_INVALIDDATA; } s->blocksize = fi.blocksize; - if (!s->samplerate && !fi.samplerate) { + if (!s->flac_stream_info.samplerate && !fi.samplerate) { av_log(s->avctx, AV_LOG_ERROR, "sample rate not found in STREAMINFO" " or frame header\n"); return AVERROR_INVALIDDATA; } if (fi.samplerate == 0) - fi.samplerate = s->samplerate; - s->samplerate = s->avctx->sample_rate = fi.samplerate; + fi.samplerate = s->flac_stream_info.samplerate; + s->flac_stream_info.samplerate = s->avctx->sample_rate = fi.samplerate; if (!s->got_streaminfo) { ret = allocate_buffers(s); if (ret < 0) return ret; s->got_streaminfo = 1; - dump_headers(s->avctx, (FLACStreaminfo *)s); + dump_headers(s->avctx, &s->flac_stream_info); } - ff_flacdsp_init(&s->dsp, s->avctx->sample_fmt, s->channels, s->bps); + ff_flacdsp_init(&s->dsp, s->avctx->sample_fmt, + s->flac_stream_info.channels, s->flac_stream_info.bps); -// dump_headers(s->avctx, (FLACStreaminfo *)s); +// dump_headers(s->avctx, &s->flac_stream_info); /* subframes */ - for (i = 0; i < s->channels; i++) { + for (i = 0; i < s->flac_stream_info.channels; i++) { if ((ret = decode_subframe(s, i)) < 0) return ret; } @@ -506,9 +514,9 @@ static int flac_decode_frame(AVCodecContext *avctx, void *data, *got_frame_ptr = 0; - if (s->max_framesize == 0) { - s->max_framesize = - ff_flac_get_max_frame_size(s->max_blocksize ? s->max_blocksize : FLAC_MAX_BLOCKSIZE, + if (s->flac_stream_info.max_framesize == 0) { + s->flac_stream_info.max_framesize = + ff_flac_get_max_frame_size(s->flac_stream_info.max_blocksize ? s->flac_stream_info.max_blocksize : FLAC_MAX_BLOCKSIZE, FLAC_MAX_CHANNELS, 32); } @@ -559,7 +567,8 @@ static int flac_decode_frame(AVCodecContext *avctx, void *data, if ((ret = ff_thread_get_buffer(avctx, &tframe, 0)) < 0) return ret; - s->dsp.decorrelate[s->ch_mode](frame->data, s->decoded, s->channels, + s->dsp.decorrelate[s->ch_mode](frame->data, s->decoded, + s->flac_stream_info.channels, s->blocksize, s->sample_shift); if (bytes_read > buf_size) { @@ -582,7 +591,7 @@ static int init_thread_copy(AVCodecContext *avctx) s->decoded_buffer = NULL; s->decoded_buffer_size = 0; s->avctx = avctx; - if (s->max_blocksize) + if (s->flac_stream_info.max_blocksize) return allocate_buffers(s); return 0; } -- 1.7.10.4
From 1232ed4821dfbb2f539038248f6be72f0399c394 Mon Sep 17 00:00:00 2001 From: Carl Eugen Hoyos <ceho...@ag.or.at> Date: Tue, 12 May 2015 13:00:29 +0200 Subject: [PATCH 2/2] lavc/flac: Fix encoding and decoding with high lpc. Based on an analysis by trac user lvqcl. Fixes ticket #4421, reported by Chase Walker. --- doc/decoders.texi | 17 +++++++++++++++++ libavcodec/arm/flacdsp_init_arm.c | 4 ++-- libavcodec/flacdec.c | 24 +++++++++++++++++++++++- libavcodec/flacdsp.c | 11 ++++------- libavcodec/flacdsp.h | 12 ++++++++---- libavcodec/flacenc.c | 27 ++++++++++++++++++++++----- libavcodec/version.h | 2 +- libavcodec/x86/flacdsp_init.c | 10 ++++------ 8 files changed, 81 insertions(+), 26 deletions(-) diff --git a/doc/decoders.texi b/doc/decoders.texi index 01fca9f..68196cf 100644 --- a/doc/decoders.texi +++ b/doc/decoders.texi @@ -83,6 +83,23 @@ Loud sounds are fully compressed. Soft sounds are enhanced. @end table +@section flac + +FLAC audio decoder. + +This decoder aims to implement the complete FLAC specification from Xiph. + +@subsection FLAC Decoder options + +@table @option + +@item -use_buggy_lpc +The lavc FLAC encoder used to produce buggy streams with high lpc values +(like the default value). This option allows to decode such streams +correctly by using lavc's old buggy lpc logic for decoding. + +@end table + @section ffwavesynth Internal wave synthetizer. diff --git a/libavcodec/arm/flacdsp_init_arm.c b/libavcodec/arm/flacdsp_init_arm.c index 82a807f..564e3dc 100644 --- a/libavcodec/arm/flacdsp_init_arm.c +++ b/libavcodec/arm/flacdsp_init_arm.c @@ -27,6 +27,6 @@ void ff_flac_lpc_16_arm(int32_t *samples, const int coeffs[32], int order, av_cold void ff_flacdsp_init_arm(FLACDSPContext *c, enum AVSampleFormat fmt, int channels, int bps) { - if (CONFIG_FLAC_DECODER && bps <= 16) - c->lpc = ff_flac_lpc_16_arm; + if (CONFIG_FLAC_DECODER) + c->lpc16 = ff_flac_lpc_16_arm; } diff --git a/libavcodec/flacdec.c b/libavcodec/flacdec.c index 00b4726..36d2928 100644 --- a/libavcodec/flacdec.c +++ b/libavcodec/flacdec.c @@ -35,6 +35,7 @@ #include "libavutil/avassert.h" #include "libavutil/crc.h" +#include "libavutil/opt.h" #include "avcodec.h" #include "internal.h" #include "get_bits.h" @@ -48,6 +49,7 @@ typedef struct FLACContext { + AVClass *class; struct FLACStreaminfo flac_stream_info; AVCodecContext *avctx; ///< parent AVCodecContext @@ -61,6 +63,7 @@ typedef struct FLACContext { int32_t *decoded[FLAC_MAX_CHANNELS]; ///< decoded samples uint8_t *decoded_buffer; unsigned int decoded_buffer_size; + int buggy_lpc; ///< use workaround for old lavc encoded files FLACDSPContext dsp; } FLACContext; @@ -343,7 +346,13 @@ static int decode_subframe_lpc(FLACContext *s, int32_t *decoded, int pred_order, if ((ret = decode_residuals(s, decoded, pred_order)) < 0) return ret; - s->dsp.lpc(decoded, coeffs, pred_order, qlevel, s->blocksize); + if ( ( s->buggy_lpc && s->flac_stream_info.bps <= 16) + || ( !s->buggy_lpc && bps <= 16 + && bps + coeff_prec + av_log2(pred_order) <= 32)) { + s->dsp.lpc16(decoded, coeffs, pred_order, qlevel, s->blocksize); + } else { + s->dsp.lpc32(decoded, coeffs, pred_order, qlevel, s->blocksize); + } return 0; } @@ -605,6 +614,18 @@ static av_cold int flac_decode_close(AVCodecContext *avctx) return 0; } +static const AVOption options[] = { +{ "use_buggy_lpc", "emulate old buggy lavc behavior", offsetof(FLACContext, buggy_lpc), AV_OPT_TYPE_INT, {.i64 = 0 }, 0, 1, AV_OPT_FLAG_DECODING_PARAM | AV_OPT_FLAG_AUDIO_PARAM }, +{ NULL }, +}; + +static const AVClass flac_decoder_class = { + "FLAC decoder", + av_default_item_name, + options, + LIBAVUTIL_VERSION_INT, +}; + AVCodec ff_flac_decoder = { .name = "flac", .long_name = NULL_IF_CONFIG_SMALL("FLAC (Free Lossless Audio Codec)"), @@ -621,4 +642,5 @@ AVCodec ff_flac_decoder = { AV_SAMPLE_FMT_S32, AV_SAMPLE_FMT_S32P, AV_SAMPLE_FMT_NONE }, + .priv_class = &flac_decoder_class, }; diff --git a/libavcodec/flacdsp.c b/libavcodec/flacdsp.c index a83eb83..30b6648 100644 --- a/libavcodec/flacdsp.c +++ b/libavcodec/flacdsp.c @@ -88,13 +88,10 @@ static void flac_lpc_32_c(int32_t *decoded, const int coeffs[32], av_cold void ff_flacdsp_init(FLACDSPContext *c, enum AVSampleFormat fmt, int channels, int bps) { - if (bps > 16) { - c->lpc = flac_lpc_32_c; - c->lpc_encode = flac_lpc_encode_c_32; - } else { - c->lpc = flac_lpc_16_c; - c->lpc_encode = flac_lpc_encode_c_16; - } + c->lpc16 = flac_lpc_16_c; + c->lpc32 = flac_lpc_32_c; + c->lpc16_encode = flac_lpc_encode_c_16; + c->lpc32_encode = flac_lpc_encode_c_32; switch (fmt) { case AV_SAMPLE_FMT_S32: diff --git a/libavcodec/flacdsp.h b/libavcodec/flacdsp.h index 417381c..f5cbd94 100644 --- a/libavcodec/flacdsp.h +++ b/libavcodec/flacdsp.h @@ -25,10 +25,14 @@ typedef struct FLACDSPContext { void (*decorrelate[4])(uint8_t **out, int32_t **in, int channels, int len, int shift); - void (*lpc)(int32_t *samples, const int coeffs[32], int order, - int qlevel, int len); - void (*lpc_encode)(int32_t *res, const int32_t *smp, int len, int order, - const int32_t coefs[32], int shift); + void (*lpc16)(int32_t *samples, const int coeffs[32], int order, + int qlevel, int len); + void (*lpc32)(int32_t *samples, const int coeffs[32], int order, + int qlevel, int len); + void (*lpc16_encode)(int32_t *res, const int32_t *smp, int len, int order, + const int32_t coefs[32], int shift); + void (*lpc32_encode)(int32_t *res, const int32_t *smp, int len, int order, + const int32_t coefs[32], int shift); } FLACDSPContext; void ff_flacdsp_init(FLACDSPContext *c, enum AVSampleFormat fmt, int channels, int bps); diff --git a/libavcodec/flacenc.c b/libavcodec/flacenc.c index bc6d00a..b6dc4d5 100644 --- a/libavcodec/flacenc.c +++ b/libavcodec/flacenc.c @@ -838,8 +838,13 @@ static int encode_residual_ch(FlacEncodeContext *s, int ch) order = av_clip(order, min_order - 1, max_order - 1); if (order == last_order) continue; - s->flac_dsp.lpc_encode(res, smp, n, order+1, coefs[order], - shift[order]); + if (s->bps_code * 4 + s->options.lpc_coeff_precision + av_log2(order) <= 32) { + s->flac_dsp.lpc16_encode(res, smp, n, order+1, coefs[order], + shift[order]); + } else { + s->flac_dsp.lpc32_encode(res, smp, n, order+1, coefs[order], + shift[order]); + } bits[i] = find_subframe_rice_params(s, sub, order+1); if (bits[i] < bits[opt_index]) { opt_index = i; @@ -853,7 +858,11 @@ static int encode_residual_ch(FlacEncodeContext *s, int ch) opt_order = 0; bits[0] = UINT32_MAX; for (i = min_order-1; i < max_order; i++) { - s->flac_dsp.lpc_encode(res, smp, n, i+1, coefs[i], shift[i]); + if (s->bps_code * 4 + s->options.lpc_coeff_precision + av_log2(i) <= 32) { + s->flac_dsp.lpc16_encode(res, smp, n, i+1, coefs[i], shift[i]); + } else { + s->flac_dsp.lpc32_encode(res, smp, n, i+1, coefs[i], shift[i]); + } bits[i] = find_subframe_rice_params(s, sub, i+1); if (bits[i] < bits[opt_order]) opt_order = i; @@ -871,7 +880,11 @@ static int encode_residual_ch(FlacEncodeContext *s, int ch) for (i = last-step; i <= last+step; i += step) { if (i < min_order-1 || i >= max_order || bits[i] < UINT32_MAX) continue; - s->flac_dsp.lpc_encode(res, smp, n, i+1, coefs[i], shift[i]); + if (s->bps_code * 4 + s->options.lpc_coeff_precision + av_log2(i) <= 32) { + s->flac_dsp.lpc32_encode(res, smp, n, i+1, coefs[i], shift[i]); + } else { + s->flac_dsp.lpc16_encode(res, smp, n, i+1, coefs[i], shift[i]); + } bits[i] = find_subframe_rice_params(s, sub, i+1); if (bits[i] < bits[opt_order]) opt_order = i; @@ -886,7 +899,11 @@ static int encode_residual_ch(FlacEncodeContext *s, int ch) for (i = 0; i < sub->order; i++) sub->coefs[i] = coefs[sub->order-1][i]; - s->flac_dsp.lpc_encode(res, smp, n, sub->order, sub->coefs, sub->shift); + if (s->bps_code * 4 + s->options.lpc_coeff_precision + av_log2(opt_order) <= 32) { + s->flac_dsp.lpc16_encode(res, smp, n, sub->order, sub->coefs, sub->shift); + } else { + s->flac_dsp.lpc32_encode(res, smp, n, sub->order, sub->coefs, sub->shift); + } find_subframe_rice_params(s, sub, sub->order); diff --git a/libavcodec/version.h b/libavcodec/version.h index 1d0525a..75ee26d 100644 --- a/libavcodec/version.h +++ b/libavcodec/version.h @@ -30,7 +30,7 @@ #define LIBAVCODEC_VERSION_MAJOR 56 #define LIBAVCODEC_VERSION_MINOR 38 -#define LIBAVCODEC_VERSION_MICRO 100 +#define LIBAVCODEC_VERSION_MICRO 101 #define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \ LIBAVCODEC_VERSION_MINOR, \ diff --git a/libavcodec/x86/flacdsp_init.c b/libavcodec/x86/flacdsp_init.c index d04af45..e28c5c9 100644 --- a/libavcodec/x86/flacdsp_init.c +++ b/libavcodec/x86/flacdsp_init.c @@ -85,8 +85,7 @@ av_cold void ff_flacdsp_init_x86(FLACDSPContext *c, enum AVSampleFormat fmt, int } } if (EXTERNAL_SSE4(cpu_flags)) { - if (bps > 16) - c->lpc = ff_flac_lpc_32_sse4; + c->lpc32 = ff_flac_lpc_32_sse4; } if (EXTERNAL_AVX(cpu_flags)) { if (fmt == AV_SAMPLE_FMT_S16) { @@ -102,15 +101,14 @@ av_cold void ff_flacdsp_init_x86(FLACDSPContext *c, enum AVSampleFormat fmt, int } } if (EXTERNAL_XOP(cpu_flags)) { - if (bps > 16) - c->lpc = ff_flac_lpc_32_xop; + c->lpc32 = ff_flac_lpc_32_xop; } #endif #if CONFIG_FLAC_ENCODER if (EXTERNAL_SSE4(cpu_flags)) { - if (CONFIG_GPL && bps == 16) - c->lpc_encode = ff_flac_enc_lpc_16_sse4; + if (CONFIG_GPL) + c->lpc16_encode = ff_flac_enc_lpc_16_sse4; } #endif #endif /* HAVE_YASM */ -- 1.7.10.4
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel