[FFmpeg-devel] [PATCH 3/3] lavf/concat: implement FFSEEK_SIZE.

[Andreas Rheinhardt]

The commit title is wrong: It's AVSEEK_SIZE.

Nicolas George:
> Signed-off-by: Nicolas George <george at nsup.org>
> ---
>  libavformat/concat.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/concat.c b/libavformat/concat.c
> index 19c83c309a..ea3bc1dfde 100644
> --- a/libavformat/concat.c
> +++ b/libavformat/concat.c
> @@ -38,6 +38,7 @@ struct concat_data {
>      struct concat_nodes *nodes;    ///< list of nodes to concat
>      size_t               length;   ///< number of cat'ed nodes
>      size_t               current;  ///< index of currently read node
> +    uint64_t total_size;
>  };
>  
>  static av_cold int concat_close(URLContext *h)
> @@ -59,7 +60,7 @@ static av_cold int concat_open(URLContext *h, const char *uri, int flags)
>  {
>      char *node_uri = NULL;
>      int err = 0;
> -    int64_t size;
> +    int64_t size, total_size = 0;
>      size_t len, i;
>      URLContext *uc;
>      struct concat_data  *data = h->priv_data;
> @@ -112,6 +113,7 @@ static av_cold int concat_open(URLContext *h, const char *uri, int flags)
>          /* assembling */
>          nodes[i].uc   = uc;
>          nodes[i].size = size;
> +        total_size += size;
There is a potential for overflow here which is undefined behaviour as
total_size is signed. Simply making it unsigned (like you store it)
will solve the undefined behaviour here, but if a wraparound happened,
then seeking may return a nonsense value (not only when using
AVSEEK_SIZE, but certainly when using AVSEEK_SIZE) and may not work at
all. I see two solutions for this:
a) Erroring out when the total size doesn't fit into a int64_t.
b) Disallow seeking if the total size doesn't fit into a int64_t (in
this case, one might even allow input files that don't report a usable
size).
I don't know if a) would break any use cases (maybe some (buggy)
protocol reports INT64_MAX if the size is unknown?).
>      }
>      av_free(node_uri);
>      data->length = i;
> @@ -123,6 +125,7 @@ static av_cold int concat_open(URLContext *h, const char *uri, int flags)
>          err = AVERROR(ENOMEM);
>      } else
>          data->nodes = nodes;
> +    data->total_size = total_size;
>      return err;
>  }
>  
> @@ -158,6 +161,8 @@ static int64_t concat_seek(URLContext *h, int64_t pos, int whence)
>      struct concat_nodes *nodes = data->nodes;
>      size_t i;
>  
> +    if ((whence & AVSEEK_SIZE))
Unnecessary parentheses.
> +        return data->total_size;
>      switch (whence) {
>      case SEEK_END:
>          for (i = data->length - 1; i && pos < -nodes[i].size; i--)
>