cURL / Mailing Lists / curl-library / Single Mail

curl-library

librtmp support

From: Howard Chu <hyc_at_highlandsun.com>
Date: Sat, 01 May 2010 20:04:40 -0700

I started looking into adding librtmp to libcurl, but I have a few concerns
about the library structure. E.g.

diff --git a/include/curl/curl.h b/include/curl/curl.h
index e635968..41b6ae9 100644
--- a/include/curl/curl.h
+++ b/include/curl/curl.h
@@ -623,6 +623,11 @@ typedef enum {
  #define CURLPROTO_SMTP (1<<16)
  #define CURLPROTO_SMTPS (1<<17)
  #define CURLPROTO_RTSP (1<<18)
+#define CURLPROTO_RTMP (1<<19)
+#define CURLPROTO_RTMPT (1<<20)
+#define CURLPROTO_RTMPE (1<<21)
+#define CURLPROTO_RTMPTE (1<<22)
+#define CURLPROTO_RTMPS (1<<23)
  #define CURLPROTO_ALL (~0) /* enable everything */

  /* long may be 32 or 64 bits, but we should never depend on anything else
diff --git a/lib/url.c b/lib/url.c
index 56dd5dc..477af5f 100644
--- a/lib/url.c
+++ b/lib/url.c
@@ -137,6 +137,7 @@ void idn_free (void *ptr); /* prototype from idn-free.h, not
  #include "http_ntlm.h"
  #include "socks.h"
  #include "rtsp.h"
+#include "curl_rtmp.h"

  #define _MPRINTF_REPLACE /* use our functions only */
  #include <curl/mprintf.h>
@@ -231,6 +232,14 @@ static const struct Curl_handler * const protocols[] = {
    &Curl_handler_rtsp,
  #endif

+#ifndef CURL_DISABLE_RTMP
+ &Curl_handler_rtmp,
+ &Curl_handler_rtmpt,
+ &Curl_handler_rtmpe,
+ &Curl_handler_rtmpte,
+ &Curl_handler_rtmps,
+#endif
+
    (struct Curl_handler *) NULL
  };

It strikes me that using an enum for the base protocol types would have been
better than using a bitmask. You're going to run out of bits very soon.
(Judging from this in urldata.h)
>>>>
/* (1<<18) is currently the highest used bit in the public bitmask. We make
    sure we use "private bits" above the public ones to make things easier. */

#define PROT_EXTMASK 0xfffff

#define PROT_SSL (1<<25) /* protocol requires SSL */
<<<<

Also from a coding practices standpoint, bitmasks should be used for options
that may potentially be used together. Protocol types are strictly exclusive;
you cannot have a connection be both HTTP and FTP simultaneously. An enum
would embody that exclusivity, while the current bitmap arrangement allows for
nonsense combinations.

On a related note, it seems redundant to need a separate Curl_handler struct
for each variant of the base protocol. The only difference in each of these
variants is that some new I/O layer is wrapped around the base layer. Once you
setup the wrapper at connect time, there's no further difference between the
variants.

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette:  http://curl.haxx.se/mail/etiquette.html
Received on 2010-05-02