On 03/18/2012 03:43 AM, Kostya Shishkov wrote:
> Here's the new version (again with ralfdata.h cut out).
>
>
> 0001-RealAudio-Lossless-decoder.patch
>
>
>>From 5ca870c71f05f39008bc38ffaa94cf791421809e Mon Sep 17 00:00:00 2001
> From: Kostya Shishkov <[email protected]>
> Date: Sat, 17 Mar 2012 08:48:57 +0100
> Subject: [PATCH] RealAudio Lossless decoder
>
> ---
> Changelog | 1 +
> doc/general.texi | 1 +
> libavcodec/Makefile | 1 +
> libavcodec/allcodecs.c | 1 +
> libavcodec/avcodec.h | 1 +
> libavcodec/ralf.c | 536 +++
> libavcodec/ralfdata.h | 9920
> ++++++++++++++++++++++++++++++++++++++++++++++++
> libavformat/rm.c | 1 +
> libavformat/rmdec.c | 9 +
> 9 files changed, 10471 insertions(+), 0 deletions(-)
> create mode 100644 libavcodec/ralf.c
> create mode 100644 libavcodec/ralfdata.h
>
[...]
> +/**
> + * @file
> + * This is a decoder for Retarded Audio Lossless format.
> + * Dedicated to the mastermind behind it, Ralph Wiggum.
> + */
Please leave out the "Retarded" part.
> +
> +#include "avcodec.h"
> +#include "get_bits.h"
> +#include "golomb.h"
> +#include "unary.h"
> +#include "libavutil/audioconvert.h"
> +#include "ralfdata.h"
> +
> +#define FILTER_NONE 0
> +#define FILTER_RAW 642
> +
> +typedef struct VLCSet {
> + VLC filter_params;
> + VLC bias;
> + VLC coding_mode;
> + VLC filter_coeffs[10][11];
> + VLC short_codes[15];
> + VLC long_codes[125];
> +} VLCSet;
> +
> +#define RALF_MAX_PKT_SIZE 8192
> +
> +typedef struct RALFContext {
> + AVFrame frame;
> +
> + int version;
> + int max_frame_size;
> + VLCSet sets[3];
> + int channel_data[2][4096];
could you make that int32_t for more reliable size if we want to add asm
optimizations at some point?
> +
> + int filter_params;
> + int filter_length;
> + int filter_bits;
> + int filter[64];
> +
> + int bias[2];
> +
> + int num_blocks;
> + int sample_offset;
> + int block_size[1 << 12];
> + int block_pts[1 << 12];
> +
> + uint8_t pkt[16384];
> + int has_pkt;
> +} RALFContext;
would you mind documenting some of those fields?
[...]
> +static int decode_block(AVCodecContext *avctx, GetBitContext *gb, int16_t
> *dst)
> +{
> + RALFContext *ctx = avctx->priv_data;
> + int len, ch, ret;
> + int dmode, mode[2], bits[2];
> + int *ch0, *ch1;
> + int i, t, t2;
> +
> + len = 12 - get_unary(gb, 0, 6);
> +
> + if (len <= 7) len ^= 1; // codes for length = 6 and 7 are swapped
> + len = 1 << len;
> +
> + if (ctx->sample_offset + len > ctx->max_frame_size)
> + return AVERROR_INVALIDDATA;
maybe add some log message about sum of block durations exceeding the
maximum frame duration
> +
> + if (avctx->channels > 1)
> + dmode = get_bits(gb, 2) + 1;
> + else
> + dmode = 0;
> +
> + mode[0] = (dmode == 4) ? 1 : 0;
> + mode[1] = (dmode >= 2) ? 2 : 0;
> + bits[0] = 16;
> + bits[1] = (mode[1] == 2) ? 17 : 16;
> +
> + if (ctx->sample_offset + len > ctx->max_frame_size)
> + return -1;
didn't you just check that above?
[...]
> + if (ctx->has_pkt) {
> + ctx->has_pkt = 0;
> + table_bytes = (AV_RB16(avpkt->data) + 7) >> 3;
> + if (table_bytes + 3 > avpkt->size || avpkt->size >
> RALF_MAX_PKT_SIZE) {
> + av_log(avctx, AV_LOG_ERROR, "Wrong packet's breath smells of
> wrong data!\n");
> + return AVERROR_INVALIDDATA;
> + }
> + if (memcmp(ctx->pkt, avpkt->data, 2 + table_bytes)) {
> + av_log(avctx, AV_LOG_ERROR, "Wrong packets are wrong!\n");
> + return AVERROR_INVALIDDATA;
> + }
a better message might be something like "second partial packet does not
match the first. discarding packet."
> +
> + src = ctx->pkt;
> + src_size = RALF_MAX_PKT_SIZE + avpkt->size;
> + memcpy(ctx->pkt + RALF_MAX_PKT_SIZE, avpkt->data + 2 + table_bytes,
> + avpkt->size - 2 - table_bytes);
> + } else {
> + if (avpkt->size == RALF_MAX_PKT_SIZE) {
> + memcpy(ctx->pkt, avpkt->data, avpkt->size);
> + ctx->has_pkt = 1;
> + *got_frame_ptr = 0;
> +
> + return avpkt->size;
> + }
> + src = avpkt->data;
> + src_size = avpkt->size;
> + }
it doesn't look like you're validating that the packet is not larger
than RALF_MAX_PKT_SIZE in the single-packet case.
> +
> + ctx->frame.nb_samples = ctx->max_frame_size;
> + if ((ret = avctx->get_buffer(avctx, &ctx->frame)) < 0) {
> + av_log(avctx, AV_LOG_ERROR, "Me fail get_buffer()? That's
> unpossible!\n");
> + return ret;
> + }
> + samples = (int16_t*)ctx->frame.data[0];
> +
> + if (src_size < 5) {
> + av_log(avctx, AV_LOG_ERROR, "too short packets are too short!\n");
> + return AVERROR_INVALIDDATA;
> + }
> + table_size = AV_RB16(src);
> + table_bytes = (table_size + 7) >> 3;
> + if (src_size < table_bytes + 3) {
> + av_log(avctx, AV_LOG_ERROR, "short packets are short!\n");
> + return AVERROR_INVALIDDATA;
> + }
> + init_get_bits(&gb, src + 2, table_size);
> + ctx->num_blocks = 0;
> + while (get_bits_left(&gb) > 0) {
> + ctx->block_size[ctx->num_blocks] = get_bits(&gb, 15);
> + if (get_bits1(&gb)) {
> + ctx->block_pts[ctx->num_blocks] = get_bits(&gb, 9);
> + } else {
> + ctx->block_pts[ctx->num_blocks] = 0;
> + }
> + ctx->num_blocks++;
> + }
> +
> + block_pointer = src + table_bytes + 2;
> + bytes_left = src_size - table_bytes - 2;
> + ctx->sample_offset = 0;
> + for (i = 0; i < ctx->num_blocks; i++) {
> + if (bytes_left < ctx->block_size[i]) {
> + av_log(avctx, AV_LOG_ERROR, "I'm pedaling backwards\n");
> + break;
> + }
> + init_get_bits(&gb, block_pointer, ctx->block_size[i] * 8);
> + if (decode_block(avctx, &gb, samples + ctx->sample_offset
> + * avctx->channels) < 0) {
> + av_log(avctx, AV_LOG_ERROR, "Sir, I got carsick in your
> office\n");
This one definitely needs a better error message, especially since we're
returning truncated output instead of an error.
Cheers,
Justin
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
