From 02317496c6f3334e1fb2eae72662d04db51d5851 Mon Sep 17 00:00:00 2001 From: Arnaud Vrac Date: Mon, 16 Jan 2017 11:58:02 +0100 Subject: [PATCH] souphttpsrc: make flow return values handling clearer The flow return values was stored in the element before because the result had to be set from callbacks. This is not the case anymore, we can return the flow result directly from functions, making the code easier to understand. https://bugzilla.gnome.org/show_bug.cgi?id=777222 --- ext/soup/gstsouphttpsrc.c | 107 ++++++++++++++++---------------------- ext/soup/gstsouphttpsrc.h | 1 - 2 files changed, 46 insertions(+), 62 deletions(-) diff --git a/ext/soup/gstsouphttpsrc.c b/ext/soup/gstsouphttpsrc.c index efcb3844ae..b12a727eba 100644 --- a/ext/soup/gstsouphttpsrc.c +++ b/ext/soup/gstsouphttpsrc.c @@ -175,9 +175,9 @@ static gboolean gst_soup_http_src_add_range_header (GstSoupHTTPSrc * src, guint64 offset, guint64 stop_offset); static gboolean gst_soup_http_src_session_open (GstSoupHTTPSrc * src); static void gst_soup_http_src_session_close (GstSoupHTTPSrc * src); -static void gst_soup_http_src_parse_status (SoupMessage * msg, +static GstFlowReturn gst_soup_http_src_parse_status (SoupMessage * msg, GstSoupHTTPSrc * src); -static void gst_soup_http_src_got_headers (GstSoupHTTPSrc * src, +static GstFlowReturn gst_soup_http_src_got_headers (GstSoupHTTPSrc * src, SoupMessage * msg); static void gst_soup_http_src_authenticate_cb (SoupSession * session, SoupMessage * msg, SoupAuth * auth, gboolean retrying, @@ -447,7 +447,6 @@ gst_soup_http_src_reset (GstSoupHTTPSrc * src) src->reduce_blocksize_count = 0; src->increase_blocksize_count = 0; - src->ret = GST_FLOW_OK; g_cancellable_reset (src->cancellable); if (src->input_stream) { g_object_unref (src->input_stream); @@ -1019,12 +1018,13 @@ insert_http_header (const gchar * name, const gchar * value, gpointer user_data) } } -static void +static GstFlowReturn gst_soup_http_src_got_headers (GstSoupHTTPSrc * src, SoupMessage * msg) { const char *value; GstTagList *tag_list; GstBaseSrc *basesrc; + GstFlowReturn ret; guint64 newsize; GHashTable *params = NULL; GstEvent *http_headers_event; @@ -1034,8 +1034,10 @@ gst_soup_http_src_got_headers (GstSoupHTTPSrc * src, SoupMessage * msg) GST_INFO_OBJECT (src, "got headers"); if (msg->status_code == SOUP_STATUS_PROXY_AUTHENTICATION_REQUIRED && - src->proxy_id && src->proxy_pw) - return; + src->proxy_id && src->proxy_pw) { + /* wait for authenticate callback */ + return GST_FLOW_OK; + } if (src->automatic_redirect && soup_session_would_redirect (src->session, msg) && @@ -1048,14 +1050,12 @@ gst_soup_http_src_got_headers (GstSoupHTTPSrc * src, SoupMessage * msg) msg->status_code, src->redirection_uri, src->redirection_permanent); /* force a retry with the updated message */ - src->ret = GST_FLOW_CUSTOM_ERROR; - return; + return GST_FLOW_CUSTOM_ERROR; } if (msg->status_code == SOUP_STATUS_UNAUTHORIZED) { /* force an error */ - gst_soup_http_src_parse_status (msg, src); - return; + return gst_soup_http_src_parse_status (msg, src); } src->got_headers = TRUE; @@ -1239,10 +1239,10 @@ gst_soup_http_src_got_headers (GstSoupHTTPSrc * src, SoupMessage * msg) } /* Handle HTTP errors. */ - gst_soup_http_src_parse_status (msg, src); + ret = gst_soup_http_src_parse_status (msg, src); /* Check if Range header was respected. */ - if (src->ret == GST_FLOW_CUSTOM_ERROR && + if (ret == GST_FLOW_CUSTOM_ERROR && src->read_position && msg->status_code != SOUP_STATUS_PARTIAL_CONTENT) { src->seekable = FALSE; GST_ELEMENT_ERROR_WITH_DETAILS (src, RESOURCE, SEEK, @@ -1252,15 +1252,10 @@ gst_soup_http_src_got_headers (GstSoupHTTPSrc * src, SoupMessage * msg) ("http-status-code", G_TYPE_UINT, msg->status_code, "http-redirection-uri", G_TYPE_STRING, GST_STR_NULL (src->redirection_uri), NULL)); - src->ret = GST_FLOW_ERROR; + ret = GST_FLOW_ERROR; } - /* If we are going to error out, stop all processing right here, so we - * don't output any data (such as an error html page), and return - * GST_FLOW_ERROR from the create function instead of having - * got_chunk_cb overwrite src->ret with FLOW_OK again. */ - if (src->ret == GST_FLOW_ERROR || src->ret == GST_FLOW_EOS) { - } + return ret; } static GstBuffer * @@ -1288,52 +1283,51 @@ gst_soup_http_src_alloc_buffer (GstSoupHTTPSrc * src) "http-redirect-uri", G_TYPE_STRING, GST_STR_NULL ((src)->redirection_uri), NULL)); \ } while(0) -static void +static GstFlowReturn gst_soup_http_src_parse_status (SoupMessage * msg, GstSoupHTTPSrc * src) { if (msg->method == SOUP_METHOD_HEAD) { if (!SOUP_STATUS_IS_SUCCESSFUL (msg->status_code)) GST_DEBUG_OBJECT (src, "Ignoring error %d during HEAD request", msg->status_code); - } else if (SOUP_STATUS_IS_TRANSPORT_ERROR (msg->status_code)) { + return GST_FLOW_OK; + } + + if (SOUP_STATUS_IS_TRANSPORT_ERROR (msg->status_code)) { switch (msg->status_code) { case SOUP_STATUS_CANT_RESOLVE: case SOUP_STATUS_CANT_RESOLVE_PROXY: SOUP_HTTP_SRC_ERROR (src, msg, RESOURCE, NOT_FOUND, _("Could not resolve server name.")); - src->ret = GST_FLOW_ERROR; - break; + return GST_FLOW_ERROR; case SOUP_STATUS_CANT_CONNECT: case SOUP_STATUS_CANT_CONNECT_PROXY: SOUP_HTTP_SRC_ERROR (src, msg, RESOURCE, OPEN_READ, _("Could not establish connection to server.")); - src->ret = GST_FLOW_ERROR; - break; + return GST_FLOW_ERROR; case SOUP_STATUS_SSL_FAILED: SOUP_HTTP_SRC_ERROR (src, msg, RESOURCE, OPEN_READ, _("Secure connection setup failed.")); - src->ret = GST_FLOW_ERROR; - break; + return GST_FLOW_ERROR; case SOUP_STATUS_IO_ERROR: - if (src->max_retries == -1 || src->retry_count < src->max_retries) { - src->ret = GST_FLOW_CUSTOM_ERROR; - } else { - SOUP_HTTP_SRC_ERROR (src, msg, RESOURCE, READ, - _("A network error occurred, or the server closed the connection " - "unexpectedly.")); - src->ret = GST_FLOW_ERROR; - } - break; + if (src->max_retries == -1 || src->retry_count < src->max_retries) + return GST_FLOW_CUSTOM_ERROR; + SOUP_HTTP_SRC_ERROR (src, msg, RESOURCE, READ, + _("A network error occurred, or the server closed the connection " + "unexpectedly.")); + return GST_FLOW_ERROR; case SOUP_STATUS_MALFORMED: SOUP_HTTP_SRC_ERROR (src, msg, RESOURCE, READ, _("Server sent bad data.")); - src->ret = GST_FLOW_ERROR; - break; + return GST_FLOW_ERROR; case SOUP_STATUS_CANCELLED: /* No error message when interrupted by program. */ break; } - } else if (SOUP_STATUS_IS_CLIENT_ERROR (msg->status_code) || + return GST_FLOW_OK; + } + + if (SOUP_STATUS_IS_CLIENT_ERROR (msg->status_code) || SOUP_STATUS_IS_REDIRECTION (msg->status_code) || SOUP_STATUS_IS_SERVER_ERROR (msg->status_code)) { const gchar *reason_phrase; @@ -1353,8 +1347,7 @@ gst_soup_http_src_parse_status (SoupMessage * msg, GstSoupHTTPSrc * src) src->have_body && !src->have_size) { GST_DEBUG_OBJECT (src, "Requested range out of limits and received full " "body, returning EOS"); - src->ret = GST_FLOW_EOS; - return; + return GST_FLOW_EOS; } /* FIXME: reason_phrase is not translated and not suitable for user @@ -1389,8 +1382,10 @@ gst_soup_http_src_parse_status (SoupMessage * msg, GstSoupHTTPSrc * src) "http-redirect-uri", G_TYPE_STRING, GST_STR_NULL (src->redirection_uri), NULL)); } - src->ret = GST_FLOW_ERROR; + return GST_FLOW_ERROR; } + + return GST_FLOW_OK; } static gboolean @@ -1434,6 +1429,8 @@ gst_soup_http_src_build_message (GstSoupHTTPSrc * src, const gchar * method) static GstFlowReturn gst_soup_http_src_send_message (GstSoupHTTPSrc * src) { + GstFlowReturn ret; + g_return_val_if_fail (src->msg != NULL, GST_FLOW_ERROR); /* FIXME We are ignoring the GError here, might be useful to debug */ @@ -1443,11 +1440,9 @@ gst_soup_http_src_send_message (GstSoupHTTPSrc * src) if (g_cancellable_is_cancelled (src->cancellable)) return GST_FLOW_FLUSHING; - src->ret = GST_FLOW_OK; - - gst_soup_http_src_got_headers (src, src->msg); - if (src->ret != GST_FLOW_OK) { - return src->ret; + ret = gst_soup_http_src_got_headers (src, src->msg); + if (ret != GST_FLOW_OK) { + return ret; } if (!src->input_stream) { @@ -1470,8 +1465,7 @@ gst_soup_http_src_do_request (GstSoupHTTPSrc * src, const gchar * method) { if (src->max_retries != -1 && src->retry_count > src->max_retries) { GST_DEBUG_OBJECT (src, "Max retries reached"); - src->ret = GST_FLOW_ERROR; - return src->ret; + return GST_FLOW_ERROR; } src->retry_count++; @@ -1495,13 +1489,10 @@ gst_soup_http_src_do_request (GstSoupHTTPSrc * src, const gchar * method) if (g_cancellable_is_cancelled (src->cancellable)) { GST_INFO_OBJECT (src, "interrupted"); - src->ret = GST_FLOW_FLUSHING; - goto done; + return GST_FLOW_FLUSHING; } - src->ret = gst_soup_http_src_send_message (src); -done: - return src->ret; + return gst_soup_http_src_send_message (src); } /* @@ -1658,7 +1649,7 @@ static GstFlowReturn gst_soup_http_src_create (GstPushSrc * psrc, GstBuffer ** outbuf) { GstSoupHTTPSrc *src; - GstFlowReturn ret = GST_FLOW_OK; + GstFlowReturn ret; GstEvent *http_headers_event = NULL; src = GST_SOUP_HTTP_SRC (psrc); @@ -1778,7 +1769,6 @@ gst_soup_http_src_unlock (GstBaseSrc * bsrc) src = GST_SOUP_HTTP_SRC (bsrc); GST_DEBUG_OBJECT (src, "unlock()"); - src->ret = GST_FLOW_FLUSHING; gst_soup_http_src_cancel_message (src); return TRUE; } @@ -1792,7 +1782,6 @@ gst_soup_http_src_unlock_stop (GstBaseSrc * bsrc) src = GST_SOUP_HTTP_SRC (bsrc); GST_DEBUG_OBJECT (src, "unlock_stop()"); - src->ret = GST_FLOW_OK; g_cancellable_reset (src->cancellable); return TRUE; } @@ -1836,10 +1825,6 @@ gst_soup_http_src_check_seekable (GstSoupHTTPSrc * src) } } } - if (src->ret == GST_FLOW_EOS) { - /* A HEAD request shouldn't lead to EOS */ - src->ret = GST_FLOW_OK; - } g_mutex_unlock (&src->mutex); } } diff --git a/ext/soup/gstsouphttpsrc.h b/ext/soup/gstsouphttpsrc.h index dd01656298..f140f80c5b 100644 --- a/ext/soup/gstsouphttpsrc.h +++ b/ext/soup/gstsouphttpsrc.h @@ -61,7 +61,6 @@ struct _GstSoupHTTPSrc { gchar **cookies; /* HTTP request cookies. */ SoupSession *session; /* Async context. */ SoupMessage *msg; /* Request message. */ - GstFlowReturn ret; /* Return code from callback. */ gint retry_count; /* Number of retries since we received data */ gint max_retries; /* Maximum number of retries */ gchar *method; /* HTTP method */