[Bug 709795] curlsftpsink - new libcurl-based sink element for SFTP

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=709795
  GStreamer | gst-plugins-bad | git

Sebastian Dröge (slomo) <slomo> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #256880|none                        |needs-work
             status|                            |

--- Comment #1 from Sebastian Dröge (slomo) <slomo at circular-chaos.org> 2013-10-10 12:40:17 UTC ---
Review of attachment 256880:
 --> (https://bugzilla.gnome.org/review?bug=709795&attachment=256880)

::: ext/curl/Makefile.am
@@ +1,3 @@
 plugin_LTLIBRARIES = libgstcurl.la

+if USE_SSH2

You should probably add the SSH2 CFLAGS and LIBS to the CFLAGS and LIBADD
variables too

::: ext/curl/gstcurlsftpsink.c
@@ +31,3 @@
+ * gst-launch filesrc location=/home/jdoe/some.file ! curlsftpsink  \
+ *     file-name=some.file.backup  \
+ *     user=john location=sftp://192.168.0.1/~/sftp_tests/  \

Why not combine location and filename?

::: ext/curl/gstcurlsshsink.c
@@ +141,3 @@
+
+  g_object_class_install_property (gobject_class, PROP_SSH_PVT_KEYFILE,
+      g_param_spec_string ("ssh-pvt-keyfile",

pvt -> priv?

@@ +201,3 @@
+
+  gst_element_get_state (GST_ELEMENT (sink), &cur_state, NULL, 0);
+  if (cur_state != GST_STATE_PLAYING && cur_state != GST_STATE_PAUSED) {

maybe inverse this and return, instead of indenting the complete block below

@@ +371,3 @@
+   * is also set! */
+  if ((curl_err = curl_easy_setopt (bcsink->curl, CURLOPT_SSH_KEYFUNCTION,
+              klass->curl_sshkey_cb)) != CURLE_OK) {

Why is the callback stored in the class?

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.