[FFmpeg-devel] [PATCH] RTP packetizer for AAC audio

[Luca Abeni]

Hi Luca,

Luca Barbato wrote:
>> - aac_encapsulation.diff implements the basic packetization algorithm
>> (without fragmentation). Some notes:
>>     + The patch introduces a new rtp_aac.c file (since this is really
>> the "AAC-hbr" mode of RFC3640, maybe the file should be called
>> rtp_rfc3640.c, in case someone implements the other modes in the future?).
> 
> leave it aac, makes more sense now.

Ok

>>     + The maximum number of AAC frames per packet is fixed (maybe we can
>> make it runtime-configurable in the future).
> 
> Isn't possible to just set an mtu threshold and use it? This way you
> fill the packet in a more uniform way.

There are two problems:
1) putting too many frames in a packet increases the streaming latency 
(and the amount of buffering needed by the client to correctly 
synchronize audio and video). I see that AAC frames can be quite small, 
so this can be a real problem
2) the payload header contains 2 bytes for each AAC frame contained in 
the packet. If a maximum number of frames per packet is not known in 
advance, a memcpy() of the packet payload is needed when sending the 
packet... Knowing the maiximum number of frames, I can reserve the space 
for the payload header, and just move few bytes before sending the 
packet. Anyway, setting MAX_FRAMES_PER_PACKET to 20 or something similar 
we can obtain the behaviour you suggest... So, I think this 
implementation is more generic. I just need to transofrm 
MAX_FRAMES_PER_PACKET in a variable, and to find a way for setting it - 
I think the "max_delay" field of AVFormatContext can be used for this 
purpose, but I need to think to this a little bit more.


>>     + Instead of adding a new field to the RTPDemuxContext structure, I
>> reused the "read_buf_index" field (which was only used in receiving AAC
>> audio). If this is not ok, I'll fix the problem.
> 
> I doubt it is a problem, still would be better having a variable for
> frame stacking (since it will be used also on vorbis and theora)

Ok. So, I propose to use read_buf_index for now, and to introduce a new 
field when it will be needed by more packetizers.

>>     + The patch depends on the two "RTP timestamps" patches that I
>> posted some time ago
> 
> Commit them anytime =)

Ok, thanks. I am leaving for the weekend in few moments, so I'll commit 
on Monday (when I'll be more responsive if there are problems with my 
commits).


>>     + Fragmentation support is not implemented because I've not been
>> able to generate big AAC frames, so I could not test it
> 
> make the mtu ridiculously little like 200 to test.

Ok; I'll modify the code in that way, and test during the weekend.



			Thanks,
				Luca