[FFmpeg-devel] [PATCH] Funcom ISS playback

[Stefan Gehrer]

Michael Niedermayer wrote:
> On Fri, Jan 16, 2009 at 10:18:21PM +0100, Stefan Gehrer wrote:
>> I reworked Jai Menon's patch from last April to play back Funcom ISS
>> Audio files and I think I took care of all the comments made at the
>> time.
>> To actually play back without hissing artefacts I also had to
>> introduce a new variation to the long list of ADPCM codec IDs.
>> The samples in http://samples.mplayerhq.hu/game-formats/funcom-iss/
>> feature both stereo and mono modes and different packet sizes
>> and all four samples play back nicely now.
>>
> [...]
>> +    get_token(pb, token, sizeof(token)); //"IMA_ADPCM_Sound"
>> +    get_token(pb, token, sizeof(token)); //packet size
>> +    sscanf(token, "%d", &iss->packet_size);
>> +    get_token(pb, token, sizeof(token)); //File ID
>> +    get_token(pb, token, sizeof(token)); //out size
>> +    get_token(pb, token, sizeof(token)); //stereo
>> +    sscanf(token, "%d", &stereo);
>> +    get_token(pb, token, sizeof(token)); //Unknown1
>> +    get_token(pb, token, sizeof(token)); //RateDivisor
>> +    sscanf(token, "%d", &rate_divisor);
>> +    get_token(pb, token, sizeof(token)); //Unknown2
>> +    get_token(pb, token, sizeof(token)); //Version ID
>> +    get_token(pb, token, sizeof(token)); //Size
>> +    sscanf(token, "%d", &stream_size);
>> +    
>> +    st = av_new_stream(s, 0);
>> +    if (!st)
>> +        return AVERROR(ENOMEM);
>> +    st->codec->codec_type = CODEC_TYPE_AUDIO;
>> +    st->codec->codec_id = CODEC_ID_ADPCM_IMA_ISS;
>> +    st->codec->channels = stereo ? 2 : 1;
> 
> 
>> +    st->codec->sample_rate = 44100 / rate_divisor;
> 
> this could be 0, a negative number or even a 1/0
> similarly others should be checked to not be <=0 where this isnt valid

added a check for this division and the one in read_packet

> 
>> +    st->codec->bits_per_coded_sample = 4;
>> +    st->codec->bit_rate = st->codec->channels * st->codec->sample_rate 
>> +                                      * st->codec->bits_per_coded_sample;
>> +    st->codec->block_align = iss->packet_size;
>> +    av_set_pts_info(st, 32, 1, st->codec->sample_rate);
>> +
>> +    return 0;
>> +}
>> +
> 
>> +static int iss_read_packet(AVFormatContext *s,
>> +                           AVPacket *pkt)
>> +{
>> +    IssDemuxContext *iss = s->priv_data;
>> +    ByteIOContext *pb = s->pb;
>> +    int ret = av_get_packet(pb, pkt, iss->packet_size);
> 
> as pb is used just once its not too usefull as seperate var.

removed.

> 
>> +
>> +    if(ret < 0)
>> +        return ret;
>> +
>> +    pkt->stream_index = 0;
> 
>> +    pkt->pts = iss->sample_count;
>> +    iss->sample_count += (ret / s->streams[0]->codec->channels);
> 
> cant this be done by url_ftell() ? 
> that way it would naturally work with seeking

done with url_ftell, I hope it is correct.

> [...]
>> +        if(st) {
>> +            c->status[0].predictor  = (int16_t)AV_RL16(src + 0);
>> +            c->status[0].step_index = src[2];
> [...]
>> +        } else {
>> +            c->status[0].predictor  = (int16_t)AV_RL16(src + 0);
>> +            c->status[0].step_index = src[2];            
>> +            src += 4;
>> +        }
> 
> hmm
> 

duplicate lines moved out of the if.

>> +        while (src < buf + buf_size) {
>> +
>> +            if (st) {
>> +                *samples++ = adpcm_ima_expand_nibble(&c->status[0],
>> +                    src[0] >> 4  , 3);
>> +                *samples++ = adpcm_ima_expand_nibble(&c->status[1],
>> +                    src[0] & 0x0F, 3);
>> +            } else {
>> +                *samples++ = adpcm_ima_expand_nibble(&c->status[0],
>> +                    src[0] & 0x0F  , 3);
>> +                *samples++ = adpcm_ima_expand_nibble(&c->status[0],
>> +                    src[0] >> 4, 3);
>> +            }
> 
> are you ure stereo is not R<->L ? would be more consistent i it is
> 

I rely on the information here from 
http://wiki.multimedia.cx/index.php?title=FunCom_ISS
And I think I could not tell from the samples at hand what is meant to
be left and what right.

Thanks for review, new patch attached.

Stefan
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: funcomiss.diff
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090117/8f934576/attachment.asc>