[FFmpeg-devel] [PATCH] libavcodec: Implementation of AC3 fixed point decoder

[Nedeljko Babic]

>note, ive fixed some issues and split this patch up, please see
>the seperate thread about that
>

Thanks Michael!

>
>[...]
>> +static const AVOption options[] = {
>> +    { NULL},
>> +};
>[...]
>> +static const AVOption options[] = {
>> +    { "drc_scale", "percentage of dynamic range compression to apply", OFFSET(drc_scale), AV_OPT_TYPE_FLOAT, {.dbl = 1.0}, 0.0, 1.0, PAR },
>> +
>> +{"dmix_mode", "Preferred Stereo Downmix Mode", OFFSET(preferred_stereo_downmix), AV_OPT_TYPE_INT, {.i64 = -1 }, -1, 2, 0, "dmix_mode"},
>> +{"ltrt_cmixlev",   "Lt/Rt Center Mix Level",   OFFSET(ltrt_center_mix_level),    AV_OPT_TYPE_FLOAT, {.dbl = -1.0 }, -1.0, 2.0, 0},
>> +{"ltrt_surmixlev", "Lt/Rt Surround Mix Level", OFFSET(ltrt_surround_mix_level),  AV_OPT_TYPE_FLOAT, {.dbl = -1.0 }, -1.0, 2.0, 0},
>> +{"loro_cmixlev",   "Lo/Ro Center Mix Level",   OFFSET(loro_center_mix_level),    AV_OPT_TYPE_FLOAT, {.dbl = -1.0 }, -1.0, 2.0, 0},
>> +{"loro_surmixlev", "Lo/Ro Surround Mix Level", OFFSET(loro_surround_mix_level),  AV_OPT_TYPE_FLOAT, {.dbl = -1.0 }, -1.0, 2.0, 0},
>
>it appears these options are missing from the fixed point decoder
>is that intended ?
>

These options are not used in fixed point decoder so they are left out.

>
>
>[...]
>[...]
>> diff --git a/libavutil/fixed_dsp.h b/libavutil/fixed_dsp.h
>> new file mode 100644
>> index 0000000..85f5252
>> --- /dev/null
>> +++ b/libavutil/fixed_dsp.h
>> @@ -0,0 +1,123 @@
>> +/*
>> + * Copyright (c) 2012
>> + *      MIPS Technologies, Inc., California.
>> + *
>> + * Redistribution and use in source and binary forms, with or without
>> + * modification, are permitted provided that the following conditions
>> + * are met:
>> + * 1. Redistributions of source code must retain the above copyright
>> + *    notice, this list of conditions and the following disclaimer.
>> + * 2. Redistributions in binary form must reproduce the above copyright
>> + *    notice, this list of conditions and the following disclaimer in the
>> + *    documentation and/or other materials provided with the distribution.
>> + * 3. Neither the name of the MIPS Technologies, Inc., nor the names of its
>> + *    contributors may be used to endorse or promote products derived from
>> + *    this software without specific prior written permission.
>> + *
>> + * THIS SOFTWARE IS PROVIDED BY THE MIPS TECHNOLOGIES, INC. ``AS IS'' AND
>> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
>> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
>> + * ARE DISCLAIMED.  IN NO EVENT SHALL THE MIPS TECHNOLOGIES, INC. BE LIABLE
>> + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
>> + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
>> + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
>> + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
>> + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
>> + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
>> + * SUCH DAMAGE.
>> + *
>> + * Author:  Nedeljko Babic (nbabic at mips.com)
>> + *
>> + * This file is part of FFmpeg.
>> + *
>> + * FFmpeg is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2.1 of the License, or (at your option) any later version.
>> + *
>> + * FFmpeg is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with FFmpeg; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>> + */
>> +
>> +#ifndef AVUTIL_FIXED_DSP_H
>> +#define AVUTIL_FIXED_DSP_H
>> +
>> +#include <stdint.h>
>> +#include "attributes.h"
>> +#include "common.h"
>> +
>> +typedef struct AVFixedDSPContext {
>> +    /**
>> +     * Overlap/add with window function.
>> +     * Used primarily by MDCT-based audio codecs.
>> +     * Source and destination vectors must overlap exactly or not at all.
>> +     *
>> +     * @param dst  result vector
>> +     *             constraints: 16-byte aligned
>> +     * @param src0 first source vector
>> +     *             constraints: 16-byte aligned
>> +     * @param src1 second source vector
>> +     *             constraints: 16-byte aligned
>> +     * @param win  half-window vector
>> +     *             constraints: 16-byte aligned
>> +     * @param len  length of vector
>> +     *             constraints: multiple of 4
>> +     * @param bits scaling parameter
>> +     *
>> +     */
>> +    void (*vector_fmul_window_fixed_scaled)(int16_t *dst, const int32_t *src0, const int32_t *src1, const int32_t *win, int len, uint8_t bits);
>> +
>> +    /**
>> +     * Overlap/add with window function.
>> +     * Used primarily by MDCT-based audio codecs.
>> +     * Source and destination vectors must overlap exactly or not at all.
>> +     *
>> +     * @param dst  result vector
>> +     *             constraints: 32-byte aligned
>> +     * @param src0 first source vector
>> +     *             constraints: 16-byte aligned
>> +     * @param src1 second source vector
>> +     *             constraints: 16-byte aligned
>> +     * @param win  half-window vector
>> +     *             constraints: 16-byte aligned
>> +     * @param len  length of vector
>> +     *             constraints: multiple of 4
>> +     */
>> +    void (*vector_fmul_window_fixed)(int32_t *dst, const int32_t *src0, const int32_t *src1, const int32_t *win, int len);
>> +
>> +} AVFixedDSPContext;
>> +
>> +/**
>> + * Initialize a fixed DSP context.
>> + *
>> + * @param fdsp    fixed DSP context
>> + * @param strict  setting to non-zero avoids using functions which may not be IEEE-754 compliant
>> + */
>> +void avpriv_fixed_dsp_init(AVFixedDSPContext *fdsp, int strict);
>
>This API/ABI is not extendible, any field added to the struct requires
>a major version and soname bump, adding such API/ABI is not ok
>yes i know you copied that design from existing code, its not ok there
>either

We can make these functions stand alone and call them as such, but I guess
that this is bad idea also. Do you have a suggestion how should we treat them
correctly?


-Nedeljko