[2/2] tls_gnutls: Readd support for nonblocking operation
diff mbox

Message ID 20170619125246.65326-2-martin@martin.st
State Committed
Commit eb061ad6fd0e3cea7cf7cfbff0749bc90dd7d888
Headers show

Commit Message

Martin Storsjö June 19, 2017, 12:52 p.m. 
The rtmp protocol uses nonblocking reads, to poll for incoming
messages from the server while publishing a stream. Prior to
94599a6de3822b13c94096d764868128f388ba28 and
d13b124eaf452b267480074b2e6946538ed03a6e, the tls protocol
handled the nonblocking flag.

This fixes publishing over rtmps, with the gnutls backend.
---
 libavformat/tls_gnutls.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

Comments

Diego Biurrun June 20, 2017, 9:35 a.m. | #1 
On Mon, Jun 19, 2017 at 03:52:46PM +0300, Martin Storsjö wrote:
> The rtmp protocol uses nonblocking reads, to poll for incoming
> messages from the server while publishing a stream. Prior to
> 94599a6de3822b13c94096d764868128f388ba28 and
> d13b124eaf452b267480074b2e6946538ed03a6e, the tls protocol
> handled the nonblocking flag.

Those commit logs claim that the tls protocol does not have to handle
the nonblocking flag, apparently it has to now. An explanation would
be illuminating.

probably OK

> --- a/libavformat/tls_gnutls.c
> +++ b/libavformat/tls_gnutls.c
> @@ -97,7 +98,10 @@ static ssize_t gnutls_url_pull(gnutls_transport_ptr_t transport,
>          return ret;
>      if (ret == AVERROR_EXIT)
>          return 0;
> -    errno = EIO;
> +    if (ret == AVERROR(EAGAIN))
> +        errno = EAGAIN;
> +    else
> +        errno = EIO;
>      return -1;
>  }
>  
> @@ -110,7 +114,10 @@ static ssize_t gnutls_url_push(gnutls_transport_ptr_t transport,
>          return ret;
>      if (ret == AVERROR_EXIT)
>          return 0;
> -    errno = EIO;
> +    if (ret == AVERROR(EAGAIN))
> +        errno = EAGAIN;
> +    else
> +        errno = EIO;
>      return -1;
>  }

Convert to switch/case at some point?

Diego
Martin Storsjö June 20, 2017, 10:10 a.m. | #2 
On Tue, 20 Jun 2017, Diego Biurrun wrote:

> On Mon, Jun 19, 2017 at 03:52:46PM +0300, Martin Storsjö wrote:
>> The rtmp protocol uses nonblocking reads, to poll for incoming
>> messages from the server while publishing a stream. Prior to
>> 94599a6de3822b13c94096d764868128f388ba28 and
>> d13b124eaf452b267480074b2e6946538ed03a6e, the tls protocol
>> handled the nonblocking flag.
>
> Those commit logs claim that the tls protocol does not have to handle
> the nonblocking flag, apparently it has to now. An explanation would
> be illuminating.

It's more or less a revert of d13b124eaf452b267480074b2e6946538ed03a6e. 
After chaining the IO calls to the underlying urlprotocol, we didn't need 
to handle nonblocking mode here when the tls protocol was used in 
blocking mode.

I had overlooked the fact that callers actually wanted to use it in 
nonblocking mode as well (as the rtmps protocol does), and at that point 
when I removed it, the fact that it supported nonblocking mode felt more 
like a sideeffect, not a necessary feature in itself. But it is indeed 
necessary for rtmps.

// Martin

Patch
diff mbox 

diff --git a/libavformat/tls_gnutls.c b/libavformat/tls_gnutls.c
index 1ae7656dcb..7bfe02deb0 100644
--- a/libavformat/tls_gnutls.c
+++ b/libavformat/tls_gnutls.c
@@ -61,6 +61,7 @@  static int print_tls_error(URLContext *h, int ret)
 {
     switch (ret) {
     case GNUTLS_E_AGAIN:
+        return AVERROR(EAGAIN);
     case GNUTLS_E_INTERRUPTED:
         break;
     case GNUTLS_E_WARNING_ALERT_RECEIVED:
@@ -97,7 +98,10 @@  static ssize_t gnutls_url_pull(gnutls_transport_ptr_t transport,
         return ret;
     if (ret == AVERROR_EXIT)
         return 0;
-    errno = EIO;
+    if (ret == AVERROR(EAGAIN))
+        errno = EAGAIN;
+    else
+        errno = EIO;
     return -1;
 }
 
@@ -110,7 +114,10 @@  static ssize_t gnutls_url_push(gnutls_transport_ptr_t transport,
         return ret;
     if (ret == AVERROR_EXIT)
         return 0;
-    errno = EIO;
+    if (ret == AVERROR(EAGAIN))
+        errno = EAGAIN;
+    else
+        errno = EIO;
     return -1;
 }
 
@@ -202,7 +209,11 @@  fail:
 static int tls_read(URLContext *h, uint8_t *buf, int size)
 {
     TLSContext *c = h->priv_data;
-    int ret = gnutls_record_recv(c->session, buf, size);
+    int ret;
+    // Set or clear the AVIO_FLAG_NONBLOCK on c->tls_shared.tcp
+    c->tls_shared.tcp->flags &= ~AVIO_FLAG_NONBLOCK;
+    c->tls_shared.tcp->flags |= h->flags & AVIO_FLAG_NONBLOCK;
+    ret = gnutls_record_recv(c->session, buf, size);
     if (ret > 0)
         return ret;
     if (ret == 0)
@@ -213,7 +224,11 @@  static int tls_read(URLContext *h, uint8_t *buf, int size)
 static int tls_write(URLContext *h, const uint8_t *buf, int size)
 {
     TLSContext *c = h->priv_data;
-    int ret = gnutls_record_send(c->session, buf, size);
+    int ret;
+    // Set or clear the AVIO_FLAG_NONBLOCK on c->tls_shared.tcp
+    c->tls_shared.tcp->flags &= ~AVIO_FLAG_NONBLOCK;
+    c->tls_shared.tcp->flags |= h->flags & AVIO_FLAG_NONBLOCK;
+    ret = gnutls_record_send(c->session, buf, size);
     if (ret > 0)
         return ret;
     if (ret == 0)