[Ffmpeg-devel] [PATCH] RTP payload format for AMR

[Kite Flyer]

Hi Michael,

Thanks for your input. Attached patch is based on the same. Pls see inline.

-Subrata

On 3/10/07, Michael Niedermayer <michaelni at gmx.at> wrote:
> Hi
>
> On Fri, Mar 09, 2007 at 09:31:47PM +0530, Kite Flyer wrote:
> > Hi Michael,
> >
> > Please see inline. New patch is attached.
> >
> > Thanks,
> > Subrata
> >
> > On 3/8/07, Michael Niedermayer <michaelni at gmx.at> wrote:
> > >Hi
> > >
> > >On Thu, Mar 08, 2007 at 06:21:34PM +0530, Kite Flyer wrote:
> > >[...]
> > >> >
> > >> >
> > >> >[...]
> > >> >
> > >> >> +        //TODO: find a way to pack maximum number of frames based on
> > >> >max_payload_size
> > >> >
> > >> >yes
> > >> >
> > >>
> > >> Keeping it as fixed no of frames as of now.
> > >>
> > >> payload format requires that all speech data frames be written after
> > >> all frame headers i.e. ToC. If encoder always generates constant bit
> > >> rate frames then offset of data frames can be calculated and maximum
> > >> no of frames can be packed. But if encoder can generate variable bit
> > >> rate frames, then two memory areas can be taken: one for ToC and
> > >> another for data but the memcpy to concatenate the memories would be
> > >> expensive per send.
> > >
> > >no, copying a ToC (that ends at about 50 byte per second is totally
> > >negligible)
> > >
> > >8khz (only one supported by AMR NB) / 160 (frame size) = 50 (1 byte Toc
> > >bytes)
> > >
> > >additionally AMR is low bitrate stuff so copying the encoded bitstream
> > >around cannot be timeconsuming, copying part of it even less so ...
> > >
> >
> > OK. Implemented at the cost of having a static buffer in rtp_send_amr
> > routine. No more amr specific code in RTPDemuxContext common code. Now
>
> non constant static and global arrays are forbidden in libav* (with exceptions
> for debug code and arrays which are initalized once but never changed)
> 2 RTP instances could not coexist otherwise ...
>

Very true. My indifference.

> one easy way to pack the AMR frames is:
>
> uint8_t *toc_buf= s->buf;

I made it s->buf  + 1 to take care of the (CMR,RRRR) byte before TOC.

> uint8_t *buf= s->buf + TOC_MAX;

I have set TOC_MAX to max_payload_size/ 13 where 13 = 1 (toc_byte) +
12 (4.75 kbps frame length). It should take care of the one byte
length for the (CMR,RRRR) byte too.

>
> payload_size= s->buf_ptr - buf + size;
> if(   s->foobar >= TOC_MAX
>    || TOC_MAX + payload_size  > sizeof(s->buf)
>    || s->foobar + 1 + payload_size > s->max_payload_size){
>

First condition checks if TOC buffer is full. Second condition should
check if speech buf alone is full. If so, is the third condition
needed? Pls see my patch.

>     memmove(buf - s->foobar, toc_buf, s->foobar);

I didn't know of such a nice readymade function memmove!

>     send   (buf - s->foobar, s->foobar + s->buf_ptr - buf);
>     s->buf_ptr= buf;
>     s->foobar=0;
> }
>
> toc_buf[ s->foobar++ ]= toc_byte
> memcpy(s->buf_ptr, input, size);
> s->buf_ptr+= size;
>
> [...]
> > +        /* append speech data after ToC and send */
>
> trailing whitespace (this is forbidden in svn)
>

oops

>
> [...]
> > +    flen = packed_size[ft];
> > +    if (flen > 0)
> > +    {
> > +        if (size >= flen)
>
> isnt flen always == size if so this code isnt needed, also packed_size[] isnt
> needed OTOH if flen can be != size id like to know how that is possible
>

Actually I had an impression that encoder gives multiple frames at a
time. Now removed the lines.

> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> No snowflake in an avalanche ever feels responsible. -- Voltaire
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at mplayerhq.hu
> http://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: amr_rtp_payload3.patch
Type: application/octet-stream
Size: 3106 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070312/00b7aee1/attachment.obj>