[FFmpeg-devel] [PATCH 1/3] lavu: add helper functions for integer lists.

[Stefano Sabatini]

On date Thursday 2013-04-11 14:58:06 +0200, Nicolas George encoded:
> Add av_int_list_length() to compute a list length.
> Add av_opt_set_int_list() to set a binary option.

Note, please send updated patches as attachment to the previous
review, this helps me to manage the various patches.

> 
> Signed-off-by: Nicolas George <nicolas.george at normalesup.org>
> ---
>  doc/APIchanges     |    4 ++++
>  libavutil/avutil.h |   21 +++++++++++++++++++++
>  libavutil/opt.h    |   14 ++++++++++++++
>  libavutil/utils.c  |   19 +++++++++++++++++++
>  4 files changed, 58 insertions(+)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 08d4d66..652f8f8 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -15,6 +15,10 @@ libavutil:     2012-10-22
>  
>  API changes, most recent first:
>  
> +2013-04-10 - xxxxxxx - lavu 25.26.100 - avutil.h,opt.h
> +  Add av_int_list_length()
> +  and av_opt_set_int_list().
> +
>  2013-03-30 - xxxxxxx - lavu 52.24.100 - samplefmt.h
>    Add av_samples_alloc_array_and_samples().
>  
> diff --git a/libavutil/avutil.h b/libavutil/avutil.h
> index 78deff1..d71eb1e 100644
> --- a/libavutil/avutil.h
> +++ b/libavutil/avutil.h
> @@ -253,6 +253,27 @@ static inline void *av_x_if_null(const void *p, const void *x)
>  }
>  
>  /**
> + * Compute the length of an integer list.
> + *

> + * @param elsize  size in bytes of each list element (only 1, 2, 4 or 8)

Nit: (only sizes 1, 2, 4, or 8 are supported)

> + * @param term    list terminator (usually 0 or -1)
> + * @param list    pointer to the list
> + * @return  length of the list, in elements, not counting the terminator
> + */
> +unsigned av_int_list_length_for_size(unsigned elsize,
> +                                     const void *list, uint64_t term);
> +
> +/**
> + * Compute the length of an integer list.
> + *
> + * @param term  list terminator (usually 0 or -1)
> + * @param list  pointer to the list
> + * @return  length of the list, in elements, not counting the terminator
> + */
> +#define av_int_list_length(list, term) \
> +    av_int_list_length_for_size(sizeof(*list), list, term)
> +
> +/**
>   * @}
>   * @}
>   */
> diff --git a/libavutil/opt.h b/libavutil/opt.h
> index 30f729e..e368259 100644
> --- a/libavutil/opt.h
> +++ b/libavutil/opt.h
> @@ -656,6 +656,20 @@ int av_opt_set_image_size(void *obj, const char *name, int w, int h, int search_
>  int av_opt_set_pixel_fmt (void *obj, const char *name, enum AVPixelFormat fmt, int search_flags);
>  int av_opt_set_sample_fmt(void *obj, const char *name, enum AVSampleFormat fmt, int search_flags);
>  int av_opt_set_video_rate(void *obj, const char *name, AVRational val, int search_flags);
> +
> +/**
> + * Set a binary option to an integer list.

Maybe mention something along the lines of:

The size in bytes of the list is set in the obj field immediately next
after the pointer to the set list.

It was not obvious (at least for me).

> + *
> + * @param obj    AVClass object to set options on
> + * @param name   name of the binary option
> + * @param val    pointer to an integer list (must have the correct type with
> + *               regard to the contents of the list)
> + * @param term   list terminator (usually 0 or -1)
> + * @param flags  search flags
> + */

> +#define av_opt_set_int_list(obj, name, val, term, flags) \
> +    av_opt_set_bin(obj, name, (const uint8_t *)val, \
> +                   av_int_list_length(val, term) * sizeof(*val), flags)

Couldn't this overflow?

>  /**
>   * @}
>   */
> diff --git a/libavutil/utils.c b/libavutil/utils.c
> index fbfbc49..291c736 100644
> --- a/libavutil/utils.c
> +++ b/libavutil/utils.c
> @@ -79,3 +79,22 @@ char av_get_picture_type_char(enum AVPictureType pict_type)
>      default:                 return '?';
>      }
>  }
> +
> +unsigned av_int_list_length_for_size(unsigned elsize,
> +                                     const void *list, uint64_t term)
> +{
> +    unsigned i;
> +
> +    if (!list)
> +        return 0;
> +#define LIST_LENGTH(type) \
> +    { type t = term, *l = list; for (i = 0; l[i] != t; i++); }
> +    switch (elsize) {
> +    case 1: LIST_LENGTH(uint8_t);  break;
> +    case 2: LIST_LENGTH(uint16_t); break;
> +    case 4: LIST_LENGTH(uint32_t); break;
> +    case 8: LIST_LENGTH(uint64_t); break;
> +    default: av_assert0(!"valid element size");
> +    }
> +    return i;
> +}

LGTM otherwise, thanks.
-- 
FFmpeg = Frenzy and Formidable MultiPurpose Erotic Gadget