[FFmpeg-soc] [PATCH] decouple mpeg4/aac from rtsp

[Martin Storsjö]

Hi Josh,

For part 1, I don't have too much opinion...

On Sun, 20 Jun 2010, Josh Allmann wrote:

> From dc9586eef1ef936b8fcaca1cbaeb7d5d7690d036 Mon Sep 17 00:00:00 2001
> From: Josh Allmann <joshua.allmann at gmail.com>
> Date: Sun, 20 Jun 2010 12:25:59 -0700
> Subject: [PATCH 2/2] Decouple MPEG-4 and AAC specific parts from rtsp.c.
> 
> ---
>  libavformat/Makefile      |    1 +
>  libavformat/rtpdec.c      |    8 +--
>  libavformat/rtpdec_mpeg.c |  123 +++++++++++++++++++++++++++++++++++++++++++++
>  libavformat/rtpdec_mpeg.h |   38 ++++++++++++++
>  libavformat/rtsp.c        |   58 ---------------------
>  5 files changed, 165 insertions(+), 63 deletions(-)
>  create mode 100644 libavformat/rtpdec_mpeg.c
>  create mode 100644 libavformat/rtpdec_mpeg.h

I think mpeg4 would be a better/more specific name for this, since it 
doesn't cover mpeg1/2 afaik.

While looks like a good first step, it doesn't remove all of the mpeg4 
stuff from rtsp.c.

There's still the AttrNameMap, attr_names and sdp_parse_fmtp which does 
nothing but AAC-specific things. These routines parse parameters and set 
them in the deceivingly generically named RTPPayloadData, which despite 
the name isn't used by any other format than AAC. (Additionally, this 
struct is allocated and owned by the RTSP layer.) So all of this code 
could be moved out to the dynamic payload handler, and be parsed by a 
normal parse_sdp_a_line just as for all other formats. There's also quite 
a bit of AAC specific parsing code in rtpdec.c that needs to be moved to 
the dynamic payload handler, too.

Side note: When looking at that code, there's a small bit of code for 
depacketizing MP2/MP3 and MPEG1VIDEO/MPEG2VIDEO, too, that perhaps could 
be moved out, but I'm not totally sure on how to handle it since they 
aren't dynamic payload formats at all. (The only difference in practice is 
that they have hardcoded RTP payload numbers, but they could just as well 
use the same structure anyway.)

To do this cleanly, I guess it's best to do this in smaller chunks, and as 
such, I think your current patch #2 looks quite ok, except for the naming.

// Martin