[FFmpeg-devel] [PATCH] RealAudio SIPR @16k decoder (4/4) demuxer

[Benjamin Larsson]

Michael Niedermayer wrote:
> On Sat, Jul 12, 2008 at 02:17:03PM +0700, Vladimir Voroshilov wrote:
>> Hi, All
>>
>> Here is first draft version of fixed-point RealAudio sipr decoder
>> (only 16kHz mode)
>>
>> This patch contains demuxer changes from acelpdotnet patch.
>> It was written by Benjamin Larsson and i didn't touch it.
> 
> 
> [...]
>> @@ -146,6 +157,8 @@ static int rm_read_audio_stream_info(AVFormatContext *s, AVStream *st,
>>                  ((uint8_t*)st->codec->extradata)[i] = get_byte(pb);
>>              rm->audio_framesize = st->codec->block_align;
> 
>>              st->codec->block_align = rm->sub_packet_size;
>> +            if (st->codec->codec_id == CODEC_ID_SIPR)
>> +                st->codec->block_align = sipr_subpk_size[flavor];
> 
> why does block_align differ from sub_packet_size?

No idea. But I think the problem arises from having one codecid that can
have 4 different flavors. The only way to divide them is to look at the
frame sizes.

> and flavor is not checked for validity at all, please check things
> properly

Added check.

> 
> 
>>  
>>              if(rm->audio_framesize >= UINT_MAX / sub_packet_h){
>>                  av_log(s, AV_LOG_ERROR, "rm->audio_framesize * sub_packet_h too large\n");
>> @@ -557,7 +570,8 @@ ff_rm_parse_packet (AVFormatContext *s, AVStream *st, int len, AVPacket *pkt,
>>      } else if (st->codec->codec_type == CODEC_TYPE_AUDIO) {
>>          if ((st->codec->codec_id == CODEC_ID_RA_288) ||
>>              (st->codec->codec_id == CODEC_ID_COOK) ||
>> -            (st->codec->codec_id == CODEC_ID_ATRAC3)) {
>> +            (st->codec->codec_id == CODEC_ID_ATRAC3) ||
>> +            (st->codec->codec_id == CODEC_ID_SIPR)) {
>>              int x;
>>              int sps = rm->sub_packet_size;
>>              int cfs = rm->coded_framesize;
>> @@ -580,6 +594,33 @@ ff_rm_parse_packet (AVFormatContext *s, AVStream *st, int len, AVPacket *pkt,
>>                      for (x = 0; x < w/sps; x++)
>>                          get_buffer(pb, rm->audiobuf+sps*(h*x+((h+1)/2)*(y&1)+(y>>1)), sps);
>>                      break;
>> +                case CODEC_ID_SIPR:
>> +                        get_buffer(pb, rm->audiobuf + y * w, w);
>> +                        if (y == h - 1) {
>> +                            int n;
> 
>> +                            int bs = h * w * 2 / 96;  // nibbles per subpacket
> 
> *2 / 96 == /48

Fixed.

> 
> 
>> +                          // Perform reordering
>> +                          for(n=0; n < 38; n++) {
>> +                              int j;
>> +                              int i = bs * sipr_swaps[n][0];
>> +                              int o = bs * sipr_swaps[n][1];
>> +                                // swap nibbles of block 'i' with 'o'
>> +                                for(j = 0;j < bs; j++) {
> 
> Fix the indention, and check if the code cannot write out of the
> array. Ill checkit as well but the rm demuxer is messy its better if
> its checked by more than 1 person.

Indentation fixed. To me by inspection the code looks ok, and this code
has been running in mplayer for as long as it had binary rm codec
support. And I ran it through valgrind with no complaints on a test file.

MvH
Benjamin Larsson


-------------- next part --------------
A non-text attachment was scrubbed...
Name: sipr_rm_demux.diff
Type: text/x-diff
Size: 4957 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080924/2152d382/attachment.diff>