[FFmpeg-devel] [MPlayer-dev-eng] rtp support in mplayer

Michael Niedermayer michaelni
Tue Jun 10 21:00:57 CEST 2008


On Mon, Jun 09, 2008 at 01:38:26PM -0400, Chas Williams (CONTRACTOR) wrote:
> In message <20080516134500.8485095e.tempn at twmi.rr.com>,compn writes:
> >libavcodec patches go to ffmpeg-devel list.
> 
> here is a patch to libavcodec for g722 audio support.

[...]


> +/*! \file */

useless


> +
> +#if !defined(_G722_H_)
> +#define _G722_H_

This isnt a header file


> +
> +#include <inttypes.h>
> +
> +/*! \page g722_page G.722 encoding and decoding
> +\section g722_page_sec_1 What does it do?
> +The G.722 module is a bit exact implementation of the ITU G.722 specification for all three
> +specified bit rates - 64000bps, 56000bps and 48000bps. It passes the ITU tests.
> +
> +To allow fast and flexible interworking with narrow band telephony, the encoder and decoder
> +support an option for the linear audio to be an 8k samples/second stream. In this mode the
> +codec is considerably faster, and still fully compatible with wideband terminals using G.722.
> +

> +\section g722_page_sec_2 How does it work?
> +???.

yes ??? iam confused ... what does this comment mean?



[...]
> +
> +#ifndef INT16_MAX
> +#define INT16_MAX       32767
> +#endif
> +#ifndef INT16_MIN
> +#define INT16_MIN       (-32768)
> +#endif

doesnt belong in a codec


> +
> +typedef struct
> +{
> +    /*! TRUE if the operating in the special ITU test mode, with the band split filters
> +             disabled. */
> +    int itu_test_mode;

Get rid of all the TRUE and FALSE please, we have 0 and 1
Besides whats a "special ITU test mode" and why would it be in here?


> +    /*! TRUE if the G.722 data is packed */
> +    int packed;
> +    /*! TRUE if encode from 8k samples/second */
> +    int eight_k;
> +    /*! 6 for 48000kbps, 7 for 56000kbps, or 8 for 64000kbps. */
> +    int bits_per_sample;
> +

> +    /*! Signal history for the QMF */
> +    int x[24];
> +
> +    struct
> +    {
> +        int s;
> +        int sp;
> +        int sz;
> +        int r[3];
> +        int a[3];
> +        int ap[3];
> +        int p[3];
> +        int d[7];
> +        int b[7];
> +        int bp[7];
> +        int sg[7];
> +        int nb;
> +        int det;
> +    } band[2];

Please rename these so they are english words or abbreviations.


> +
> +    unsigned int in_buffer;
> +    int in_bits;
> +    unsigned int out_buffer;
> +    int out_bits;
> +} g722_encode_state_t;

> +
> +typedef struct
> +{
> +    /*! TRUE if the operating in the special ITU test mode, with the band split filters
> +             disabled. */
> +    int itu_test_mode;
> +    /*! TRUE if the G.722 data is packed */
> +    int packed;
> +    /*! TRUE if decode to 8k samples/second */
> +    int eight_k;
> +    /*! 6 for 48000kbps, 7 for 56000kbps, or 8 for 64000kbps. */
> +    int bits_per_sample;
> +
> +    /*! Signal history for the QMF */
> +    int x[24];
> +
> +    struct
> +    {
> +        int s;
> +        int sp;
> +        int sz;
> +        int r[3];
> +        int a[3];
> +        int ap[3];
> +        int p[3];
> +        int d[7];
> +        int b[7];
> +        int bp[7];
> +        int sg[7];
> +        int nb;
> +        int det;
> +    } band[2];
> +    
> +    unsigned int in_buffer;
> +    int in_bits;
> +    unsigned int out_buffer;
> +    int out_bits;
> +} g722_decode_state_t;

Duplicate


> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif

No C++ here -> we dont need to extern "C" anything


> +
> +g722_decode_state_t *g722_decode_init(g722_decode_state_t *s, int rate, int options);
> +int g722_decode_release(g722_decode_state_t *s);
> +int g722_decode(g722_decode_state_t *s, int16_t amp[], const uint8_t g722_data[], int len);

unneeded


> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif
> +/*
> + * SpanDSP - a series of DSP components for telephony
> + *
> + * g722_decode.c - The ITU G.722 codec, decode part.
> + *
> + * Written by Steve Underwood <steveu at coppice.org>
> + *
> + * Copyright (C) 2005 Steve Underwood
> + *
> + *  Despite my general liking of the GPL, I place my own contributions 
> + *  to this code in the public domain for the benefit of all mankind -
> + *  even the slimy ones who might try to proprietize my work and use it
> + *  to my detriment.
> + *
> + * Based in part on a single channel G.722 codec which is:
> + *
> + * Copyright (c) CMU 1993
> + * Computer Science, Speech Group
> + * Chengxiang Lu and Alex Hauptmann
> + *
> + * $Id$
> + */

duplicate

> +
> +/*! \file */
> +
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#include <stdio.h>
> +#include <inttypes.h>
> +#include <memory.h>
> +#include <stdlib.h>
> +
> +#include <limits.h>
> +#include "avcodec.h"
> +#include "bitstream.h"

#includes in the middle of c files are no good


> +
> +#if !defined(FALSE)
> +#define FALSE 0
> +#endif
> +#if !defined(TRUE)
> +#define TRUE (!FALSE)
> +#endif

duplicate


> +
> +static int16_t saturate(int32_t amp)
> +{
> +    int16_t amp16;
> +
> +    /* Hopefully this is optimised for the common case - not clipping */
> +    amp16 = (int16_t) amp;
> +    if (amp == amp16)
> +        return amp16;
> +    if (amp > INT16_MAX)
> +        return  INT16_MAX;
> +    return  INT16_MIN;
> +}

av_clip_int16()


[...]
> +    if (wd3 > 12288)
> +        wd3 = 12288;
> +    else if (wd3 < -12288)
> +        wd3 = -12288;

av_clip()


[...]
> +g722_decode_state_t *g722_decode_init(g722_decode_state_t *s, int rate, int options)

static


> +{
> +    if (s == NULL)
> +    {
> +        if ((s = (g722_decode_state_t *) av_malloc(sizeof(*s))) == NULL)
> +            return NULL;
> +    }
> +    memset(s, 0, sizeof(*s));

This is unneeded as s is always NULL


[...]
> +int g722_decode_release(g722_decode_state_t *s)
> +{
> +    av_free(s);
> +    return 0;
> +}

senseles wraper


> +/*- End of function --------------------------------------------------------*/
> +

> +int g722_decode(g722_decode_state_t *s, int16_t amp[], const uint8_t g722_data[], int len)

static


> +{
> +    static const int wl[8] = {-60, -30, 58, 172, 334, 538, 1198, 3042 };
> +    static const int rl42[16] = {0, 7, 6, 5, 4, 3, 2, 1, 7, 6, 5, 4, 3,  2, 1, 0 };
> +    static const int ilb[32] =
> +    {
> +        2048, 2093, 2139, 2186, 2233, 2282, 2332,
> +        2383, 2435, 2489, 2543, 2599, 2656, 2714,
> +        2774, 2834, 2896, 2960, 3025, 3091, 3158,
> +        3228, 3298, 3371, 3444, 3520, 3597, 3676,
> +        3756, 3838, 3922, 4008
> +    };
> +    static const int wh[3] = {0, -214, 798};
> +    static const int rh2[4] = {2, 1, 2, 1};
> +    static const int qm2[4] = {-7408, -1616,  7408,   1616};

these do not need to be itnt, smaller types are sufficient


[...]
> +        switch (s->bits_per_sample)
> +        {
> +        default:
> +        case 8:
> +            wd1 = code & 0x3F;
> +            ihigh = (code >> 6) & 0x03;
> +            wd2 = qm6[wd1];
> +            wd1 >>= 2;
> +            break;
> +        case 7:
> +            wd1 = code & 0x1F;
> +            ihigh = (code >> 5) & 0x03;
> +            wd2 = qm5[wd1];
> +            wd1 >>= 1;
> +            break;
> +        case 6:
> +            wd1 = code & 0x0F;
> +            ihigh = (code >> 4) & 0x03;
> +            wd2 = qm4[wd1];
> +            break;
> +        }

this can be simplified


[...]
> +typedef struct AVG722Context {
> +    g722_decode_state_t *s;
> +} AVG722Context;

redundant wraper, everthing should be in AVG722Context not in a secondary
struct.


> +
> +static int g722_init(AVCodecContext * avctx)
> +{

> +    AVG722Context *c = (AVG722Context *) avctx->priv_data;

unneeed cast


> +    int options = 0;
> +
> +    if (avctx->channels != 1 ||
> +        (avctx->bit_rate != 48000 && avctx->bit_rate != 56000 &&
> +                                     avctx->bit_rate != 64000)) {
> +        av_log(avctx, AV_LOG_ERROR, "G722: unsupported audio format\n");
> +        return -1;
> +    }
> +

> +    if (avctx->sample_rate != 8000 && avctx->sample_rate != 16000
> +	&& avctx->strict_std_compliance>FF_COMPLIANCE_INOFFICIAL) {
> +        av_log(avctx, AV_LOG_ERROR, "G722: unsupported audio format\n");
> +        return -1;
> +    }

> +    

trailing whitespace is forbidden in svn


[...]

> +static int g722_decode_frame(AVCodecContext *avctx,
> +                             void *data, int *data_size,
> +                             uint8_t *buf, int buf_size)
> +{
> +    AVG722Context *c = avctx->priv_data;
> +    int16_t *amp = data;
> +
> +    *data_size = g722_decode(c->s, amp, buf, buf_size) * sizeof(short);
> +    return buf_size;
> +}
> +
> +static int g722_close(AVCodecContext *avctx)
> +{
> +    AVG722Context* c = (AVG722Context *) avctx->priv_data;
> +
> +    g722_decode_release(c->s);
> +    av_freep(&avctx->coded_frame);
> +    return 0;
> +}

senseless wraper functions


[...]
> --- mplayer-1.0rc2.orig/etc/codecs.conf	2007-10-07 15:49:33.000000000 -0400
> +++ mplayer-1.0rc2/etc/codecs.conf	2008-05-16 11:16:42.000000000 -0400
> @@ -3141,6 +3141,13 @@
>    driver ffmpeg
>    dll "g726"
>  
> +audiocodec g722
> +  info "G.722 Audio"
> +  status working
> +  format 0x41
> +  driver ffmpeg
> +  dll "g722"
> +

belongs to mplayer-dev

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Everything should be made as simple as possible, but not simpler.
-- Albert Einstein
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080610/90623ac8/attachment.pgp>



More information about the ffmpeg-devel mailing list