diff mbox

[FFmpeg-devel] libavformat/udp: add options send_pkt_size for udp over ip, for ts over ip, this value must be 1316(7*188)

Message ID 20190408154257.72984-1-edward_email@126.com
State New
Headers show

Commit Message

edward_email April 8, 2019, 3:42 p.m. UTC 
From: “Edward.Wu” <“edward_email@126.com”>

Signed-off-by: “Edward.Wu” <“edward_email@126.com”>
---
 libavformat/udp.c | 45 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 44 insertions(+), 1 deletion(-)

Comments

Liu Steven April 8, 2019, 3:56 p.m. UTC | #1 
> 在 2019年4月8日，23:42，edward_email <edward_email@126.com> 写道：
> 
> From: “Edward.Wu” <“edward_email@126.com”>
> 
> Signed-off-by: “Edward.Wu” <“edward_email@126.com”>
> —
Document please.
> libavformat/udp.c | 45 ++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 44 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/udp.c b/libavformat/udp.c
> index cf73d331e0..7563b671fe 100644
> --- a/libavformat/udp.c
> +++ b/libavformat/udp.c
> @@ -73,6 +73,7 @@
> #define UDP_TX_BUF_SIZE 32768
> #define UDP_MAX_PKT_SIZE 65536
> #define UDP_HEADER_SIZE 8
> +#define UDP_PACKET_MAX 1472
> 
> typedef struct UDPContext {
>     const AVClass *class;
> @@ -81,6 +82,9 @@ typedef struct UDPContext {
>     int udplite_coverage;
>     int buffer_size;
>     int pkt_size;
> +    int send_pkt_size;
> +    uint8_t send_pkt_buf[UDP_PACKET_MAX];
> +    int cur_send_pkt_len;
>     int is_multicast;
>     int is_broadcast;
>     int local_port;
> @@ -125,6 +129,7 @@ static const AVOption options[] = {
>     { "localaddr",      "Local address",                                   OFFSET(localaddr),      AV_OPT_TYPE_STRING, { .str = NULL },               .flags = D|E },
>     { "udplite_coverage", "choose UDPLite head size which should be validated by checksum", OFFSET(udplite_coverage), AV_OPT_TYPE_INT, {.i64 = 0}, 0, INT_MAX, D|E },
>     { "pkt_size",       "Maximum UDP packet size",                         OFFSET(pkt_size),       AV_OPT_TYPE_INT,    { .i64 = 1472 },  -1, INT_MAX, .flags = D|E },
> +    { "send_pkt_size",   "Send UDP packet size, ts over ip must be 1316",  OFFSET(send_pkt_size),  AV_OPT_TYPE_INT,    { .i64 = 0 },  -1, 1472, .flags = D|E },
>     { "reuse",          "explicitly allow reusing UDP sockets",            OFFSET(reuse_socket),   AV_OPT_TYPE_BOOL,   { .i64 = -1 },    -1, 1,       D|E },
>     { "reuse_socket",   "explicitly allow reusing UDP sockets",            OFFSET(reuse_socket),   AV_OPT_TYPE_BOOL,   { .i64 = -1 },    -1, 1,       .flags = D|E },
>     { "broadcast", "explicitly allow or disallow broadcast destination",   OFFSET(is_broadcast),   AV_OPT_TYPE_BOOL,   { .i64 = 0  },     0, 1,       E },
> @@ -390,6 +395,7 @@ static int udp_port(struct sockaddr_storage *addr, int addr_len)
>  * option: 'ttl=n'       : set the ttl value (for multicast only)
>  *         'localport=n' : set the local port
>  *         'pkt_size=n'  : set max packet size
> + *         'send_pkt_size=n' : set send packet size
>  *         'reuse=1'     : enable reusing the socket
>  *         'overrun_nonfatal=1': survive in case of circular buffer overrun
>  *
> @@ -683,6 +689,13 @@ static int udp_open(URLContext *h, const char *uri, int flags)
>         if (av_find_info_tag(buf, sizeof(buf), "pkt_size", p)) {
>             s->pkt_size = strtol(buf, NULL, 10);
>         }
> +        if (av_find_info_tag(buf, sizeof(buf), "send_pkt_size", p)) {
> +            s->send_pkt_size = strtol(buf, NULL, 10);
> +            if (s->send_pkt_size > UDP_PACKET_MAX) {//UDP max
> +                av_log(h, AV_LOG_WARNING, "send_pkt_size is %s, bigger than %d, set as %d.\n", buf, UDP_PACKET_MAX, UDP_PACKET_MAX);
> +                s->send_pkt_size = UDP_PACKET_MAX;
> +            }
> +        }
>         if (av_find_info_tag(buf, sizeof(buf), "buffer_size", p)) {
>             s->buffer_size = strtol(buf, NULL, 10);
>         }
> @@ -1001,7 +1014,8 @@ static int udp_read(URLContext *h, uint8_t *buf, int size)
>     return ret;
> }
> 
> -static int udp_write(URLContext *h, const uint8_t *buf, int size)
> +
> +static int udp_write_internal(URLContext *h, const uint8_t *buf, int size)
> {
>     UDPContext *s = h->priv_data;
>     int ret;
> @@ -1050,6 +1064,35 @@ static int udp_write(URLContext *h, const uint8_t *buf, int size)
> 
>     return ret < 0 ? ff_neterrno() : ret;
> }
> +static int udp_write(URLContext *h, const uint8_t *buf, int size)
> +{
> +    UDPContext *s = h->priv_data;
> +    int copied = 0;
> +    int capacity = 0;
> +    uint8_t * p = NULL;
uint8_t *p = NULL;
> +    uint8_t * p_end = NULL;
uint8_t *p_end = NULL;
> +    int re = 0;
What about change re to ret?
> +
> +    if (s->send_pkt_size == 0) {
> +        return udp_write_internal(h, buf, size);
> +    } else {
> +        p = (uint8_t*)buf;
> +        p_end = p + size;
> +        while (p < p_end) {
> +            if ( s->cur_send_pkt_len >= s->send_pkt_size ){
> +                re +=  udp_write_internal(h, s->send_pkt_buf, s->cur_send_pkt_len);
Maybe you want check the return value of the udp_write_internal?
What do you want to use when the re + the value of udp_write_internal result?
> +                s->cur_send_pkt_len = 0;
> +            }
> +
> +            capacity = s->send_pkt_size - s->cur_send_pkt_len;
> +            copied = capacity  > (p_end - p) ? (p_end - p) : capacity;
> +            memcpy(s->send_pkt_buf + s->cur_send_pkt_len, p, copied);
> +            p += copied;
> +            s->cur_send_pkt_len += copied;
> +        }
> +    }
> +    return size;
> +}
> 
> static int udp_close(URLContext *h)
> {
> -- 
> 2.11.0
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Thanks
Steven
Thomas Volkert April 8, 2019, 4:10 p.m. UTC | #2 
Hi,


On 08.04.19 17:42, edward_email wrote:
> From: “Edward.Wu” <“edward_email@126.com”>
>
> Signed-off-by: “Edward.Wu” <“edward_email@126.com”>
> ---
>   libavformat/udp.c | 45 ++++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 44 insertions(+), 1 deletion(-)
>
> diff --git a/libavformat/udp.c b/libavformat/udp.c
> index cf73d331e0..7563b671fe 100644
> --- a/libavformat/udp.c
> +++ b/libavformat/udp.c
> @@ -73,6 +73,7 @@
>   #define UDP_TX_BUF_SIZE 32768
>   #define UDP_MAX_PKT_SIZE 65536
>   #define UDP_HEADER_SIZE 8
> +#define UDP_PACKET_MAX 1472

Could you explain how you calculate this value and why it generally
valid in this case?


>
>   typedef struct UDPContext {
>       const AVClass *class;
> @@ -81,6 +82,9 @@ typedef struct UDPContext {
>       int udplite_coverage;
>       int buffer_size;
>       int pkt_size;
> +    int send_pkt_size;
> +    uint8_t send_pkt_buf[UDP_PACKET_MAX];
> +    int cur_send_pkt_len;
>       int is_multicast;
>       int is_broadcast;
>       int local_port;
> @@ -125,6 +129,7 @@ static const AVOption options[] = {
>       { "localaddr",      "Local address",                                   OFFSET(localaddr),      AV_OPT_TYPE_STRING, { .str = NULL },               .flags = D|E },
>       { "udplite_coverage", "choose UDPLite head size which should be validated by checksum", OFFSET(udplite_coverage), AV_OPT_TYPE_INT, {.i64 = 0}, 0, INT_MAX, D|E },
>       { "pkt_size",       "Maximum UDP packet size",                         OFFSET(pkt_size),       AV_OPT_TYPE_INT,    { .i64 = 1472 },  -1, INT_MAX, .flags = D|E },
> +    { "send_pkt_size",   "Send UDP packet size, ts over ip must be 1316",  OFFSET(send_pkt_size),  AV_OPT_TYPE_INT,    { .i64 = 0 },  -1, 1472, .flags = D|E },

I assume ts means "MPEG-TS": why does it need to be 7 times an MPEG-TS
packet? Why not 6 times or less? What about jumbo packets which exceeds
the size of an 1500 MTU and are definitely possible under defined
circumstances?


Best regards,
Thomas.
Marton Balint April 8, 2019, 4:29 p.m. UTC | #3 
On Mon, 8 Apr 2019, edward_email wrote:

> From: “Edward.Wu” <“edward_email@126.com”>
>
> Signed-off-by: “Edward.Wu” <“edward_email@126.com”>
> ---
> libavformat/udp.c | 45 ++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 44 insertions(+), 1 deletion(-)

Why is this patch needed at all? If you want fixed packet size, you simply 
use -pkt_size 1316 -flush_packets 0.

Regards,
Marton
edward_email April 8, 2019, 10:33 p.m. UTC | #4 
At 2019-04-09 00:29:43, "Marton Balint" <cus@passwd.hu> wrote:
>
>
>On Mon, 8 Apr 2019, edward_email wrote:
>
>> From: “Edward.Wu” <“edward_email@126.com”>
>>
>> Signed-off-by: “Edward.Wu” <“edward_email@126.com”>
>> ---
>> libavformat/udp.c | 45 ++++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 44 insertions(+), 1 deletion(-)
>
>Why is this patch needed at all? If you want fixed packet size, you simply 

>use -pkt_size 1316 -flush_packets 0.
thanks for your reminding，I try these two parameters, it's OK.
>
>Regards,
>Marton
>_______________________________________________
>ffmpeg-devel mailing list
>ffmpeg-devel@ffmpeg.org
>https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>To unsubscribe, visit link above, or email
>ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff mbox

Patch

diff --git a/libavformat/udp.c b/libavformat/udp.c
index cf73d331e0..7563b671fe 100644
--- a/libavformat/udp.c
+++ b/libavformat/udp.c
@@ -73,6 +73,7 @@ 
 #define UDP_TX_BUF_SIZE 32768
 #define UDP_MAX_PKT_SIZE 65536
 #define UDP_HEADER_SIZE 8
+#define UDP_PACKET_MAX 1472
 
 typedef struct UDPContext {
     const AVClass *class;
@@ -81,6 +82,9 @@  typedef struct UDPContext {
     int udplite_coverage;
     int buffer_size;
     int pkt_size;
+    int send_pkt_size;
+    uint8_t send_pkt_buf[UDP_PACKET_MAX];
+    int cur_send_pkt_len;
     int is_multicast;
     int is_broadcast;
     int local_port;
@@ -125,6 +129,7 @@  static const AVOption options[] = {
     { "localaddr",      "Local address",                                   OFFSET(localaddr),      AV_OPT_TYPE_STRING, { .str = NULL },               .flags = D|E },
     { "udplite_coverage", "choose UDPLite head size which should be validated by checksum", OFFSET(udplite_coverage), AV_OPT_TYPE_INT, {.i64 = 0}, 0, INT_MAX, D|E },
     { "pkt_size",       "Maximum UDP packet size",                         OFFSET(pkt_size),       AV_OPT_TYPE_INT,    { .i64 = 1472 },  -1, INT_MAX, .flags = D|E },
+    { "send_pkt_size",   "Send UDP packet size, ts over ip must be 1316",  OFFSET(send_pkt_size),  AV_OPT_TYPE_INT,    { .i64 = 0 },  -1, 1472, .flags = D|E },
     { "reuse",          "explicitly allow reusing UDP sockets",            OFFSET(reuse_socket),   AV_OPT_TYPE_BOOL,   { .i64 = -1 },    -1, 1,       D|E },
     { "reuse_socket",   "explicitly allow reusing UDP sockets",            OFFSET(reuse_socket),   AV_OPT_TYPE_BOOL,   { .i64 = -1 },    -1, 1,       .flags = D|E },
     { "broadcast", "explicitly allow or disallow broadcast destination",   OFFSET(is_broadcast),   AV_OPT_TYPE_BOOL,   { .i64 = 0  },     0, 1,       E },
@@ -390,6 +395,7 @@  static int udp_port(struct sockaddr_storage *addr, int addr_len)
  * option: 'ttl=n'       : set the ttl value (for multicast only)
  *         'localport=n' : set the local port
  *         'pkt_size=n'  : set max packet size
+ *         'send_pkt_size=n' : set send packet size
  *         'reuse=1'     : enable reusing the socket
  *         'overrun_nonfatal=1': survive in case of circular buffer overrun
  *
@@ -683,6 +689,13 @@  static int udp_open(URLContext *h, const char *uri, int flags)
         if (av_find_info_tag(buf, sizeof(buf), "pkt_size", p)) {
             s->pkt_size = strtol(buf, NULL, 10);
         }
+        if (av_find_info_tag(buf, sizeof(buf), "send_pkt_size", p)) {
+            s->send_pkt_size = strtol(buf, NULL, 10);
+            if (s->send_pkt_size > UDP_PACKET_MAX) {//UDP max
+                av_log(h, AV_LOG_WARNING, "send_pkt_size is %s, bigger than %d, set as %d.\n", buf, UDP_PACKET_MAX, UDP_PACKET_MAX);
+                s->send_pkt_size = UDP_PACKET_MAX;
+            }
+        }
         if (av_find_info_tag(buf, sizeof(buf), "buffer_size", p)) {
             s->buffer_size = strtol(buf, NULL, 10);
         }
@@ -1001,7 +1014,8 @@  static int udp_read(URLContext *h, uint8_t *buf, int size)
     return ret;
 }
 
-static int udp_write(URLContext *h, const uint8_t *buf, int size)
+
+static int udp_write_internal(URLContext *h, const uint8_t *buf, int size)
 {
     UDPContext *s = h->priv_data;
     int ret;
@@ -1050,6 +1064,35 @@  static int udp_write(URLContext *h, const uint8_t *buf, int size)
 
     return ret < 0 ? ff_neterrno() : ret;
 }
+static int udp_write(URLContext *h, const uint8_t *buf, int size)
+{
+    UDPContext *s = h->priv_data;
+    int copied = 0;
+    int capacity = 0;
+    uint8_t * p = NULL;
+    uint8_t * p_end = NULL;
+    int re = 0;
+
+    if (s->send_pkt_size == 0) {
+        return udp_write_internal(h, buf, size);
+    } else {
+        p = (uint8_t*)buf;
+        p_end = p + size;
+        while (p < p_end) {
+            if ( s->cur_send_pkt_len >= s->send_pkt_size ){
+                re +=  udp_write_internal(h, s->send_pkt_buf, s->cur_send_pkt_len);
+                s->cur_send_pkt_len = 0;
+            }
+
+            capacity = s->send_pkt_size - s->cur_send_pkt_len;
+            copied = capacity  > (p_end - p) ? (p_end - p) : capacity;
+            memcpy(s->send_pkt_buf + s->cur_send_pkt_len, p, copied);
+            p += copied;
+            s->cur_send_pkt_len += copied;
+        }
+    }
+    return size;
+}
 
 static int udp_close(URLContext *h)
 {