[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