[FFmpeg-devel] Update: IFF File Demuxer

[Vitor Sessak]

Hi

Jai Menon wrote:
> Hi,
> 
> I was working on an IFF demuxer and i have gotten to the point where it plays 
> IFF files containing 8SVX soundtreams ( as mentioned in the GSoC 
> Qualification task) properly. I have tested it on as many samples as i could 
> find, (basically from -> http://ftp.ticklers.org/pub/aminet/mods/smpl/)
> It currently supports decoding of Fibonacci Delta Encoded streams only 
> alongwith uncompressed audio. I will add support for Exponential encoding the 
> moment i can find some samples.
> Some files like -> http://ftp.ticklers.org/pub/aminet/mods/smpl/Pool.lha are 
> played perfectly (it doesn't play with  xine-lib, atleast on my system). 
> Demuxing of a number of other media as described in the IFF spec like ILBM 
> bitmaps, animations and 16SV support are still to be added, depending on 
> whether i can find some samples files. If you do know where i could find any 
> of these, i would be happy to add support.
> Please do tell me if the code conforms to FFmpeg's style guidelines and the 
> corrections i need to make to that effect.

Just some guidelines comments so you have something to work before 
Michael do the full-blown review.

> +/* 8SVX VHDR */
> +typedef struct {
> +    unsigned long  OneShotHigh;
> +    unsigned long  RepeatHigh;
> +    unsigned long  SamplesCycle;
> +    unsigned short SamplesPerSec;
> +    unsigned char  Octaves;
> +    unsigned char  Compression;
> +    long           Volume;
> +} SVX8_Vhdr;

Vars where the size is not important should be just declared as int (to 
avoid slow emulation in systems where a specific size is not supported).

> +      switch(id)
> +      {
> +	 case ID_VHDR:
> +	      iff->vhdr.OneShotHigh = get_be32(pb);
> +	      iff->vhdr.RepeatHigh = get_be32(pb);
> +	      iff->vhdr.SamplesCycle = get_be32(pb);
> +	      iff->vhdr.SamplesPerSec = get_be16(pb);
> +	      iff->vhdr.Octaves = get_byte(pb);
> +	      iff->vhdr.Compression = get_byte(pb);
> +	      iff->vhdr.Volume = get_be32(pb);
> +	      iff->gotVhdr = 1;
> +	      break;

Tabs are forbidden in FFmpeg codebase.

> +    st->codec->codec_tag = 0;  /* no tag */
> +    st->codec->channels = iff->channels;
> +    st->codec->sample_rate = iff->vhdr.SamplesPerSec;
> +    st->codec->bits_per_sample = 8;
> +    st->codec->bit_rate = st->codec->channels * st->codec->sample_rate * st->codec->bits_per_sample;
> +    st->codec->block_align = st->codec->channels * st->codec->bits_per_sample;  

Trailing whitespaces are forbidden too.

> +static int iff_read_packet(AVFormatContext *s,
> +                           AVPacket *pkt)
> +{
> +    IffDemuxContext *iff = s->priv_data;
> +    ByteIOContext *pb = s->pb;
> +    int ret = 0;
> +    char CodeToDelta[16] = { -34,-21,-13,-8,-5,-3,-2,-1,0,1,2,3,5,8,13,21 };

This could be
static const char CodeToDelta[16] = ...

> +    char *buf = NULL,*o_data = NULL, *src = NULL, *dest = NULL;	
> +    char x,d;
> +    long i,n,limit;

Some vars could be just int. Are all those variable initializations needed?

> +static int iff_read_close(AVFormatContext *s)
> +{
> +//    IffDemuxContext *iff = s->priv_data;
> +
> +    return 0;
> +}
> +

This function can be removed.

-Vitor