diff mbox

[FFmpeg-devel] avformat/tcp: enable TCP_NODELAY by default

Message ID 20171113202234.94828-1-ffmpeg@tmm1.net
State Superseded
Headers show

Commit Message

Aman Karmani Nov. 13, 2017, 8:22 p.m. UTC
From: Aman Gupta <aman@tmm1.net>

---
 libavformat/network.h | 1 +
 libavformat/tcp.c     | 5 +++++
 2 files changed, 6 insertions(+)

Comments

Nicolas George Nov. 13, 2017, 8:37 p.m. UTC | #1
Le tridi 23 brumaire, an CCXXVI, Aman Gupta a écrit :
> From: Aman Gupta <aman@tmm1.net>
> 
> ---
>  libavformat/network.h | 1 +
>  libavformat/tcp.c     | 5 +++++
>  2 files changed, 6 insertions(+)

Why?

Regards,
Aman Gupta Nov. 13, 2017, 8:43 p.m. UTC | #2
On Mon, Nov 13, 2017 at 12:37 PM, Nicolas George <george@nsup.org> wrote:

> Le tridi 23 brumaire, an CCXXVI, Aman Gupta a écrit :
> > From: Aman Gupta <aman@tmm1.net>
> >
> > ---
> >  libavformat/network.h | 1 +
> >  libavformat/tcp.c     | 5 +++++
> >  2 files changed, 6 insertions(+)
>
> Why?
>

To potentially reduce latency, particularly when using a protocol that
builds on top of tcp like http.

I can disable this option by default if that makes the patch more
palatable. That would preserve the existing behavior, and let users enable
the option if they decide Nagle's algorithm is not a good fit for their use
case.

Aman


>
> Regards,
>
> --
>   Nicolas George
>
Jeyapal, Karthick Nov. 14, 2017, 8:11 a.m. UTC | #3
>On 11/14/17, 2:47 AM, "Aman Gupta" <aman@tmm1.net> wrote:

>

>>On Mon, Nov 13, 2017 at 12:37 PM, Nicolas George <george@nsup.org> wrote:

>>

>> Why?

>>

>

>To potentially reduce latency, particularly when using a protocol that

>builds on top of tcp like http.

Yes. Apart from the latency, overall throughput could also
increase in a high latency network with TCP_NODELAY
>

>I can disable this option by default if that makes the patch more

>palatable. That would preserve the existing behavior, and let users enable

>the option if they decide Nagle's algorithm is not a good fit for their use

>case.

I think, it is safer to set default as disable. We should modify its callers like 
http or hls etc., to enable this option as per the use-case. 
>

>Aman

>

>

>>

>> Regards,

>>

>> --

>>   Nicolas George

>>
diff mbox

Patch

diff --git a/libavformat/network.h b/libavformat/network.h
index f83c796a95..b78e3ad6ed 100644
--- a/libavformat/network.h
+++ b/libavformat/network.h
@@ -59,6 +59,7 @@  int ff_neterrno(void);
 #include <sys/types.h>
 #include <sys/socket.h>
 #include <netinet/in.h>
+#include <netinet/tcp.h>
 #include <netdb.h>
 
 #define ff_neterrno() AVERROR(errno)
diff --git a/libavformat/tcp.c b/libavformat/tcp.c
index 07b4ed9fa3..419eed1d02 100644
--- a/libavformat/tcp.c
+++ b/libavformat/tcp.c
@@ -41,6 +41,7 @@  typedef struct TCPContext {
     int listen_timeout;
     int recv_buffer_size;
     int send_buffer_size;
+    int tcp_nodelay;
 } TCPContext;
 
 #define OFFSET(x) offsetof(TCPContext, x)
@@ -52,6 +53,7 @@  static const AVOption options[] = {
     { "listen_timeout",  "Connection awaiting timeout (in milliseconds)",      OFFSET(listen_timeout), AV_OPT_TYPE_INT, { .i64 = -1 },         -1, INT_MAX, .flags = D|E },
     { "send_buffer_size", "Socket send buffer size (in bytes)",                OFFSET(send_buffer_size), AV_OPT_TYPE_INT, { .i64 = -1 },         -1, INT_MAX, .flags = D|E },
     { "recv_buffer_size", "Socket receive buffer size (in bytes)",             OFFSET(recv_buffer_size), AV_OPT_TYPE_INT, { .i64 = -1 },         -1, INT_MAX, .flags = D|E },
+    { "tcp_nodelay", "Use TCP_NODELAY to disable nagle's algorithm",           OFFSET(tcp_nodelay), AV_OPT_TYPE_BOOL, { .i64 = 1 },             0, 1, .flags = D|E },
     { NULL }
 };
 
@@ -148,6 +150,9 @@  static int tcp_open(URLContext *h, const char *uri, int flags)
     if (s->send_buffer_size > 0) {
         setsockopt (fd, SOL_SOCKET, SO_SNDBUF, &s->send_buffer_size, sizeof (s->send_buffer_size));
     }
+    if (s->tcp_nodelay > 0) {
+        setsockopt (fd, IPPROTO_TCP, TCP_NODELAY, &s->tcp_nodelay, sizeof (s->tcp_nodelay));
+    }
 
     if (s->listen == 2) {
         // multi-client