diff mbox

[FFmpeg-devel] cache: read ahead to avoid excessive small requests

Message ID CALdTkGESWV=5RKvY0z11zmT02r2NDw9Cdnf--aLsxX03CeaodQ@mail.gmail.com
State New
Headers show

Commit Message

Robert Nagy Sept. 22, 2018, 5:32 p.m. UTC

Comments

Michael Niedermayer Sept. 23, 2018, 11:40 a.m. UTC | #1 
On Sat, Sep 22, 2018 at 07:32:28PM +0200, Robert Nagy wrote:
> diff --git a/libavformat/cache.c b/libavformat/cache.c
> index 66bbbf54c9..48ff5ab363 100644
> --- a/libavformat/cache.c
> +++ b/libavformat/cache.c
> @@ -153,6 +153,38 @@ fail:
>      return ret;
>  }
> 
> +static int cache_read_ahead(URLContext *h)
> +{
> +    Context *c= h->priv_data;
> +    int64_t r, read_ahead, pos;
> +    uint8_t buf[32768];
> +
> +    pos = c->logical_pos;
> +    read_ahead = c->read_ahead_limit < 0
> +        ? 512 * 512
> +        : FFMIN(512 * 512, c->read_ahead_limit);
> +
> +    while (read_ahead > 0) {
> +        r = ffurl_read(c->inner, buf, FFMIN(read_ahead, sizeof(buf)));
> +        if (r == AVERROR_EOF) {
> +            c->is_true_eof = 1;
> +            av_assert0(c->end >= c->logical_pos);
> +        }
> +        if (r<=0)
> +            break;
> +        c->inner_pos += r;
> +
> +        add_entry(h, buf, r);
> +        c->logical_pos += r;
> +        c->end = FFMAX(c->end, c->logical_pos);
> +        read_ahead -= r;
> +    }
> +
> +    c->logical_pos = pos;
> +
> +    return r < 0 ? r : 0;
> +}
> +
>  static int cache_read(URLContext *h, unsigned char *buf, int size)
>  {
>      Context *c= h->priv_data;

> @@ -215,6 +247,10 @@ static int cache_read(URLContext *h, unsigned
> char *buf, int size)

still not cleanly applying (due to new lines)

Applying: cache: read ahead to avoid excessive small requests
error: corrupt patch at line 45
error: could not build fake ancestor
Patch failed at 0001 cache: read ahead to avoid excessive small requests
hint: Use 'git am --show-current-patch' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


[...]
Robert Nagy Sept. 23, 2018, 6:52 p.m. UTC | #2 
On Sun, Sep 23, 2018 at 1:40 PM Michael Niedermayer
<michael@niedermayer.cc> wrote:
>
> On Sat, Sep 22, 2018 at 07:32:28PM +0200, Robert Nagy wrote:
> > diff --git a/libavformat/cache.c b/libavformat/cache.c
> > index 66bbbf54c9..48ff5ab363 100644
> > --- a/libavformat/cache.c
> > +++ b/libavformat/cache.c
> > @@ -153,6 +153,38 @@ fail:
> >      return ret;
> >  }
> >
> > +static int cache_read_ahead(URLContext *h)
> > +{
> > +    Context *c= h->priv_data;
> > +    int64_t r, read_ahead, pos;
> > +    uint8_t buf[32768];
> > +
> > +    pos = c->logical_pos;
> > +    read_ahead = c->read_ahead_limit < 0
> > +        ? 512 * 512
> > +        : FFMIN(512 * 512, c->read_ahead_limit);
> > +
> > +    while (read_ahead > 0) {
> > +        r = ffurl_read(c->inner, buf, FFMIN(read_ahead, sizeof(buf)));
> > +        if (r == AVERROR_EOF) {
> > +            c->is_true_eof = 1;
> > +            av_assert0(c->end >= c->logical_pos);
> > +        }
> > +        if (r<=0)
> > +            break;
> > +        c->inner_pos += r;
> > +
> > +        add_entry(h, buf, r);
> > +        c->logical_pos += r;
> > +        c->end = FFMAX(c->end, c->logical_pos);
> > +        read_ahead -= r;
> > +    }
> > +
> > +    c->logical_pos = pos;
> > +
> > +    return r < 0 ? r : 0;
> > +}
> > +
> >  static int cache_read(URLContext *h, unsigned char *buf, int size)
> >  {
> >      Context *c= h->priv_data;
>
> > @@ -215,6 +247,10 @@ static int cache_read(URLContext *h, unsigned
> > char *buf, int size)
>
> still not cleanly applying (due to new lines)
>
> Applying: cache: read ahead to avoid excessive small requests
> error: corrupt patch at line 45
> error: could not build fake ancestor
> Patch failed at 0001 cache: read ahead to avoid excessive small requests
> hint: Use 'git am --show-current-patch' to see the failed patch
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
>
>
> [...]
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> The greatest way to live with honor in this world is to be what we pretend
> to be. -- Socrates
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Michael Niedermayer Sept. 24, 2018, 8:01 p.m. UTC | #3 
On Sun, Sep 23, 2018 at 08:52:06PM +0200, Robert Nagy wrote:
> On Sun, Sep 23, 2018 at 1:40 PM Michael Niedermayer
> <michael@niedermayer.cc> wrote:
> >
> > On Sat, Sep 22, 2018 at 07:32:28PM +0200, Robert Nagy wrote:
> > > diff --git a/libavformat/cache.c b/libavformat/cache.c
> > > index 66bbbf54c9..48ff5ab363 100644
> > > --- a/libavformat/cache.c
> > > +++ b/libavformat/cache.c
> > > @@ -153,6 +153,38 @@ fail:
> > >      return ret;
> > >  }
> > >
> > > +static int cache_read_ahead(URLContext *h)
> > > +{
> > > +    Context *c= h->priv_data;
> > > +    int64_t r, read_ahead, pos;
> > > +    uint8_t buf[32768];
> > > +
> > > +    pos = c->logical_pos;
> > > +    read_ahead = c->read_ahead_limit < 0
> > > +        ? 512 * 512
> > > +        : FFMIN(512 * 512, c->read_ahead_limit);
> > > +
> > > +    while (read_ahead > 0) {
> > > +        r = ffurl_read(c->inner, buf, FFMIN(read_ahead, sizeof(buf)));
> > > +        if (r == AVERROR_EOF) {
> > > +            c->is_true_eof = 1;
> > > +            av_assert0(c->end >= c->logical_pos);
> > > +        }
> > > +        if (r<=0)
> > > +            break;
> > > +        c->inner_pos += r;
> > > +
> > > +        add_entry(h, buf, r);
> > > +        c->logical_pos += r;
> > > +        c->end = FFMAX(c->end, c->logical_pos);
> > > +        read_ahead -= r;
> > > +    }
> > > +
> > > +    c->logical_pos = pos;
> > > +
> > > +    return r < 0 ? r : 0;
> > > +}
> > > +
> > >  static int cache_read(URLContext *h, unsigned char *buf, int size)
> > >  {
> > >      Context *c= h->priv_data;
> >
> > > @@ -215,6 +247,10 @@ static int cache_read(URLContext *h, unsigned
> > > char *buf, int size)
> >
> > still not cleanly applying (due to new lines)
> >
> > Applying: cache: read ahead to avoid excessive small requests
> > error: corrupt patch at line 45
> > error: could not build fake ancestor
> > Patch failed at 0001 cache: read ahead to avoid excessive small requests
> > hint: Use 'git am --show-current-patch' to see the failed patch
> > When you have resolved this problem, run "git am --continue".
> > If you prefer to skip this patch, run "git am --skip" instead.
> > To restore the original branch and stop patching, run "git am --abort".
> >
> >
> > [...]
> >
> > --
> > Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> >
> > The greatest way to live with honor in this world is to be what we pretend
> > to be. -- Socrates
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

>  cache.c |  111 ++++++++++++++++++++++++++++++++++++++++++++++++----------------
>  1 file changed, 84 insertions(+), 27 deletions(-)
> 5a1f791cd8dd72085fc27c9cdbf2d41a87f24fee  0001-cache-read-ahead-to-avoid-excessive-small-requests.patch
> From 17be70d9ffbfd1f55770e81958b597994abf2c99 Mon Sep 17 00:00:00 2001
> From: Robert Nagy <ronagy@icloud.com>
> Date: Sat, 22 Sep 2018 19:18:54 +0200
> Subject: [PATCH] cache: read ahead to avoid excessive small requests
> 

> This "fakes" a filler thread for reading ahead.

applies cleanly now, but why fake ?
reading alot more than requested would cause extra latency even if the user
application runs the format code in a seperate thread
It would be better to avoid this and not block a request for "block 1" any
longer than it takes to read "block 1"


[...]
Robert Nagy Sept. 25, 2018, 12:12 p.m. UTC | #4 
This is to resolve https://trac.ffmpeg.org/ticket/5080
On Mon, Sep 24, 2018 at 10:01 PM Michael Niedermayer
<michael@niedermayer.cc> wrote:
>
> On Sun, Sep 23, 2018 at 08:52:06PM +0200, Robert Nagy wrote:
> > On Sun, Sep 23, 2018 at 1:40 PM Michael Niedermayer
> > <michael@niedermayer.cc> wrote:
> > >
> > > On Sat, Sep 22, 2018 at 07:32:28PM +0200, Robert Nagy wrote:
> > > > diff --git a/libavformat/cache.c b/libavformat/cache.c
> > > > index 66bbbf54c9..48ff5ab363 100644
> > > > --- a/libavformat/cache.c
> > > > +++ b/libavformat/cache.c
> > > > @@ -153,6 +153,38 @@ fail:
> > > >      return ret;
> > > >  }
> > > >
> > > > +static int cache_read_ahead(URLContext *h)
> > > > +{
> > > > +    Context *c= h->priv_data;
> > > > +    int64_t r, read_ahead, pos;
> > > > +    uint8_t buf[32768];
> > > > +
> > > > +    pos = c->logical_pos;
> > > > +    read_ahead = c->read_ahead_limit < 0
> > > > +        ? 512 * 512
> > > > +        : FFMIN(512 * 512, c->read_ahead_limit);
> > > > +
> > > > +    while (read_ahead > 0) {
> > > > +        r = ffurl_read(c->inner, buf, FFMIN(read_ahead, sizeof(buf)));
> > > > +        if (r == AVERROR_EOF) {
> > > > +            c->is_true_eof = 1;
> > > > +            av_assert0(c->end >= c->logical_pos);
> > > > +        }
> > > > +        if (r<=0)
> > > > +            break;
> > > > +        c->inner_pos += r;
> > > > +
> > > > +        add_entry(h, buf, r);
> > > > +        c->logical_pos += r;
> > > > +        c->end = FFMAX(c->end, c->logical_pos);
> > > > +        read_ahead -= r;
> > > > +    }
> > > > +
> > > > +    c->logical_pos = pos;
> > > > +
> > > > +    return r < 0 ? r : 0;
> > > > +}
> > > > +
> > > >  static int cache_read(URLContext *h, unsigned char *buf, int size)
> > > >  {
> > > >      Context *c= h->priv_data;
> > >
> > > > @@ -215,6 +247,10 @@ static int cache_read(URLContext *h, unsigned
> > > > char *buf, int size)
> > >
> > > still not cleanly applying (due to new lines)
> > >
> > > Applying: cache: read ahead to avoid excessive small requests
> > > error: corrupt patch at line 45
> > > error: could not build fake ancestor
> > > Patch failed at 0001 cache: read ahead to avoid excessive small requests
> > > hint: Use 'git am --show-current-patch' to see the failed patch
> > > When you have resolved this problem, run "git am --continue".
> > > If you prefer to skip this patch, run "git am --skip" instead.
> > > To restore the original branch and stop patching, run "git am --abort".
> > >
> > >
> > > [...]
> > >
> > > --
> > > Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> > >
> > > The greatest way to live with honor in this world is to be what we pretend
> > > to be. -- Socrates
> > > _______________________________________________
> > > ffmpeg-devel mailing list
> > > ffmpeg-devel@ffmpeg.org
> > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> >  cache.c |  111 ++++++++++++++++++++++++++++++++++++++++++++++++----------------
> >  1 file changed, 84 insertions(+), 27 deletions(-)
> > 5a1f791cd8dd72085fc27c9cdbf2d41a87f24fee  0001-cache-read-ahead-to-avoid-excessive-small-requests.patch
> > From 17be70d9ffbfd1f55770e81958b597994abf2c99 Mon Sep 17 00:00:00 2001
> > From: Robert Nagy <ronagy@icloud.com>
> > Date: Sat, 22 Sep 2018 19:18:54 +0200
> > Subject: [PATCH] cache: read ahead to avoid excessive small requests
> >
>
> > This "fakes" a filler thread for reading ahead.
>
> applies cleanly now, but why fake ?
> reading alot more than requested would cause extra latency even if the user
> application runs the format code in a seperate thread
> It would be better to avoid this and not block a request for "block 1" any
> longer than it takes to read "block 1"
>
>
> [...]
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Observe your enemies, for they first find out your faults. -- Antisthenes
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
diff mbox

Patch

diff --git a/libavformat/cache.c b/libavformat/cache.c
index 66bbbf54c9..48ff5ab363 100644
--- a/libavformat/cache.c
+++ b/libavformat/cache.c
@@ -153,6 +153,38 @@  fail:
     return ret;
 }

+static int cache_read_ahead(URLContext *h)
+{
+    Context *c= h->priv_data;
+    int64_t r, read_ahead, pos;
+    uint8_t buf[32768];
+
+    pos = c->logical_pos;
+    read_ahead = c->read_ahead_limit < 0
+        ? 512 * 512
+        : FFMIN(512 * 512, c->read_ahead_limit);
+
+    while (read_ahead > 0) {
+        r = ffurl_read(c->inner, buf, FFMIN(read_ahead, sizeof(buf)));
+        if (r == AVERROR_EOF) {
+            c->is_true_eof = 1;
+            av_assert0(c->end >= c->logical_pos);
+        }
+        if (r<=0)
+            break;
+        c->inner_pos += r;
+
+        add_entry(h, buf, r);
+        c->logical_pos += r;
+        c->end = FFMAX(c->end, c->logical_pos);
+        read_ahead -= r;
+    }
+
+    c->logical_pos = pos;
+
+    return r < 0 ? r : 0;
+}
+
 static int cache_read(URLContext *h, unsigned char *buf, int size)
 {
     Context *c= h->priv_data;
@@ -215,6 +247,10 @@  static int cache_read(URLContext *h, unsigned
char *buf, int size)
     c->logical_pos += r;
     c->end = FFMAX(c->end, c->logical_pos);

+    // Fake filling thread to avoid excessive
+    // small reads.
+    cache_read_ahead(h);
+
     return r;
 }