[FFmpeg-devel] [PATCH] RTP depacketizer for AMR

[Luca Abeni]

Hi Martin,

Martin Storsj? wrote:
> Hi,
> 
> Here's the first shot at an RTP depacketizer for AMR. It has been tested 
> with libavformat's RTP AMR packetizer, QuickTime Broadcaster and samples 
> from Darwin Streaming Server.

Nice work, thanks!


> Currently, this returns the AMR frames from the packet one at a time, by 
> buffering up the packet within the depacketizer.
> 
> I also tried returning all frames at once, but ffmpeg.c gives the 
> "Multiple frames in a packet from stream" error message (but works fine 
> except for that).

I think that if you return all the frames at once, you need an AMR parser
(which I do not think we currently have...).
So, I'd say that returning one frame at time is ok.

I leave the patch review to the maintainer, but it looks almost
fine to me.
The only thing that looks debatable is the fact that the payload parser
touches the timestamp. I believe the timestamp should only be modified
by rtpdec.c::rtp_parse_packet() (in other word, the first frame of the
RTP packet is assigned the RTP timestamp, the other frames get
AV_NOPTS_VALUE). I think libavformat can correctly fill the missing
timestamps... Can you try it and see what happens?

I also am not sure if it is worth introducing a new header
(rtpdec_amr.h) just for two "extern" declarations... Maybe they
can be moved to rtpdec.h?


			Thanks,
				Luca