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

Michael Niedermayer michaelni
Wed Sep 24 20:06:15 CEST 2008


On Wed, Sep 24, 2008 at 10:33:53AM +0200, Benjamin Larsson wrote:
> 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
> 
> 

> Index: libavcodec/avcodec.h
> ===================================================================
> --- libavcodec/avcodec.h	(revision 15394)
> +++ libavcodec/avcodec.h	(working copy)
> @@ -304,6 +304,7 @@
>      CODEC_ID_WMALOSSLESS,
>      CODEC_ID_ATRAC3P,
>      CODEC_ID_EAC3,
> +    CODEC_ID_SIPR,
>  
>      /* subtitle codecs */
>      CODEC_ID_DVD_SUBTITLE= 0x17000,

ok, (yes ok-ed parts can be commited)


[...]

> @@ -127,7 +137,7 @@
>              }
>  
>              rm->audiobuf = av_malloc(rm->audio_framesize * sub_packet_h);
> -        } else if ((!strcmp(buf, "cook")) || (!strcmp(buf, "atrc"))) {
> +        } else if ((!strcmp(buf, "cook")) || (!strcmp(buf, "atrc")) || (!strcmp(buf, "sipr"))) {
>              int codecdata_length, i;
>              get_be16(pb); get_byte(pb);
>              if (((version >> 16) & 0xff) == 5)
> @@ -139,6 +149,7 @@
>              }
>  
>              if (!strcmp(buf, "cook")) st->codec->codec_id = CODEC_ID_COOK;
> +            else if (!strcmp(buf, "sipr")) st->codec->codec_id = CODEC_ID_SIPR;
>              else st->codec->codec_id = CODEC_ID_ATRAC3;
>              st->codec->extradata_size= codecdata_length;
>              st->codec->extradata= av_mallocz(st->codec->extradata_size + FF_INPUT_BUFFER_PADDING_SIZE);

ok


> @@ -146,6 +157,14 @@
>                  ((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) {
> +                if ((flavor>=0) || (flavor<4))
> +                    st->codec->block_align = sipr_subpk_size[flavor];
> +                else {
> +                    av_log(s, AV_LOG_ERROR, "flavor = %d not in [0-3] range, invalid value!\n", flavor);
> +                    return -1;
> +                }
> +            }

is there really no field at the demxuer layer that is the packet size?
this also applies a little to the other codecs which also each do
different things with block_align ...


>  
>              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 +576,8 @@
>      } 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;

ok


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Frequently ignored awnser#1 FFmpeg bugs should be sent to our bugtracker. User
questions about the command line tools should be sent to the ffmpeg-user ML.
And questions about how to use libav* should be sent to the libav-user ML.
-------------- 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/20080924/8d4feb61/attachment.pgp>



More information about the ffmpeg-devel mailing list