[FFmpeg-devel] [PATCH] avformat/cache: Separate read and seek return-values within cache_read

[Bryan Huh]

Ok. Will push a separate patch with a one-line type-change and abandon this.

I wanted to avoid coercing "r" into a 32-bit int when add_entry is called.
I thought the compiler would give me warnings about it, but it doesn't seem
to be after all.

On Mon, Nov 2, 2015 at 7:04 AM, Michael Niedermayer <michael at niedermayer.cc>
wrote:

> On Sun, Nov 01, 2015 at 10:56:45PM -0800, Bryan Huh wrote:
> > Fixes an issue where an int64_t ffurl_seek return-value was being stored
> > in a generic int "r" variable, leading to integer overflow when seeking
> > into a large file (>2GB), and ultimately a "Failed to perform internal
> > seek" error mesage. The "r" variable was used both for storing the
> > result of any seeks and for the result of any reads. Instead, I separate
> > the variable into an int64_t seek-variable and int read-variable.
> >
> > To test, try running `ffprobe 'cache:http://<something>'` on a file that
> > is ~3GB large, whose moov atom is at the end of the file
> > ---
> >  libavformat/cache.c |   49
> +++++++++++++++++++++++++------------------------
> >  1 files changed, 25 insertions(+), 24 deletions(-)
> >
> > diff --git a/libavformat/cache.c b/libavformat/cache.c
> > index a762aa9..b654f45 100644
> > --- a/libavformat/cache.c
> > +++ b/libavformat/cache.c
> > @@ -156,7 +156,8 @@ static int cache_read(URLContext *h, unsigned char
> *buf, int size)
> >  {
> >      Context *c= h->priv_data;
> >      CacheEntry *entry, *next[2] = {NULL, NULL};
>
> > -    int r;
> > +    int64_t off;
> > +    int bytes_read = 0;
>
> renaming the variable is unrelated (its not neccessary)
> but if you want to rename/split it, i dont mind but it should be in
> a seperate patch
>
>
> >
> >      entry = av_tree_find(c->root, &c->logical_pos, cmp, (void**)next);
> >
> > @@ -169,21 +170,21 @@ static int cache_read(URLContext *h, unsigned char
> *buf, int size)
> >          if (in_block_pos < entry->size) {
> >              int64_t physical_target = entry->physical_pos +
> in_block_pos;
> >
>
> > -            if (c->cache_pos != physical_target) {
> > +            if (c->cache_pos != physical_target)
>
> removing {} is not needed, without it the change is smaller and
> simpeler to read (for example when someone reads git log changes)
>
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Avoid a single point of failure, be that a person or equipment.
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>