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 <[email protected]> > 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 _______________________________________________ FFmpeg-soc mailing list [email protected] https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-soc
