[FFmpeg-devel] [PATCH] Add support for packetizing AMR into RTP

Luca Abeni lucabe72
Wed Apr 8 09:19:50 CEST 2009

Hi Martin,

Martin Storsj? wrote:
>> I think in this case the right thing to do is to perform the check in
>> rtp_write_header(), and to fail if the payload size is too small. (I do
>> not like the idea of a function failing without returning an error code)
> Ok, fixed. I had to move the default case for max_frames_per_packet to 
> rtp_write_header in order to get a sensible check for the minimum 
> max_payload_size.
>> If you add the check described above (and remove the useless "else"),
>> the patch would be ok as far as I can see.
> Redundant else removed

Committed, thanks (with few small modifications to make patcheck happier).

> When checking for the corner cases in handling max_payload_size, I found a 
> minor bug that I had inherited from rtp_aac.c; I fixed this in my version, 
> and a fix for the case in rtp_aac.c is attached as a separate patch. 
>      /* test if the packet must be sent */
>      len = (s->buf_ptr - s->buf);
> -    if ((s->num_frames == MAX_FRAMES_PER_PACKET) || (len && (len + size) > max_packet_size)) {
> +    if ((s->num_frames == MAX_FRAMES_PER_PACKET) || (len && (len + size) > s->max_payload_size)) {
> Since s->buf_ptr starts from s->buf + MAX_AU_HEADERS_SIZE, len shouldn't 
> be checked against max_packet_size = s->max_payload_size - 
> MAX_AU_HEADERS_SIZE, but checked against s->max_payload_size instead.
> And the second hunk of the patch fixes cases where the data packet would 
> fit exactly into the space available.

Uh... Looks like you are right. I'll commit this later.

			Thanks again,

More information about the ffmpeg-devel mailing list