curl*sink: post error on bus in element, not transfer thread

https://bugzilla.gnome.org/show_bug.cgi?id=728960

7 files changed, 49 insertions, 53 deletions

diff --git a/ext/curl/gstcurlbasesink.c b/ext/curl/gstcurlbasesink.c
index d3f150bd2..d0a3ebf6d 100644
--- a/ext/curl/gstcurlbasesink.c
+++ b/ext/curl/gstcurlbasesink.c
@@ -249,6 +249,7 @@ gst_curl_base_sink_init (GstCurlBaseSink * sink)
   sink->url = g_strdup (DEFAULT_URL);
   sink->transfer_thread_close = FALSE;
   sink->new_file = TRUE;
+  sink->error = NULL;
   sink->flow_ret = GST_FLOW_OK;
   sink->is_live = FALSE;
 }
@@ -338,16 +339,18 @@ gst_curl_base_sink_render (GstBaseSink * bsink, GstBuffer * buf)
   guint8 *data;
   size_t size;
   GstFlowReturn ret;
+  gchar *error;
 
   GST_LOG ("enter render");
 
   sink = GST_CURL_BASE_SINK (bsink);
+
+  GST_OBJECT_LOCK (sink);
+
   gst_buffer_map (buf, &map, GST_MAP_READ);
   data = map.data;
   size = map.size;
 
-  GST_OBJECT_LOCK (sink);
-
   /* check if the transfer thread has encountered problems while the
    * pipeline thread was working elsewhere */
   if (sink->flow_ret != GST_FLOW_OK) {
@@ -376,9 +379,19 @@ gst_curl_base_sink_render (GstBaseSink * bsink, GstBuffer * buf)
   gst_curl_base_sink_wait_for_transfer_thread_to_send_unlocked (sink);
 
 done:
+  gst_buffer_unmap (buf, &map);
+
+  /* Hand over error from transfer thread to streaming thread */
+  error = sink->error;
+  sink->error = NULL;
   ret = sink->flow_ret;
   GST_OBJECT_UNLOCK (sink);
-  gst_buffer_unmap (buf, &map);
+
+  if (error != NULL) {
+    GST_ERROR_OBJECT (sink, "%s", error);
+    GST_ELEMENT_ERROR (sink, RESOURCE, WRITE, ("%s", error), (NULL));
+    g_free (error);
+  }
 
   GST_LOG ("exit render");
 
@@ -841,14 +854,13 @@ handle_transfer (GstCurlBaseSink * sink)
 
         goto fail;
       } else {
-        GST_DEBUG_OBJECT (sink, "poll failed: %s", g_strerror (errno));
-        GST_ELEMENT_ERROR (sink, RESOURCE, WRITE, ("poll failed"), (NULL));
+        sink->error = g_strdup_printf ("poll failed: %s", g_strerror (errno));
         retval = GST_FLOW_ERROR;
         goto fail;
       }
     } else if (G_UNLIKELY (activated_fds == 0)) {
-      GST_DEBUG_OBJECT (sink, "poll timed out");
-      GST_ELEMENT_ERROR (sink, RESOURCE, WRITE, ("poll timed out"), (NULL));
+      sink->error = g_strdup_printf ("poll timed out after %" GST_TIME_FORMAT,
+          GST_TIME_ARGS (timeout * GST_SECOND));
       retval = GST_FLOW_ERROR;
       goto fail;
     }
@@ -860,9 +872,8 @@ handle_transfer (GstCurlBaseSink * sink)
   }
 
   if (m_code != CURLM_OK) {
-    GST_DEBUG_OBJECT (sink, "curl multi error");
-    GST_ELEMENT_ERROR (sink, RESOURCE, WRITE, ("%s",
-            curl_multi_strerror (m_code)), (NULL));
+    sink->error = g_strdup_printf ("failed to write data: %s",
+        curl_multi_strerror (m_code));
     retval = GST_FLOW_ERROR;
     goto fail;
   }
@@ -870,9 +881,8 @@ handle_transfer (GstCurlBaseSink * sink)
   /* problems still might have occurred on individual transfers even when
    * curl_multi_perform returns CURLM_OK */
   if ((e_code = gst_curl_base_sink_transfer_check (sink)) != CURLE_OK) {
-    GST_DEBUG_OBJECT (sink, "curl easy error");
-    GST_ELEMENT_ERROR (sink, RESOURCE, WRITE, ("%s",
-            curl_easy_strerror (e_code)), (NULL));
+    sink->error = g_strdup_printf ("failed to transfer data: %s",
+        curl_easy_strerror (e_code));
     retval = GST_FLOW_ERROR;
     goto fail;
   }
@@ -966,8 +976,7 @@ gst_curl_base_sink_transfer_thread_func (gpointer data)
   GST_LOG ("transfer thread started");
   GST_OBJECT_LOCK (sink);
   if (!gst_curl_base_sink_transfer_setup_unlocked (sink)) {
-    GST_DEBUG_OBJECT (sink, "curl setup error");
-    GST_ELEMENT_ERROR (sink, RESOURCE, WRITE, ("curl setup error"), (NULL));
+    /* no need to set sink->error, as it is set by the called function */
     sink->flow_ret = GST_FLOW_ERROR;
     goto done;
   }
@@ -983,11 +992,8 @@ gst_curl_base_sink_transfer_thread_func (gpointer data)
     data_available = gst_curl_base_sink_wait_for_data_unlocked (sink);
     if (data_available) {
       if (G_UNLIKELY (!klass->set_protocol_dynamic_options_unlocked (sink))) {
+        sink->error = g_strdup ("unexpected state");
         sink->flow_ret = GST_FLOW_ERROR;
-        GST_OBJECT_UNLOCK (sink);
-        GST_ELEMENT_ERROR (sink, RESOURCE, FAILED, ("Unexpected state."),
-            (NULL));
-        GST_OBJECT_LOCK (sink);
         goto done;
       }
     }
@@ -1057,20 +1063,20 @@ gst_curl_base_sink_transfer_setup_unlocked (GstCurlBaseSink * sink)
   if (sink->curl == NULL) {
     /* curl_easy_init automatically calls curl_global_init(3) */
     if ((sink->curl = curl_easy_init ()) == NULL) {
-      g_warning ("Failed to init easy handle");
+      sink->error = g_strdup ("failed to init curl easy handle");
       return FALSE;
     }
   }
 
   if (!gst_curl_base_sink_transfer_set_options_unlocked (sink)) {
-    g_warning ("Failed to setup easy handle");
-    GST_OBJECT_UNLOCK (sink);
+    sink->error = g_strdup ("failed to setup curl easy handle");
     return FALSE;
   }
 
   /* init a multi stack (non-blocking interface to libcurl) */
   if (sink->multi_handle == NULL) {
     if ((sink->multi_handle = curl_multi_init ()) == NULL) {
+      sink->error = g_strdup ("failed to init curl multi handle");
       return FALSE;
     }
   }
diff --git a/ext/curl/gstcurlbasesink.h b/ext/curl/gstcurlbasesink.h
index ce5214243..51059817b 100644
--- a/ext/curl/gstcurlbasesink.h
+++ b/ext/curl/gstcurlbasesink.h
@@ -68,6 +68,7 @@ struct _GstCurlBaseSink
   GstPollFD fd;
   GstPoll *fdset;
   GThread *transfer_thread;
+  gchar *error;
   GstFlowReturn flow_ret;
   TransferBuffer *transfer_buf;
   TransferCondition *transfer_cond;
diff --git a/ext/curl/gstcurlfilesink.c b/ext/curl/gstcurlfilesink.c
index a39a000ad..dfaca20c2 100644
--- a/ext/curl/gstcurlfilesink.c
+++ b/ext/curl/gstcurlfilesink.c
@@ -213,9 +213,7 @@ gst_curl_file_sink_prepare_transfer (GstCurlBaseSink * basesink)
     gchar *url = g_strdup_printf ("%s%s", basesink->url, basesink->file_name);
     file_name = g_filename_from_uri (url, NULL, NULL);
     if (file_name == NULL) {
-      GST_DEBUG_OBJECT (sink, "failed to parse file name of '%s'", url);
-      GST_ELEMENT_ERROR (sink, RESOURCE, WRITE, ("failed to parse file name"),
-          (NULL));
+      basesink->error = g_strdup_printf ("failed to parse file name '%s'", url);
       g_free (url);
       return FALSE;
     }
@@ -226,9 +224,8 @@ gst_curl_file_sink_prepare_transfer (GstCurlBaseSink * basesink)
       /* create dir if file name contains dir component */
       gchar *dir_name = g_strndup (file_name, last_slash - file_name);
       if (g_mkdir_with_parents (dir_name, S_IRWXU) < 0) {
-        GST_DEBUG_OBJECT (sink, "failed to create directory '%s'", dir_name);
-        GST_ELEMENT_ERROR (sink, RESOURCE, WRITE,
-            ("failed to create directory"), (NULL));
+        basesink->error = g_strdup_printf ("failed to create directory '%s'",
+            dir_name);
         g_free (file_name);
         g_free (dir_name);
         return FALSE;
diff --git a/ext/curl/gstcurlftpsink.c b/ext/curl/gstcurlftpsink.c
index bb1e9cb51..114bc97be 100644
--- a/ext/curl/gstcurlftpsink.c
+++ b/ext/curl/gstcurlftpsink.c
@@ -249,12 +249,8 @@ set_ftp_options_unlocked (GstCurlBaseSink * basesink)
         sink->ftp_port_arg);
 
     if (res != CURLE_OK) {
-      GST_DEBUG_OBJECT (sink, "Failed to set up active mode: %s",
+      basesink->error = g_strdup_printf ("failed to set up active mode: %s",
           curl_easy_strerror (res));
-      GST_ELEMENT_ERROR (sink, RESOURCE, WRITE,
-          ("Failed to set up active mode: %s", curl_easy_strerror (res)),
-          (NULL));
-
       return FALSE;
     }
 
diff --git a/ext/curl/gstcurlhttpsink.c b/ext/curl/gstcurlhttpsink.c
index 61704f7e4..0f46c8832 100644
--- a/ext/curl/gstcurlhttpsink.c
+++ b/ext/curl/gstcurlhttpsink.c
@@ -397,9 +397,8 @@ gst_curl_http_sink_transfer_verify_response_code (GstCurlBaseSink * bcsink)
   GST_DEBUG_OBJECT (sink, "response code: %ld", resp);
 
   if (resp < 100 || resp >= 300) {
-    GST_ELEMENT_ERROR (sink, RESOURCE, WRITE,
-        ("HTTP response error: (received: %ld)", resp), (NULL));
-
+    bcsink->error = g_strdup_printf ("HTTP response error: (received: %ld)",
+        resp);
     return FALSE;
   }
 
diff --git a/ext/curl/gstcurlsftpsink.c b/ext/curl/gstcurlsftpsink.c
index 2a68d3afc..831c11000 100644
--- a/ext/curl/gstcurlsftpsink.c
+++ b/ext/curl/gstcurlsftpsink.c
@@ -137,13 +137,14 @@ gst_curl_sftp_sink_finalize (GObject * gobject)
 static gboolean
 set_sftp_dynamic_options_unlocked (GstCurlBaseSink * basesink)
 {
-  GstCurlSftpSink *sink = GST_CURL_SFTP_SINK (basesink);
   gchar *tmp = g_strdup_printf ("%s%s", basesink->url, basesink->file_name);
-  gint curl_err = CURLE_OK;
+  CURLcode curl_err = CURLE_OK;
 
   if ((curl_err =
           curl_easy_setopt (basesink->curl, CURLOPT_URL, tmp)) != CURLE_OK) {
-    GST_ERROR_OBJECT (sink, "curl error: %d setting URL to: %s", curl_err, tmp);
+    basesink->error = g_strdup_printf ("failed to set URL: %s",
+        curl_easy_strerror (curl_err));
+    return FALSE;
   }
 
   g_free (tmp);
@@ -156,19 +157,22 @@ set_sftp_options_unlocked (GstCurlBaseSink * basesink)
 {
   GstCurlSftpSink *sink = GST_CURL_SFTP_SINK (basesink);
   GstCurlSshSinkClass *parent_class;
-  gint curl_err = CURLE_OK;
+  CURLcode curl_err = CURLE_OK;
 
   if ((curl_err =
           curl_easy_setopt (basesink->curl, CURLOPT_UPLOAD, 1L)) != CURLE_OK) {
-    GST_ERROR_OBJECT (sink, "curl error: %d setting CURLOPT_UPLOAD to 1",
-        curl_err);
+    basesink->error = g_strdup_printf ("failed to prepare for upload: %s",
+        curl_easy_strerror (curl_err));
+    return FALSE;
   }
 
   if (sink->create_dirs) {
     if ((curl_err = curl_easy_setopt (basesink->curl,
                 CURLOPT_FTP_CREATE_MISSING_DIRS, 1L)) != CURLE_OK) {
-      GST_ERROR_OBJECT (sink,
-          "curl error: %d setting FTP_CREATE_MISSING_DIRS to 1", curl_err);
+      basesink->error =
+          g_strdup_printf ("failed to set create missing dirs: %s",
+          curl_easy_strerror (curl_err));
+      return FALSE;
     }
   }
 
diff --git a/ext/curl/gstcurlsmtpsink.c b/ext/curl/gstcurlsmtpsink.c
index 9ee3682c8..e1ea5314a 100644
--- a/ext/curl/gstcurlsmtpsink.c
+++ b/ext/curl/gstcurlsmtpsink.c
@@ -941,7 +941,6 @@ transfer_payload_headers (GstCurlSmtpSink * sink,
   return bytes_to_send;
 }
 
-
 static gboolean
 gst_curl_smtp_sink_prepare_transfer (GstCurlBaseSink * bcsink)
 {
@@ -951,10 +950,7 @@ gst_curl_smtp_sink_prepare_transfer (GstCurlBaseSink * bcsink)
 
   if (sink->pop_location && strlen (sink->pop_location)) {
     if ((sink->pop_curl = curl_easy_init ()) == NULL) {
-      GST_DEBUG_OBJECT (sink, "POP protocol: failed to create handler");
-      GST_ELEMENT_ERROR (sink, RESOURCE, WRITE,
-          ("POP protocol: failed to create handler"), (NULL));
-
+      bcsink->error = g_strdup ("POP protocol: failed to create handler");
       return FALSE;
     }
 
@@ -973,11 +969,8 @@ gst_curl_smtp_sink_prepare_transfer (GstCurlBaseSink * bcsink)
   /* ready to initialize connection to POP server */
   res = curl_easy_perform (sink->pop_curl);
   if (res != CURLE_OK) {
-    GST_DEBUG_OBJECT (sink, "POP transfer failed: %s",
+    bcsink->error = g_strdup_printf ("POP transfer failed: %s",
         curl_easy_strerror (res));
-    GST_ELEMENT_ERROR (sink, RESOURCE, WRITE, ("POP transfer failed: %s",
-            curl_easy_strerror (res)), (NULL));
-
     ret = FALSE;
   }