[Ffmpeg-devel] THP decoder

[Michael Niedermayer]

Hi

On Wed, Apr 04, 2007 at 11:46:51PM +0400, Kislyakov Maxim wrote:
> Hi,
> Made some cosmetics and removed whitespaces, made thp.c as new file,
> because i have written it myself.
> This is new patch.
> 
[...]

> +//THP format description
> +typedef struct HeaderData  {

comment is not doxygen compatible


> +    int magicBites;
> +    int version;
> +    int maxBufferSize;
> +    int maxAudioSamples;
> +    int numFrames;
> +    int firstFrameSize;
> +    int dataSize;
> +    uint32_t componentDataOffset;
> +    uint32_t offsetsDataOffset;
> +    uint32_t firstFrameOffset;
> +    uint32_t lastFrameOffset;
> +} HeaderData;
> +
> +typedef struct ComponentStructure {
> +    uint32_t numComponents;
> +    uint8_t componentTypes[16];
> +    uint32_t curFrame;
> +    int frameSize;
> +    int indexComponent;
> +} ComponentStructure;
> +
> +typedef struct FrameData  {
> +    int nextTotalSize;
> +    int prevTotalSize;
> +    int imageSize;
> +    int audioSize;
> +} FrameData;
> +
> +typedef struct ThpDemuxerContext  {
> +    HeaderData thpHeader;
> +    ComponentStructure compStruct;
> +    FrameData frameHeader;

why this nested structs?


> +    int videoStreamIndex;
> +    int audioStreamIndex;
> +    AVRational fps;
> +} ThpDemuxerContext;
> +
> +static int thp_probe(AVProbeData *p)
> +{
> +
> +    if (p->buf_size < 4)
> +        return 0;
> +    if (p->buf[0] == 'T' && p->buf[1] == 'H' && p->buf[2] == 'P' && p->buf[3] == '\0') {
> +        return AVPROBE_SCORE_MAX;

AV_RL32(p->buf) and MKTAG


> +    }
> +    else
> +        return 0;
> +}
> +
> +static int thp_read_header(AVFormatContext *s, AVFormatParameters *ap)
> +{
> +    ThpDemuxerContext *thpDemux = s->priv_data;
> +    ByteIOContext *pb = &s->pb;
> +    AVStream *st;
> +    int *header;
> +    const int head = 0x00504854;
> +    get_buffer(pb, header, 4);
> +    if (*header != head)
> +    {
> +        return AVERROR_INVALIDDATA;
> +    }

this is wrong


[...]
> +    thpDemux->videoStreamIndex = st->index;

this will always be 0


> +    st->codec->codec_type = CODEC_TYPE_VIDEO;
> +    st->codec->codec_id = CODEC_ID_THP;
> +    st->codec->codec_tag = 0;  /* no fourcc */

> +    st->codec->sample_rate=av_q2d(thpDemux->fps);

looks wrong


> +
> +    if (thpDemux->thpHeader.maxAudioSamples != 0)
> +    {
> +        st = av_new_stream(s, 0);
> +        if (!st)
> +        {
> +            return AVERROR_NOMEM;
> +        }
> +
> +        thpDemux->audioStreamIndex = st->index;
> +        st->codec->codec_type = CODEC_TYPE_AUDIO;
> +        st->codec->codec_id = CODEC_ID_ADPCM_THP;
> +        st->codec->codec_tag = 0;
> +        st->codec->channels = get_be32(pb);/*Number of channels*/
> +        st->codec->sample_rate = get_be32(pb);/* Frequency */
> +        av_set_pts_info(st, 64, 1, st->codec->sample_rate);
> +        st->codec->bits_per_sample = 4;
> +        st->codec->bit_rate = st->codec->sample_rate * st->codec->bits_per_sample * st->codec->channels;
> +        thpDemux->audioStreamIndex = st->index;

this will always be 1


[...]

> +    float tableFC[16];
> +    float tableSC[16];

please remove all floats in the adpcm decoder


> +    int16_t channel1Prev1;
> +    int16_t channel1Prev2; 
> +    int16_t channel2Prev1;
> +    int16_t channel2Prev2;

these could be a [2][2] array


[...]
> +    if (!st)  { /*first channel*/
> +        factor1 = frameHeader->tableFC[2*index];
> +        factor2 = frameHeader->tableFC[2*index + 1];
> +        temp = frameHeader->channel1Prev1*factor1 + frameHeader->channel1Prev2*factor2 + (nibble << exponent);
> +        frameHeader->channel1Prev2 = frameHeader->channel1Prev1;
> +        frameHeader->channel1Prev1 = temp;
> +    }
> +    else  { /*second channel*/
> +        factor1 = frameHeader->tableSC[2*index];
> +        factor2 = frameHeader->tableSC[2*index + 1];
> +        temp = frameHeader->channel2Prev1*factor1 + frameHeader->channel2Prev2*factor2 + (nibble << exponent);
> +        frameHeader->channel2Prev2 = frameHeader->channel2Prev1;
> +        frameHeader->channel2Prev1 = temp;
> +    }

code duplication


[...]
> @@ -1111,6 +1147,59 @@
>              buf_size -= 128;
>          }
>          break;
> +    case CODEC_ID_ADPCM_THP: 
> +        frameHeader->channelSize = (uint32_t)(src[3] + (src[2] << 8) + (src[1] << 16) + (src[0] << 24)); /* Channel Size */
> +        src += 4;
> +        frameHeader->numSamples = (uint32_t)(src[3] + (src[2] << 8) + (src[1] << 16) + (src[0] << 24)); /* Number of samples */
> +        src += 4;
> +        int16_t tempCell;

breaks gcc 2.95


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

If you really think that XML is the answer, then you definitly missunderstood
the question -- Attila Kinali
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070405/fcd2c6e3/attachment.pgp>