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
This commit is contained in:
Arnaud Vrac 2017-01-16 11:58:02 +01:00 committed by Thiago Santos
parent 9b23732559
commit 02317496c6
2 changed files with 46 additions and 62 deletions

View file

@ -175,9 +175,9 @@ static gboolean gst_soup_http_src_add_range_header (GstSoupHTTPSrc * src,
guint64 offset, guint64 stop_offset); guint64 offset, guint64 stop_offset);
static gboolean gst_soup_http_src_session_open (GstSoupHTTPSrc * src); 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_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); GstSoupHTTPSrc * src);
static void gst_soup_http_src_got_headers (GstSoupHTTPSrc * src, static GstFlowReturn gst_soup_http_src_got_headers (GstSoupHTTPSrc * src,
SoupMessage * msg); SoupMessage * msg);
static void gst_soup_http_src_authenticate_cb (SoupSession * session, static void gst_soup_http_src_authenticate_cb (SoupSession * session,
SoupMessage * msg, SoupAuth * auth, gboolean retrying, SoupMessage * msg, SoupAuth * auth, gboolean retrying,
@ -447,7 +447,6 @@ gst_soup_http_src_reset (GstSoupHTTPSrc * src)
src->reduce_blocksize_count = 0; src->reduce_blocksize_count = 0;
src->increase_blocksize_count = 0; src->increase_blocksize_count = 0;
src->ret = GST_FLOW_OK;
g_cancellable_reset (src->cancellable); g_cancellable_reset (src->cancellable);
if (src->input_stream) { if (src->input_stream) {
g_object_unref (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) gst_soup_http_src_got_headers (GstSoupHTTPSrc * src, SoupMessage * msg)
{ {
const char *value; const char *value;
GstTagList *tag_list; GstTagList *tag_list;
GstBaseSrc *basesrc; GstBaseSrc *basesrc;
GstFlowReturn ret;
guint64 newsize; guint64 newsize;
GHashTable *params = NULL; GHashTable *params = NULL;
GstEvent *http_headers_event; GstEvent *http_headers_event;
@ -1034,8 +1034,10 @@ gst_soup_http_src_got_headers (GstSoupHTTPSrc * src, SoupMessage * msg)
GST_INFO_OBJECT (src, "got headers"); GST_INFO_OBJECT (src, "got headers");
if (msg->status_code == SOUP_STATUS_PROXY_AUTHENTICATION_REQUIRED && if (msg->status_code == SOUP_STATUS_PROXY_AUTHENTICATION_REQUIRED &&
src->proxy_id && src->proxy_pw) src->proxy_id && src->proxy_pw) {
return; /* wait for authenticate callback */
return GST_FLOW_OK;
}
if (src->automatic_redirect && if (src->automatic_redirect &&
soup_session_would_redirect (src->session, msg) && 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); msg->status_code, src->redirection_uri, src->redirection_permanent);
/* force a retry with the updated message */ /* force a retry with the updated message */
src->ret = GST_FLOW_CUSTOM_ERROR; return GST_FLOW_CUSTOM_ERROR;
return;
} }
if (msg->status_code == SOUP_STATUS_UNAUTHORIZED) { if (msg->status_code == SOUP_STATUS_UNAUTHORIZED) {
/* force an error */ /* force an error */
gst_soup_http_src_parse_status (msg, src); return gst_soup_http_src_parse_status (msg, src);
return;
} }
src->got_headers = TRUE; src->got_headers = TRUE;
@ -1239,10 +1239,10 @@ gst_soup_http_src_got_headers (GstSoupHTTPSrc * src, SoupMessage * msg)
} }
/* Handle HTTP errors. */ /* 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. */ /* 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->read_position && msg->status_code != SOUP_STATUS_PARTIAL_CONTENT) {
src->seekable = FALSE; src->seekable = FALSE;
GST_ELEMENT_ERROR_WITH_DETAILS (src, RESOURCE, SEEK, 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-status-code", G_TYPE_UINT, msg->status_code,
"http-redirection-uri", G_TYPE_STRING, "http-redirection-uri", G_TYPE_STRING,
GST_STR_NULL (src->redirection_uri), NULL)); 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 return ret;
* 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) {
}
} }
static GstBuffer * 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)); \ "http-redirect-uri", G_TYPE_STRING, GST_STR_NULL ((src)->redirection_uri), NULL)); \
} while(0) } while(0)
static void static GstFlowReturn
gst_soup_http_src_parse_status (SoupMessage * msg, GstSoupHTTPSrc * src) gst_soup_http_src_parse_status (SoupMessage * msg, GstSoupHTTPSrc * src)
{ {
if (msg->method == SOUP_METHOD_HEAD) { if (msg->method == SOUP_METHOD_HEAD) {
if (!SOUP_STATUS_IS_SUCCESSFUL (msg->status_code)) if (!SOUP_STATUS_IS_SUCCESSFUL (msg->status_code))
GST_DEBUG_OBJECT (src, "Ignoring error %d during HEAD request", GST_DEBUG_OBJECT (src, "Ignoring error %d during HEAD request",
msg->status_code); 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) { switch (msg->status_code) {
case SOUP_STATUS_CANT_RESOLVE: case SOUP_STATUS_CANT_RESOLVE:
case SOUP_STATUS_CANT_RESOLVE_PROXY: case SOUP_STATUS_CANT_RESOLVE_PROXY:
SOUP_HTTP_SRC_ERROR (src, msg, RESOURCE, NOT_FOUND, SOUP_HTTP_SRC_ERROR (src, msg, RESOURCE, NOT_FOUND,
_("Could not resolve server name.")); _("Could not resolve server name."));
src->ret = GST_FLOW_ERROR; return GST_FLOW_ERROR;
break;
case SOUP_STATUS_CANT_CONNECT: case SOUP_STATUS_CANT_CONNECT:
case SOUP_STATUS_CANT_CONNECT_PROXY: case SOUP_STATUS_CANT_CONNECT_PROXY:
SOUP_HTTP_SRC_ERROR (src, msg, RESOURCE, OPEN_READ, SOUP_HTTP_SRC_ERROR (src, msg, RESOURCE, OPEN_READ,
_("Could not establish connection to server.")); _("Could not establish connection to server."));
src->ret = GST_FLOW_ERROR; return GST_FLOW_ERROR;
break;
case SOUP_STATUS_SSL_FAILED: case SOUP_STATUS_SSL_FAILED:
SOUP_HTTP_SRC_ERROR (src, msg, RESOURCE, OPEN_READ, SOUP_HTTP_SRC_ERROR (src, msg, RESOURCE, OPEN_READ,
_("Secure connection setup failed.")); _("Secure connection setup failed."));
src->ret = GST_FLOW_ERROR; return GST_FLOW_ERROR;
break;
case SOUP_STATUS_IO_ERROR: case SOUP_STATUS_IO_ERROR:
if (src->max_retries == -1 || src->retry_count < src->max_retries) { if (src->max_retries == -1 || src->retry_count < src->max_retries)
src->ret = GST_FLOW_CUSTOM_ERROR; return GST_FLOW_CUSTOM_ERROR;
} else { SOUP_HTTP_SRC_ERROR (src, msg, RESOURCE, READ,
SOUP_HTTP_SRC_ERROR (src, msg, RESOURCE, READ, _("A network error occurred, or the server closed the connection "
_("A network error occurred, or the server closed the connection " "unexpectedly."));
"unexpectedly.")); return GST_FLOW_ERROR;
src->ret = GST_FLOW_ERROR;
}
break;
case SOUP_STATUS_MALFORMED: case SOUP_STATUS_MALFORMED:
SOUP_HTTP_SRC_ERROR (src, msg, RESOURCE, READ, SOUP_HTTP_SRC_ERROR (src, msg, RESOURCE, READ,
_("Server sent bad data.")); _("Server sent bad data."));
src->ret = GST_FLOW_ERROR; return GST_FLOW_ERROR;
break;
case SOUP_STATUS_CANCELLED: case SOUP_STATUS_CANCELLED:
/* No error message when interrupted by program. */ /* No error message when interrupted by program. */
break; 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_REDIRECTION (msg->status_code) ||
SOUP_STATUS_IS_SERVER_ERROR (msg->status_code)) { SOUP_STATUS_IS_SERVER_ERROR (msg->status_code)) {
const gchar *reason_phrase; const gchar *reason_phrase;
@ -1353,8 +1347,7 @@ gst_soup_http_src_parse_status (SoupMessage * msg, GstSoupHTTPSrc * src)
src->have_body && !src->have_size) { src->have_body && !src->have_size) {
GST_DEBUG_OBJECT (src, "Requested range out of limits and received full " GST_DEBUG_OBJECT (src, "Requested range out of limits and received full "
"body, returning EOS"); "body, returning EOS");
src->ret = GST_FLOW_EOS; return GST_FLOW_EOS;
return;
} }
/* FIXME: reason_phrase is not translated and not suitable for user /* 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, "http-redirect-uri", G_TYPE_STRING,
GST_STR_NULL (src->redirection_uri), NULL)); GST_STR_NULL (src->redirection_uri), NULL));
} }
src->ret = GST_FLOW_ERROR; return GST_FLOW_ERROR;
} }
return GST_FLOW_OK;
} }
static gboolean static gboolean
@ -1434,6 +1429,8 @@ gst_soup_http_src_build_message (GstSoupHTTPSrc * src, const gchar * method)
static GstFlowReturn static GstFlowReturn
gst_soup_http_src_send_message (GstSoupHTTPSrc * src) gst_soup_http_src_send_message (GstSoupHTTPSrc * src)
{ {
GstFlowReturn ret;
g_return_val_if_fail (src->msg != NULL, GST_FLOW_ERROR); g_return_val_if_fail (src->msg != NULL, GST_FLOW_ERROR);
/* FIXME We are ignoring the GError here, might be useful to debug */ /* 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)) if (g_cancellable_is_cancelled (src->cancellable))
return GST_FLOW_FLUSHING; return GST_FLOW_FLUSHING;
src->ret = GST_FLOW_OK; ret = gst_soup_http_src_got_headers (src, src->msg);
if (ret != GST_FLOW_OK) {
gst_soup_http_src_got_headers (src, src->msg); return ret;
if (src->ret != GST_FLOW_OK) {
return src->ret;
} }
if (!src->input_stream) { 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) { if (src->max_retries != -1 && src->retry_count > src->max_retries) {
GST_DEBUG_OBJECT (src, "Max retries reached"); GST_DEBUG_OBJECT (src, "Max retries reached");
src->ret = GST_FLOW_ERROR; return GST_FLOW_ERROR;
return src->ret;
} }
src->retry_count++; 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)) { if (g_cancellable_is_cancelled (src->cancellable)) {
GST_INFO_OBJECT (src, "interrupted"); GST_INFO_OBJECT (src, "interrupted");
src->ret = GST_FLOW_FLUSHING; return GST_FLOW_FLUSHING;
goto done;
} }
src->ret = gst_soup_http_src_send_message (src);
done: return gst_soup_http_src_send_message (src);
return src->ret;
} }
/* /*
@ -1658,7 +1649,7 @@ static GstFlowReturn
gst_soup_http_src_create (GstPushSrc * psrc, GstBuffer ** outbuf) gst_soup_http_src_create (GstPushSrc * psrc, GstBuffer ** outbuf)
{ {
GstSoupHTTPSrc *src; GstSoupHTTPSrc *src;
GstFlowReturn ret = GST_FLOW_OK; GstFlowReturn ret;
GstEvent *http_headers_event = NULL; GstEvent *http_headers_event = NULL;
src = GST_SOUP_HTTP_SRC (psrc); src = GST_SOUP_HTTP_SRC (psrc);
@ -1778,7 +1769,6 @@ gst_soup_http_src_unlock (GstBaseSrc * bsrc)
src = GST_SOUP_HTTP_SRC (bsrc); src = GST_SOUP_HTTP_SRC (bsrc);
GST_DEBUG_OBJECT (src, "unlock()"); GST_DEBUG_OBJECT (src, "unlock()");
src->ret = GST_FLOW_FLUSHING;
gst_soup_http_src_cancel_message (src); gst_soup_http_src_cancel_message (src);
return TRUE; return TRUE;
} }
@ -1792,7 +1782,6 @@ gst_soup_http_src_unlock_stop (GstBaseSrc * bsrc)
src = GST_SOUP_HTTP_SRC (bsrc); src = GST_SOUP_HTTP_SRC (bsrc);
GST_DEBUG_OBJECT (src, "unlock_stop()"); GST_DEBUG_OBJECT (src, "unlock_stop()");
src->ret = GST_FLOW_OK;
g_cancellable_reset (src->cancellable); g_cancellable_reset (src->cancellable);
return TRUE; 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); g_mutex_unlock (&src->mutex);
} }
} }

View file

@ -61,7 +61,6 @@ struct _GstSoupHTTPSrc {
gchar **cookies; /* HTTP request cookies. */ gchar **cookies; /* HTTP request cookies. */
SoupSession *session; /* Async context. */ SoupSession *session; /* Async context. */
SoupMessage *msg; /* Request message. */ SoupMessage *msg; /* Request message. */
GstFlowReturn ret; /* Return code from callback. */
gint retry_count; /* Number of retries since we received data */ gint retry_count; /* Number of retries since we received data */
gint max_retries; /* Maximum number of retries */ gint max_retries; /* Maximum number of retries */
gchar *method; /* HTTP method */ gchar *method; /* HTTP method */