[FFmpeg-devel] [RFC] Split rtp.h

Michael Niedermayer michaelni
Wed Feb 4 17:40:29 CET 2009

On Wed, Feb 04, 2009 at 03:02:04PM +0100, Luca Abeni wrote:
> Hi all,
> the attached patch splits the current rtp.h in three files:
> a "common" rtp.h, rtpdec.h used by the demuxer, and rtpenc.h
> used by the muxer.
> To do so, a new "RTPMuxContext" is created, ad "RTPDemuxContext"
> is simplified removing the fields used by the muxer.
> Also, the AAC packetiser was abusing a field of the RTPDemuxContext
> (read_buf_index) by using it for storing the number of AAC frames
> contained in a packet. When creating RTPMuxContext, I did not copy
> the read_buf_index field in it, but I named the field "num_frames".
> As a result, the patch contains things like
> -    if ((s->read_buf_index == MAX_FRAMES_PER_PACKET) || (len && (len + 
> size) > max_packet_size)) {
> -        int au_size = s->read_buf_index * 2;
> +    if ((s->num_frames == MAX_FRAMES_PER_PACKET) || (len && (len + size) > 
> max_packet_size)) {
> +        int au_size = s->num_frames * 2;
> Finally, the patch also removes some useless .h files, merging them
> in the newly created rtpenc.h.
> I see all this as a single change, but I understand that people might
> want this committed in smaller steps. Just let me know the "level of
> split" which is desired, and I'll work on it.

the more you can split it the better...

anyway, here are the issues my script found (these of course have nothing
to do with the split and fixes to them belong to other patches but they
should be fixed one day none the less)

x==0 / x!=0 can be simplified to !x / x
split_rtp_h.diff:270:+    if (s->num_frames == 0) {

Non static with no ff_/av_ prefix
split_rtp_h.diff:223:+int rtp_get_payload_type(AVCodecContext *codec);
split_rtp_h.diff:475:+int rtp_get_codec_info(AVCodecContext *codec, int payload_type);
split_rtp_h.diff:478:+RTPDemuxContext *rtp_parse_open(AVFormatContext *s1, AVStream *st, URLContext *rtpc, int payload_type, RTPPayloadData *rtp_payload_data);
split_rtp_h.diff:479:+void rtp_parse_set_dynamic_protocol(RTPDemuxContext *s, PayloadContext *ctx,
split_rtp_h.diff:481:+int rtp_parse_packet(RTPDemuxContext *s, AVPacket *pkt,
split_rtp_h.diff:483:+void rtp_parse_close(RTPDemuxContext *s);
split_rtp_h.diff:485:+int rtp_get_local_port(URLContext *h);
split_rtp_h.diff:486:+int rtp_set_remote_url(URLContext *h, const char *uri);
split_rtp_h.diff:487:+void rtp_get_file_handles(URLContext *h, int *prtp_fd, int *prtcp_fd);
split_rtp_h.diff:494:+int rtp_check_and_send_back_rr(RTPDemuxContext *s, int count);
split_rtp_h.diff:585:+int rtsp_next_attr_and_value(const char **p, char *attr, int attr_size, char *value, int value_size); ///< from rtsp.c, but used by rtp dynamic protocol handlers.

 Non doxy comments
split_rtp_h.diff-529-+    // fields from AVRtpDynamicPayloadType_s
split_rtp_h.diff:530:+    const char enc_name[50];    /* XXX: still why 50 ? ;-) */
split_rtp_h.diff:531:+    enum CodecType codec_type;
split_rtp_h.diff:532:+    enum CodecID codec_id;
split_rtp_h.diff-534-+    // may be null
split_rtp_h.diff-535-+    int (*parse_sdp_a_line) (AVFormatContext *s,
split_rtp_h.diff:536:+                             int st_index,
split_rtp_h.diff:553:+    struct MpegTSContext *ts;   /* only used for MP2T payloads */
split_rtp_h.diff:554:+    int read_buf_index;
split_rtp_h.diff:555:+    int read_buf_size;
split_rtp_h.diff-556-+    /* used to send back RTCP RR */
split_rtp_h.diff:557:+    URLContext *rtp_ctx;
split_rtp_h.diff:558:+    char hostname[256];
split_rtp_h.diff-562-+    /* rtcp sender statistics receive */
split_rtp_h.diff:563:+    int64_t last_rtcp_ntp_time;    // TODO: move into statistics
split_rtp_h.diff:564:+    int64_t first_rtcp_ntp_time;   // TODO: move into statistics
split_rtp_h.diff:565:+    uint32_t last_rtcp_timestamp;  // TODO: move into statistics
split_rtp_h.diff-567-+    /* rtcp sender statistics */
split_rtp_h.diff:568:+    unsigned int octet_count;      // TODO: move into statistics (outgoing)
split_rtp_h.diff:569:+    unsigned int last_octet_count; // TODO: move into statistics (outgoing)
split_rtp_h.diff:570:+    int first_packet;
split_rtp_h.diff-571-+    /* buffer for output */
split_rtp_h.diff:572:+    uint8_t buf[RTP_MAX_PACKET_LENGTH];
split_rtp_h.diff-574-+    /* special infos for au headers parsing */
split_rtp_h.diff:575:+    RTPPayloadData *rtp_payload_data; // TODO: Move into dynamic payload handlers
split_rtp_h.diff-577-+    /* dynamic payload stuff */
split_rtp_h.diff:578:+    DynamicPayloadPacketHandlerProc parse_packet;     ///< This is also copied from the dynamic protocol handler structure
split_rtp_h.diff:579:+    PayloadContext *dynamic_protocol_context;        ///< This is a copy from the values setup from the sdp parsing, in rtsp.c don't free me.
split_rtp_h.diff-743-+    /* rtcp sender statistics receive */
split_rtp_h.diff:744:+    int64_t last_rtcp_ntp_time;    // TODO: move into statistics
split_rtp_h.diff:745:+    int64_t first_rtcp_ntp_time;   // TODO: move into statistics
split_rtp_h.diff-747-+    /* rtcp sender statistics */
split_rtp_h.diff:748:+    unsigned int packet_count;     // TODO: move into statistics (outgoing)
split_rtp_h.diff:749:+    unsigned int octet_count;      // TODO: move into statistics (outgoing)
split_rtp_h.diff:750:+    unsigned int last_octet_count; // TODO: move into statistics (outgoing)
split_rtp_h.diff:751:+    int first_packet;
split_rtp_h.diff-752-+    /* buffer for output */
split_rtp_h.diff:753:+    uint8_t buf[RTP_MAX_PACKET_LENGTH];
split_rtp_h.diff:754:+    uint8_t *buf_ptr;

also i think you dont need to send patches to split code maitained by you
just split the split and commit the parts

Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have often repented speaking, but never of holding my tongue.
-- Xenocrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090204/bd05a017/attachment.pgp>

More information about the ffmpeg-devel mailing list