[FFmpeg-devel] [PATCH] QCELP decoder
Kenan Gillet
kenan.gillet
Thu Oct 30 20:48:08 CET 2008
On Oct 30, 2008, at 2:59 AM, Diego Biurrun wrote:
> On Tue, Oct 28, 2008 at 04:47:02PM -0700, Kenan Gillet wrote:
>> On Tue, Oct 28, 2008 at 4:43 PM, Kenan Gillet
>> <kenan.gillet at gmail.com> wrote:
>>> thanks Diego and Michael for your reviewing.
>>>
>>> And here is an updated set of patches with a few changes too.
>>>
>>> round 7 summary:
>>> - qcelp-round7-doc-glue.patch.txt has already been oked
>>> - grammar / spelling / cosmetics
>>> - generalize convolve function used in qcelp_lspf2lpc
>>> - remove some multiplication and division in compute_svector
>>> - frame unpacking rework
>>> - most of the previous comments
>>>
>>
>> forgot one patch in previous mail :(
>>
>> --- libavcodec/qcelpdata.h (revision 0)
>> +++ libavcodec/qcelpdata.h (revision 0)
>> @@ -0,0 +1,541 @@
>> +/**
>> + * pre-calculated table for hammsinc function
>> + * Only half of the tables is needed because of symetry.
>
> table or tables?
>
> syMMetry
done
>> + uint8_t bitpos; /*!< position of the lowet bit in the value's
>> byte */
>
> loweSt
done
>> +/**
>> + * the upper boundary of the clipping, depends on
>> + */
>> +#define QCELP_CLIP_UPPER_BOUND (8191.75/8192.)
>
> Depends on what?
done,
depends on QCELP_SCALE
>> --- libavcodec/qcelpdec.c (revision 0)
>> +++ libavcodec/qcelpdec.c (revision 0)
>> @@ -0,0 +1,723 @@
>> +/**
>> + * Decodes the 10 quantized LSP frequencies from the LSPV/LSP
>> + * transmission codes of any framerate and check for badly
>> received packets.
>
> checks
done
>> +/*
>> + * Determine the framerate from the frame size and/or the first
>> byte of the frame.
>> + *
>> + * @return 0 on success, negative error number otherwise.
>> + */
>> +static int determine_framerate(AVCodecContext *avctx,
>> + QCELPContext *q,
>> + const int buf_size,
>> + uint8_t **buf) {
>
> What rule do you follow when it comes to documenting parameters or
> not?
No firm rule, for now on if I document one parameter for a function,
I'll document the others.
Or, o you have a better suggestion?
>> + warn_insufficient_frame_quality(avctx, "Sanity check of
>> the codebook gain failed.");
>
> Codebook gain sanity check failed.
done
>> --- Changelog (revision 15692)
>> +++ Changelog (working copy)
>> @@ -138,6 +138,7 @@
>> - liba52 wrapper removed
>> +- QCELP / PureVoice decoder
>
> This has since changed.
done
>> --- libavcodec/Makefile (revision 15692)
>> +++ libavcodec/Makefile (working copy)
>> @@ -152,6 +152,7 @@
>> OBJS-$(CONFIG_PPM_ENCODER) += pnmenc.o pnm.o
>> OBJS-$(CONFIG_PTX_DECODER) += ptx.o
>> +OBJS-$(CONFIG_QCELP_DECODER) += qcelpdec.o qcelp_lsp.o
>> celp_math.o celp_filters.o
>> OBJS-$(CONFIG_QDM2_DECODER) += qdm2.o mdct.o
>> mpegaudiodec.o mpegaudiodecheader.o mpegaudio.o mpegaudiodata.o
>
> ditto
done
>> --- libavcodec/celp_filters.c (revision 15692)
>> +++ libavcodec/celp_filters.c (working copy)
>> @@ -84,3 +84,24 @@
>>
>> + for(n=0; n<buffer_length; n++)
>> + {
>
> You put the opening brace on the same line in other places. And the
> for
> expression could use some whitespace.
done
>> + out[n] = in[n];
>> + for(i=1; i<filter_length; i++)
>
> ditto
done
>> --- libavcodec/celp_math.c (revision 15692)
>> +++ libavcodec/celp_math.c (working copy)
>> @@ -195,3 +195,14 @@
>>
>> + for(i=0; i<length; i++)
>
> Some more whitespace will earn you extra good karma here.
done
Kenan
More information about the ffmpeg-devel
mailing list