[PATCHv3] avio: Do not flush the buffer if a constant packet size is requested

Message ID 20190222121848.89486-1-lu_zero@gentoo.org
State New
Headers show
Series
  • [PATCHv3] avio: Do not flush the buffer if a constant packet size is requested
Related show

Commit Message

Luca Barbato Feb. 22, 2019, 12:18 p.m.
---

Now with a separate option to be explicit on what is the behaviour
wanted.

 libavformat/aviobuf.c | 9 +++++++--
 libavformat/udp.c     | 8 ++++++++
 libavformat/url.h     | 1 +
 3 files changed, 16 insertions(+), 2 deletions(-)

--
2.12.2

Comments

Martin Storsjö Feb. 22, 2019, 12:48 p.m. | #1
On Fri, 22 Feb 2019, Luca Barbato wrote:

> ---
>
> Now with a separate option to be explicit on what is the behaviour
> wanted.
>
> libavformat/aviobuf.c | 9 +++++++--
> libavformat/udp.c     | 8 ++++++++
> libavformat/url.h     | 1 +
> 3 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
> index 98e35f776c..aa9e2fc483 100644
> --- a/libavformat/aviobuf.c
> +++ b/libavformat/aviobuf.c
> @@ -244,8 +244,13 @@ void avio_write(AVIOContext *s, const unsigned char *buf, int size)
>
> void avio_flush(AVIOContext *s)
> {
> -    flush_buffer(s);
> -    s->must_flush = 0;
> +    AVIOInternal *internal = s->opaque;
> +    URLContext *h = internal->h;
> +

No, this doesn't work. You can't assume that s->opaque exists and is an 
AVIOinternal struct. When AVIOContext has been allocated by 
avio_alloc_context, s->opaque is whatever custom pointer the caller 
provided.

The only place you can use AVIOInternal is within the callbacks you 
provide in ffio_fdopen when AVIOInternal is created.

To do this properly, you need to propagate the new value all the way into 
AVIOContext, just like the existing max_packet_size.

// Martin
Luca Barbato Feb. 24, 2019, 12:07 a.m. | #2
On 22/02/2019 13:48, Martin Storsjö wrote:
> On Fri, 22 Feb 2019, Luca Barbato wrote:
> 
>> ---
>>
>> Now with a separate option to be explicit on what is the behaviour
>> wanted.
>>
>> libavformat/aviobuf.c | 9 +++++++--
>> libavformat/udp.c     | 8 ++++++++
>> libavformat/url.h     | 1 +
>> 3 files changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
>> index 98e35f776c..aa9e2fc483 100644
>> --- a/libavformat/aviobuf.c
>> +++ b/libavformat/aviobuf.c
>> @@ -244,8 +244,13 @@ void avio_write(AVIOContext *s, const unsigned 
>> char *buf, int size)
>>
>> void avio_flush(AVIOContext *s)
>> {
>> -    flush_buffer(s);
>> -    s->must_flush = 0;
>> +    AVIOInternal *internal = s->opaque;
>> +    URLContext *h = internal->h;
>> +
> 
> No, this doesn't work. You can't assume that s->opaque exists and is an 
> AVIOinternal struct. When AVIOContext has been allocated by 
> avio_alloc_context, s->opaque is whatever custom pointer the caller 
> provided.
> 
> The only place you can use AVIOInternal is within the callbacks you 
> provide in ffio_fdopen when AVIOInternal is created.
> 
> To do this properly, you need to propagate the new value all the way 
> into AVIOContext, just like the existing max_packet_size.

You are right, thank you for spotting this.

lu

Patch

diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
index 98e35f776c..aa9e2fc483 100644
--- a/libavformat/aviobuf.c
+++ b/libavformat/aviobuf.c
@@ -244,8 +244,13 @@  void avio_write(AVIOContext *s, const unsigned char *buf, int size)

 void avio_flush(AVIOContext *s)
 {
-    flush_buffer(s);
-    s->must_flush = 0;
+    AVIOInternal *internal = s->opaque;
+    URLContext *h = internal->h;
+
+    if (!h->min_packet_size || s->buf_ptr - s->buffer >= h->min_packet_size) {
+        flush_buffer(s);
+        s->must_flush = 0;
+    }
 }

 int64_t avio_seek(AVIOContext *s, int64_t offset, int whence)
diff --git a/libavformat/udp.c b/libavformat/udp.c
index a29eb1bd33..ede74d111d 100644
--- a/libavformat/udp.c
+++ b/libavformat/udp.c
@@ -48,6 +48,7 @@  typedef struct UDPContext {
     int ttl;
     int buffer_size;
     int pkt_size;
+    int min_pkt_size;
     int is_multicast;
     int local_port;
     int reuse_socket;
@@ -72,6 +73,7 @@  static const AVOption options[] = {
     { "reuse_socket",   "Reuse socket",                                    OFFSET(reuse_socket),   AV_OPT_TYPE_INT,    { .i64 = -1 },    -1, 1,       .flags = D|E },
     { "connect",        "Connect socket",                                  OFFSET(is_connected),   AV_OPT_TYPE_INT,    { .i64 =  0 },     0, 1,       .flags = D|E },
     { "pkt_size",       "Maximum packet size",                             OFFSET(pkt_size),       AV_OPT_TYPE_INT,    { .i64 = -1 },    -1, INT_MAX, .flags = D|E },
+    { "min_pkt_size",   "Minimum packet size",                             OFFSET(min_pkt_size),   AV_OPT_TYPE_INT,    { .i64 = -1 },    -1, INT_MAX, .flags = D|E },
     { "localaddr",      "Local address",                                   OFFSET(localaddr),      AV_OPT_TYPE_STRING, { .str = NULL },               .flags = D|E },
     { "sources",        "Source list",                                     OFFSET(sources),        AV_OPT_TYPE_STRING, { .str = NULL },               .flags = D|E },
     { "block",          "Block list",                                      OFFSET(block),          AV_OPT_TYPE_STRING, { .str = NULL },               .flags = D|E },
@@ -476,6 +478,9 @@  static int udp_open(URLContext *h, const char *uri, int flags)
     if (s->pkt_size > 0)
         h->max_packet_size = s->pkt_size;

+    if (s->min_pkt_size > 0)
+        h->min_packet_size = s->min_pkt_size;
+
     p = strchr(uri, '?');
     if (p) {
         if (av_find_info_tag(buf, sizeof(buf), "reuse", p)) {
@@ -494,6 +499,9 @@  static int udp_open(URLContext *h, const char *uri, int flags)
         if (av_find_info_tag(buf, sizeof(buf), "pkt_size", p)) {
             h->max_packet_size = strtol(buf, NULL, 10);
         }
+        if (av_find_info_tag(buf, sizeof(buf), "min_pkt_size", p)) {
+            h->min_packet_size = strtol(buf, NULL, 10);
+        }
         if (av_find_info_tag(buf, sizeof(buf), "buffer_size", p)) {
             s->buffer_size = strtol(buf, NULL, 10);
         }
diff --git a/libavformat/url.h b/libavformat/url.h
index 5853ffee7a..6638a62a22 100644
--- a/libavformat/url.h
+++ b/libavformat/url.h
@@ -46,6 +46,7 @@  typedef struct URLContext {
     char *filename;             /**< specified URL */
     int flags;
     int max_packet_size;        /**< if non zero, the stream is packetized with this max packet size */
+    int min_packet_size;        /**< if non zero, the stream is flushed once at least that number of bytes is enqueued */
     int is_streamed;            /**< true if streamed (no seek possible), default = false */
     int is_connected;
     AVIOInterruptCB interrupt_callback;