[FFmpeg-devel] [PATCH] 1 of 5 Add Speex Encoding

[Diego Biurrun]

On Mon, Jul 06, 2009 at 02:55:42PM -0700, Art Clarke wrote:
> 
> --- libavcodec/libspeexenc.c	(revision 0)
> +++ libavcodec/libspeexenc.c	(revision 0)
> @@ -0,0 +1,177 @@
> +/*
> + * Copyright (c) 2009 by Xuggle Incorporated.  All rights reserved.

All rights are not reserved.
Please leave a blank line after the copyright statement.
Please add a one-line description of this file.

> + * 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
> + */
> +#include <libavcodec/avcodec.h>

#include "avcodec.h"

.. and an empty line before it would aid readability.

> +    if ((avctx->sample_fmt != SAMPLE_FMT_S16 && avctx->sample_fmt != SAMPLE_FMT_FLT) ||
> +            avctx->sample_rate <= 0 ||
> +            avctx->channels <= 0 ||
> +            avctx->channels > 2)

This could be nicely aligned for better readability.

> +    if (!s->enc_state)
> +    {

K&R style for new files please, i.e. "if () {".  You switch randomly
between styles..

> +    // initialize the header
> +    speex_init_header(&s->header, avctx->sample_rate,
> +            avctx->channels, mode);

weird indentation

> +
> +static av_cold int libspeex_encode_frame(
> +        AVCodecContext *avctx, uint8_t *frame,
> +        int buf_size, void *data)

weird indentation

> +    speex_bits_reset(&s->bits);
> +    for(i = 0; i < s->header.frames_per_packet; i++)
> +    {
> +        if (avctx->sample_fmt == SAMPLE_FMT_FLT)
> +        {

K&R please..

> +            if (avctx->channels == 2) {
> +                speex_encode_stereo(
> +                        (float*)data+i*s->header.frame_size,
> +                        s->header.frame_size,
> +                        &s->bits);
> +            }
> +            speex_encode(s->enc_state,
> +                    (float*)data+i*s->header.frame_size, &s->bits); 
> +        } else {
> +            if (avctx->channels == 2) {
> +                speex_encode_stereo_int(
> +                        (spx_int16_t*)data+i*s->header.frame_size,
> +                        s->header.frame_size,
> +                        &s->bits);
> +            }
> +            speex_encode_int(s->enc_state,
> +                    (spx_int16_t*)data+i*s->header.frame_size, &s->bits);

weird indentation of function arguments

> +    // put in a terminator so this will fit in a OGG or FLV packet

Ogg

> +    avctx->extradata = 0;
> +    avctx->extradata_size = 0;

align

Diego