[vlc-devel] [PATCH 1/1] Improve SFTP network performance with buffer

[Martin Disch]

By introducing a small (1 MiB) module-internal circular buffer,
libssh2_sftp_read() can read chunks of a larger size than what is
requested in the Read() function of the module. This allows the SFTP
module to use more of the available bandwidth and fixes stutter when
streaming content over a SFTP connection with some latency.
---
 modules/access/sftp.c | 107 +++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 101 insertions(+), 6 deletions(-)

diff --git a/modules/access/sftp.c b/modules/access/sftp.c
index 1219f573a1..c1b6d14b0a 100644
--- a/modules/access/sftp.c
+++ b/modules/access/sftp.c
@@ -90,8 +90,14 @@ struct access_sys_t
     LIBSSH2_SFTP_HANDLE* file;
     uint64_t filesize;
     char *psz_base_url;
+    void *p_buf;
+    size_t buf_tail;
+    size_t buf_head;
+    size_t buf_content_size;
 };
 
+#define BUF_SIZE 1048576
+
 static int AuthKeyAgent( stream_t *p_access, const char *psz_username )
 {
     access_sys_t* p_sys = p_access->p_sys;
@@ -485,6 +491,12 @@ static int Open( vlc_object_t* p_this )
         goto error;
     }
 
+    /* If all went well, initialize the buffer */
+    p_sys->p_buf = malloc( BUF_SIZE );
+    p_sys->buf_tail = 0;
+    p_sys->buf_head = 0;
+    p_sys->buf_content_size = 0;
+
     i_result = VLC_SUCCESS;
 
 error:
@@ -512,22 +524,99 @@ static void Close( vlc_object_t* p_this )
         libssh2_sftp_shutdown( p_sys->sftp_session );
     SSHSessionDestroy( p_access );
 
+    /* Clean up the buffer */
+    free( p_sys->p_buf );
+    p_sys->p_buf = NULL;
+
     free( p_sys->psz_base_url );
 }
 
 
+/**
+ * Return the number of bytes that can be written to the buffer.
+ *
+ * @param p_sys the access_sys_t object
+ * @return the number of bytes that can be written to the buffer
+ */
+static size_t write_length( access_sys_t *p_sys )
+{
+    if ( p_sys->buf_head < p_sys->buf_tail ) {
+        /* Head before tail, can write from tail to buffer end */
+        return BUF_SIZE - p_sys->buf_tail;
+    } else if ( p_sys->buf_head > p_sys->buf_tail ) {
+        /* Tail is catching up to head, write only until head */
+        return p_sys->buf_head - p_sys->buf_tail;
+    } else if ( p_sys->buf_content_size == 0 ) {
+        /* Head and tail at same position and buffer is empty, fill it up */
+        return BUF_SIZE - p_sys->buf_tail;
+    }
+    /* tail caught up to head (buffer full), don't request any new data */
+    return 0;
+}
+
+/**
+ * Return the number of bytes that can be read from the buffer.
+ *
+ * @param p_sys the access_sys_t object
+ * @return the number of bytes that can be read from the buffer
+ */
+static size_t read_length( access_sys_t *p_sys )
+{
+    if ( p_sys->buf_head < p_sys->buf_tail ) {
+        /* Head before tail, can read in between head and tail */
+        return p_sys->buf_tail - p_sys->buf_head;
+    } else if ( p_sys->buf_head > p_sys->buf_tail ) {
+        /* Tail before head, can read until end of buffer */
+        return BUF_SIZE - p_sys->buf_head;
+    } else if ( p_sys->buf_content_size > 0 ) {
+        /* Tail caught up to head and buffer is full, can read until its end */
+        return BUF_SIZE - p_sys->buf_head;
+    }
+    /* Head and tail at same position and buffer is empty, nothing to read */
+    return 0;
+}
+
 static ssize_t Read( stream_t *p_access, void *buf, size_t len )
 {
     access_sys_t *p_sys = p_access->p_sys;
 
-    ssize_t val = libssh2_sftp_read(  p_sys->file, buf, len );
-    if( val < 0 )
-    {
-        msg_Err( p_access, "read failed" );
-        return 0;
+    /* Get number of bytes we can write to buffer */
+    size_t max_buf = write_length( p_sys );
+    if ( max_buf > 0 ) {
+        /* Attempt reading this amount from SFTP source */
+        ssize_t amount_buffered = libssh2_sftp_read(
+            p_sys->file,
+            (char *) p_sys->p_buf + p_sys->buf_tail,
+            max_buf);
+        if( amount_buffered < 0 )
+        {
+            msg_Err( p_access, "read failed" );
+            return 0;
+        }
+        /* Update buffer variables */
+        p_sys->buf_tail += amount_buffered;
+        p_sys->buf_content_size += amount_buffered;
+        if ( p_sys->buf_tail == BUF_SIZE ) {
+            p_sys->buf_tail = 0;
+        }
+    }
+
+    /* Get number of bytes we can read from buffer */
+    size_t max_read = read_length( p_sys );
+    /* Read as much as is possible but not more than requested */
+    size_t amount_read = __MIN( max_read, len );
+    if ( amount_read > 0 ) {
+        /* Copy the bytes from own buffer to the provided one */
+        memcpy( buf, (char *) p_sys->p_buf + p_sys->buf_head, amount_read );
+        /* Update buffer variables */
+        p_sys->buf_head += amount_read;
+        p_sys->buf_content_size -= amount_read;
+        if ( p_sys->buf_head == BUF_SIZE ) {
+            p_sys->buf_head = 0;
+        }
     }
 
-    return val;
+    return amount_read;
 }
 
 
@@ -536,6 +625,12 @@ static int Seek( stream_t* p_access, uint64_t i_pos )
     access_sys_t *sys = p_access->p_sys;
 
     libssh2_sftp_seek( sys->file, i_pos );
+
+    /* Reset buffer */
+    sys->buf_head = 0;
+    sys->buf_tail = 0;
+    sys->buf_content_size = 0;
+
     return VLC_SUCCESS;
 }
 
-- 
2.14.1