[vlc-devel] [PATCH] Integrate the RTMPdump library to play rtmp and rtmpe streams.

[Rémi Denis-Courmont]

	Hello,

comments inline

diff --git a/configure.ac b/configure.ac
index 1a428e7..74bc9e5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2917,6 +2917,18 @@ if test "${enable_realrtsp}" = "yes"; then
 fi
 
 dnl
+dnl  RTMPDump access plugin
+dnl
+AC_ARG_ENABLE(rtmpdump,
+  [  --enable-rtmpdump       RTMPDump access plugin (default disabled)])
+if test "${enable_rtmpdump}" = "yes"; then
+  AC_CHECK_HEADERS(librtmp/rtmp.h, [
+    VLC_ADD_PLUGIN([access_rtmpdump])
+    VLC_ADD_LIBS([access_rtmpdump],[-lz -lssl -lcrypto -lrtmp])
+  ] )
+fi

Does librtmp not support pkg-config? Is it always a static library (otherwise 
why do we need zlib and OpenSSL)?

This will require GPLv3, oh well.

diff --git a/modules/access/rtmpdump/Modules.am 
b/modules/access/rtmpdump/Modules.am
new file mode 100644
index 0000000..7bb006a
--- /dev/null
+++ b/modules/access/rtmpdump/Modules.am
@@ -0,0 +1,10 @@
+libaccess_rtmpdump_plugin_la_SOURCES = \
+	rtmpdump.c \
+	access.c \
+	$(NULL)
+
+libaccess_rtmpdump_plugin_la_CFLAGS = $(AM_CFLAGS)
+libaccess_rtmpdump_plugin_la_LIBADD = $(AM_LIBADD)
+libaccess_rtmpdump_plugin_la_DEPENDENCIES =
+
+libvlc_LTLIBRARIES += libaccess_rtmpdump_plugin.la

That will build the plugin inconditionally which is probably wrong.
I guess it should instead look like:

libvlc_LTLIBRARIES += $(LTLIBaccess_rtmpdump)
EXTRA_LTLIBRARIES += libaccess_rtmpdump_plugin.la

diff --git a/modules/access/rtmpdump/access.c 
b/modules/access/rtmpdump/access.c

+#define SOCKSHOST_TEXT N_("Use the specified SOCKS4 proxy")
+#define SOCKSHOST_LONGTEXT N_( \
+    "Use the specified SOCKS4 proxy. " \
+    "By default no proxy is used." )

VLC already defines --socks for this. Do you really need a new option?

diff --git a/modules/access/rtmpdump/access.h 
b/modules/access/rtmpdump/access.h

+#ifndef _ACCESS_H_
+#define _ACCESS_H_ 1

That's a bit too generic macro name IMHO. Also you should probably not start 
with an underscore, as per the C standard.

diff --git a/modules/access/rtmpdump/rtmpdump.c 
b/modules/access/rtmpdump/rtmpdump.c
+FILE *netstackdump = 0;
+FILE *netstackdump_read = 0;

NULL rather than 0.

+    /* set instance variables from options */
+    psz_tmp = var_CreateGetNonEmptyString( p_access, "rtmp-sockshost" );
+    STR2AVAL( p_sys->sockshost, psz_tmp );

Should use var_InheritString() instead of var_CreateGetNonEmptyString().
Also, you need to free() the return value somewhere.

+    psz_tmp = var_CreateGetNonEmptyString( p_access, "rtmp-flashver" );
+    STR2AVAL( p_sys->flashver, psz_tmp );
+
+    psz_tmp = var_CreateGetNonEmptyString( p_access, "rtmp-hostname" );
+    STR2AVAL( p_sys->hostname, psz_tmp );
+
+    psz_tmp = var_CreateGetNonEmptyString( p_access, "rtmp-playpath" );
+    STR2AVAL( p_sys->playpath, psz_tmp );
+
+    psz_tmp = var_CreateGetNonEmptyString( p_access, "rtmp-swfurl" );
+    STR2AVAL( p_sys->swfurl, psz_tmp );
+
+    psz_tmp = var_CreateGetNonEmptyString( p_access, "rtmp-pageurl" );
+    STR2AVAL( p_sys->pageurl, psz_tmp );
+
+    psz_tmp = var_CreateGetNonEmptyString( p_access, "rtmp-app" );
+    STR2AVAL( p_sys->app, psz_tmp );
+
+    psz_tmp = var_CreateGetNonEmptyString( p_access, "rtmp-auth" );
+    STR2AVAL( p_sys->auth, psz_tmp );
+
+    psz_tmp = var_CreateGetNonEmptyString( p_access, "rtmp-swfhash" );
+    STR2AVAL( p_sys->swfhash, psz_tmp );
+
+    psz_tmp = var_CreateGetNonEmptyString( p_access, "rtmp-subscribepath" );
+    STR2AVAL( p_sys->subscribepath, psz_tmp );

Same for all of these.

+
+    p_sys->b_livestream  = var_CreateGetBool( p_access, "rtmp-livestream" );
+    p_sys->i_protocol    = var_CreateGetInteger( p_access, "rtmp-protocol" );
+    p_sys->i_port        = var_CreateGetInteger( p_access, "rtmp-port" );
+    p_sys->ui_swfsize    = var_CreateGetInteger( p_access, "rtmp-swfsize" );
+    p_sys->ui_seek       = var_CreateGetInteger( p_access, "rtmp-seek" );
+    p_sys->ui_stopoffset = var_CreateGetInteger( p_access, "rtmp-stopoffset" );
+    p_sys->l_timeout     = var_CreateGetInteger( p_access, "rtmp-timeout" );
+    p_sys->i_caching     = var_CreateGetInteger( p_access, "rtmp-caching" );

Use var_InheritBool() and var_InheritInteger().

+#ifdef _DEBUG
+    if ( netstackdump != 0 )
+      fclose( netstackdump );
+    if ( netstackdump_read != 0 )
+      fclose( netstackdump_read );
+#endif

Use NULL.

diff --git a/modules/access/rtmpdump/rtmpdump.h 
b/modules/access/rtmpdump/rtmpdump.h
+#ifndef _RTMPDUMP_H_
+#define _RTMPDUMP_H_ 1

This is quite generic again.


-- 
Rémi Denis-Courmont