[FFmpeg-devel] [PATCH] RTSP-MS 10/15: ASF header parsing

Ronald S. Bultje rsbultje
Wed Feb 4 15:01:01 CET 2009


Hi,

take 2...

On Wed, Feb 4, 2009 at 3:28 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> On Tue, Feb 03, 2009 at 09:43:49PM -0500, Ronald S. Bultje wrote:
>> +    enum RTSPLowerTransport lower_transport; /**< TCP/UDP uni-/multicast */
>
> well, better than before but primarely comments should document what a field
> is, this indeed is obvious from the enumeration and name but its not written
> in the comment.
>
>
>>  } RTSPTransportField;
>>
>>  typedef struct RTSPHeader {
>> @@ -66,7 +66,7 @@
>>      int seq; /**< sequence number */
>>      char session_id[512];
>>      char real_challenge[64]; /**< the RealChallenge1 field from the server */
>> -    char server[64];
>> +    char server[64];                 /**< the Server field, to identify WMS */
>
> and the structure represents what?

I documented most (not all, some are pretty obvious so I left those
out). I also didn't document RTSPActionServerSetup because it is
unused in the RTSP code, only used in ffserver.c. OK if I move it
there? (Separate patch, clearly.)

> and session_id?

Added.

>>  typedef struct RTSPState {
>>      URLContext *rtsp_hd; /* RTSP TCP connexion handle */
>>      int nb_rtsp_streams;
>
>> -    struct RTSPStream **rtsp_streams;
>> +    struct RTSPStream **rtsp_streams; /**< for each m= line in the SDP */
>
> this is not a description of what rtsp_streams is
> just try:
> A: what is rtsp_streams?
> B: for each m= line in the SDP
>
> didnt work ...

I added documentation for RTSPStream which describes (more general,
multi-line) what they are, is that OK? I think from that description,
this variable is pretty clear. I removed this weird line and replaced
it by a simple "streams in session".

>>      enum RTSPClientState state;
>>      int64_t seek_timestamp;
>
> no comments for these?

Added.

>> +    enum RTSPTransport transport;     /**< RTP or RDT */
>> +    enum RTSPLowerTransport lower_transport; /**< TCP/UDP uni-/multicast */
>> +    enum RTSPServerType server_type;  /**< brand of server (MS, RM, other) */
>
> align

That becomes really ugly, I have to line-wrap almost all comments. I'd
like to leave just and only lower_transport unaligned just so I can
fit more than 2 words on a comment line, please?

> and the abbreviated company names are not enum values though this is
> very minor

Fixed.

>> -    void *cur_tx;
>> +    void *cur_tx;                     /**< stream for last packet */
>
> where is the typedef for type stream? there is none? then what is a stream?
> this is a void* so no way to know ...

Crappy comment, I rewrote it so it's clear what the variable is. I
already know what your next question will be ("what is
RTSPStream->tx_ctx?"), but I think that's relatively clearly
documented.

Ronald
-------------- next part --------------
Index: ffmpeg-svn/libavformat/rtsp.h
===================================================================
--- ffmpeg-svn.orig/libavformat/rtsp.h	2009-02-04 08:46:59.000000000 -0500
+++ ffmpeg-svn/libavformat/rtsp.h	2009-02-04 08:57:24.000000000 -0500
@@ -38,8 +38,8 @@
 };
 
 enum RTSPTransport {
-    RTSP_TRANSPORT_RTP,
-    RTSP_TRANSPORT_RDT,
+    RTSP_TRANSPORT_RTP, /**< Standards-compliant RTP */
+    RTSP_TRANSPORT_RDT, /**< Realmedia Data Transport */
     /**
      * This is not part of the public API and shouldn't be used outside ffmpeg.
      */
@@ -54,6 +54,10 @@
 #define RTSP_RTP_PORT_MIN 5000
 #define RTSP_RTP_PORT_MAX 10000
 
+/**
+ * This describes the Transport: line of a single stream as negotiated
+ * by the SETUP RTSP command.
+ */
 typedef struct RTSPTransportField {
     int interleaved_min, interleaved_max;  /**< interleave ids, if TCP transport */
     int port_min, port_max; /**< RTP ports */
@@ -61,10 +65,13 @@
     int server_port_min, server_port_max; /**< RTP ports */
     int ttl; /**< ttl value */
     uint32_t destination; /**< destination IP address */
-    enum RTSPTransport transport;
-    enum RTSPLowerTransport lower_transport;
+    enum RTSPTransport transport;            /**< RTP or RDT (Real) */
+    enum RTSPLowerTransport lower_transport; /**< TCP/UDP uni-/multicast */
 } RTSPTransportField;
 
+/**
+ * This describes the server response to each RTSP command.
+ */
 typedef struct RTSPHeader {
     int content_length;
     enum RTSPStatusCode status_code; /**< response code from server */
@@ -73,9 +80,11 @@
     int64_t range_start, range_end;
     RTSPTransportField transports[RTSP_MAX_TRANSPORTS];
     int seq; /**< sequence number */
-    char session_id[512];
+    char session_id[512];            /**< The Session field, value is set
+                                      * by the server and re-transmitted by
+                                      * the client in every RTSP command. */
     char real_challenge[64]; /**< the RealChallenge1 field from the server */
-    char server[64];
+    char server[64];                 /**< the Server field, to identify WMS */
 } RTSPHeader;
 
 enum RTSPClientState {
@@ -84,6 +93,10 @@
     RTSP_STATE_PAUSED,
 };
 
+/**
+ * Identifies particular servers that require special handling, such as
+ * standards-incompliant Transport: lines in the SETUP request.
+ */
 enum RTSPServerType {
     RTSP_SERVER_RTP,  /**< Standards-compliant RTP-server */
     RTSP_SERVER_REAL, /**< Realmedia-style server */
@@ -91,28 +104,44 @@
     RTSP_SERVER_LAST
 };
 
+/**
+ * Private data for the RTSP demuxer.
+ */
 typedef struct RTSPState {
     URLContext *rtsp_hd; /* RTSP TCP connexion handle */
     int nb_rtsp_streams;
-    struct RTSPStream **rtsp_streams;
+    struct RTSPStream **rtsp_streams; /**< streams in this session */
 
-    enum RTSPClientState state;
-    int64_t seek_timestamp;
+    enum RTSPClientState state;       /**< whether we are currently receiving
+                                       * data or not from the server */
+    int64_t seek_timestamp;           /**< the seek value requested when
+                                       * calling av_seek_frame(). This way,
+                                       * the seek value is saved if we are
+                                       * currently paused, and it can be
+                                       * transmitted at the next PLAY.
+                                       * See rtsp_read_play(). */
 
     /* XXX: currently we use unbuffered input */
     //    ByteIOContext rtsp_gb;
     int seq;        /* RTSP command sequence number */
-    char session_id[512];
-    enum RTSPTransport transport;
-    enum RTSPLowerTransport lower_transport;
-    enum RTSPServerType server_type;
+    char session_id[512];             /**< same as RTSPHeader->session_id */
+    enum RTSPTransport transport;     /**< RTP or RDT */
+    enum RTSPLowerTransport lower_transport; /**< TCP/UDP uni-/multicast */
+    enum RTSPServerType server_type;  /**< brand of server (WMS/REAL/other) */
     char last_reply[2048]; /* XXX: allocate ? */
-    void *cur_tx;
+    void *cur_tx;                     /**< RTSPStream->tx_ctx of the last
+                                       * stream that we read a packet from. */
     int need_subscription;
     enum AVDiscard real_setup_cache[MAX_STREAMS];
     char last_subscription[1024];
 } RTSPState;
 
+/**
+ * Describes a single stream, as identified by a single m= line block in the
+ * SDP content. In the case of RDT, one RTSPStream can represent multiple
+ * AVStreams. In this case, each AVStream in this set has similar content
+ * (but different codec/bitrate).
+ */
 typedef struct RTSPStream {
     URLContext *rtp_handle; /* RTP stream handle */
     void *tx_ctx; /* RTP/RDT parse context */
-------------- next part --------------
Index: ffmpeg-svn/libavformat/rtsp.h
===================================================================
--- ffmpeg-svn.orig/libavformat/rtsp.h	2009-02-04 08:57:24.000000000 -0500
+++ ffmpeg-svn/libavformat/rtsp.h	2009-02-04 08:58:27.000000000 -0500
@@ -59,12 +59,12 @@
  * by the SETUP RTSP command.
  */
 typedef struct RTSPTransportField {
-    int interleaved_min, interleaved_max;  /**< interleave ids, if TCP transport */
-    int port_min, port_max; /**< RTP ports */
-    int client_port_min, client_port_max; /**< RTP ports */
-    int server_port_min, server_port_max; /**< RTP ports */
-    int ttl; /**< ttl value */
-    uint32_t destination; /**< destination IP address */
+    int interleaved_min, interleaved_max;    /**< interleave ids, if TCP transport */
+    int port_min, port_max;                  /**< RTP ports */
+    int client_port_min, client_port_max;    /**< RTP ports */
+    int server_port_min, server_port_max;    /**< RTP ports */
+    int ttl;                                 /**< ttl value */
+    uint32_t destination;                    /**< destination IP address */
     enum RTSPTransport transport;            /**< RTP or RDT (Real) */
     enum RTSPLowerTransport lower_transport; /**< TCP/UDP uni-/multicast */
 } RTSPTransportField;
@@ -79,11 +79,11 @@
     /** in AV_TIME_BASE unit, AV_NOPTS_VALUE if not used */
     int64_t range_start, range_end;
     RTSPTransportField transports[RTSP_MAX_TRANSPORTS];
-    int seq; /**< sequence number */
+    int seq;                         /**< sequence number */
     char session_id[512];            /**< The Session field, value is set
                                       * by the server and re-transmitted by
                                       * the client in every RTSP command. */
-    char real_challenge[64]; /**< the RealChallenge1 field from the server */
+    char real_challenge[64];         /**< the RealChallenge1 field from the server */
     char server[64];                 /**< the Server field, to identify WMS */
 } RTSPHeader;
 
@@ -108,7 +108,7 @@
  * Private data for the RTSP demuxer.
  */
 typedef struct RTSPState {
-    URLContext *rtsp_hd; /* RTSP TCP connexion handle */
+    URLContext *rtsp_hd;              /**< RTSP TCP connection handle */
     int nb_rtsp_streams;
     struct RTSPStream **rtsp_streams; /**< streams in this session */
 
@@ -123,12 +123,12 @@
 
     /* XXX: currently we use unbuffered input */
     //    ByteIOContext rtsp_gb;
-    int seq;        /* RTSP command sequence number */
+    int seq;                          /**< RTSP command sequence number */
     char session_id[512];             /**< same as RTSPHeader->session_id */
     enum RTSPTransport transport;     /**< RTP or RDT */
     enum RTSPLowerTransport lower_transport; /**< TCP/UDP uni-/multicast */
     enum RTSPServerType server_type;  /**< brand of server (WMS/REAL/other) */
-    char last_reply[2048]; /* XXX: allocate ? */
+    char last_reply[2048];            /* XXX: allocate ? */
     void *cur_tx;                     /**< RTSPStream->tx_ctx of the last
                                        * stream that we read a packet from. */
     int need_subscription;
@@ -143,12 +143,13 @@
  * (but different codec/bitrate).
  */
 typedef struct RTSPStream {
-    URLContext *rtp_handle; /* RTP stream handle */
-    void *tx_ctx; /* RTP/RDT parse context */
+    URLContext *rtp_handle;                /**< RTP stream handle */
+    void *tx_ctx;                          /**< RTP/RDT parse context */
 
-    int stream_index; /* corresponding stream index, if any. -1 if none (MPEG2TS case) */
-    int interleaved_min, interleaved_max;  /* interleave ids, if TCP transport */
-    char control_url[1024]; /* url for this stream (from SDP) */
+    int stream_index;                      /**< corresponding stream index,
+                                            * -1 if none (MPEG2TS case) */
+    int interleaved_min, interleaved_max;  /**< interleave ids (TCP) */
+    char control_url[1024];                /**< stream url (from SDP) */
 
     int sdp_port; /* port (from SDP content - not used in RTSP) */
     struct in_addr sdp_ip; /* IP address  (from SDP content - not used in RTSP) */
-------------- next part --------------
Index: ffmpeg-svn/libavformat/rtsp.h
===================================================================
--- ffmpeg-svn.orig/libavformat/rtsp.h	2009-02-04 08:58:27.000000000 -0500
+++ ffmpeg-svn/libavformat/rtsp.h	2009-02-04 08:58:32.000000000 -0500
@@ -151,14 +151,21 @@
     int interleaved_min, interleaved_max;  /**< interleave ids (TCP) */
     char control_url[1024];                /**< stream url (from SDP) */
 
-    int sdp_port; /* port (from SDP content - not used in RTSP) */
-    struct in_addr sdp_ip; /* IP address  (from SDP content - not used in RTSP) */
-    int sdp_ttl;  /* IP TTL (from SDP content - not used in RTSP) */
-    int sdp_payload_type; /* payload type - only used in SDP */
-    RTPPayloadData rtp_payload_data; /* rtp payload parsing infos from SDP */
-
-    RTPDynamicProtocolHandler *dynamic_handler; ///< Only valid if it's a dynamic protocol. (This is the handler structure)
-    PayloadContext *dynamic_protocol_context; ///< Only valid if it's a dynamic protocol. (This is any private data associated with the dynamic protocol)
+    /** The following are used only in SDP, not in RTSP */
+    //@{
+    int sdp_port;
+    struct in_addr sdp_ip;
+    int sdp_ttl;
+    int sdp_payload_type;
+    //@}
+    RTPPayloadData rtp_payload_data;       /**< rtp payload parsing infos
+                                            * from SDP */
+
+    /** The following are only valid for dynamic protocols */
+    //@{
+    RTPDynamicProtocolHandler *dynamic_handler;
+    PayloadContext *dynamic_protocol_context;
+    //@}
 } RTSPStream;
 
 /** the callback can be used to extend the connection setup/teardown step */



More information about the ffmpeg-devel mailing list