[vlc-devel] [PATCH v2 01/24] picture: add helpers for picture chaining

[Rémi Denis-Courmont]

Le perjantaina 18. syyskuuta 2020, 17.45.07 EEST Steve Lhomme a écrit :
> Picture chains are used either to keep a list of picture (in FIFO order) or
> to attach a picture to another and return them all at once (filters). ---
>  include/vlc_picture.h | 90 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 90 insertions(+)
> 
> diff --git a/include/vlc_picture.h b/include/vlc_picture.h
> index 7bb9ee5083c..44fcf6f3cec 100644
> --- a/include/vlc_picture.h
> +++ b/include/vlc_picture.h
> @@ -167,6 +167,96 @@ static inline vlc_video_context*
> picture_GetVideoContext(picture_t *pic) return pic->context ?
> pic->context->vctx : NULL;
>  }
> 
> +/**
> + * picture chaining helpers
> + */
> +
> +/**
> + * Check whether a picture has other pictures linked
> + */
> +static inline bool picture_has_chained_pics(const picture_t *pic)
> +{
> +    return chain->p_next != NULL;
> +}

TBH, I have mixed feelings about the style.

In principles, I prefer lower-case and underscore. But in VLC, the habit is to 
capitalise the verb - unless the function is named after some non-VLC 
function.

> +
> +/**
> + * Pop the chain from a picture chain.

? Do you mean pop a picture from the picture  chain?

> + *
> + * The next picture in the chain becomes the front of the picture chain.
> + *
> + * \return the front of the picture chain, with other pictures still
> chained
> + */
> +static inline picture_t * picture_chain_pop_chain(picture_t **chain)
> +{
> +    picture_t *front = *chain;
> +    if (front)
> +        *chain = front->p_next;
> +    return front;
> +}
> +
> +/**
> + * Pop the front of a picture chain.
> + *
> + * The next picture in the chain becomes the front of the picture chain.
> + *
> + * \return the front of the picture chain (the picture itself)
> + */
> +static inline picture_t * picture_chain_pop_front(picture_t **chain)
> +{
> +    picture_t *front = picture_chain_pop_chain(chain);
> +    if (front)
> +        // unlink the front picture from the rest of the chain
> +        front->p_next = NULL;
> +    return front;
> +}

Why do you split those two functions? I don't think the 'next = NULL' 
assignment is worth optimising against.

> +
> +/**
> + * Reset a picture chain.
> + *
> + * \return the picture chain that was contained in the picture
> + */
> +static inline picture_t * picture_get_and_reset_chain(picture_t *pic)
> +{
> +    picture_t *chain = pic->p_next;
> +    pic->p_next = NULL;
> +    return chain;
> +}
> +
> +/**
> + * Append a picture chain to a picture chain.
> + */
> +static inline void picture_chain_append_chain(picture_t *chain, picture_t
> *tail) +{
> +    chain->p_next = tail;
> +}
> +
> +/**
> + * Append a picture to a picture chain.
> + *
> + * \param chain the picture chain pointer
> + * \param pic the picture to append to the chain
> + *
> + * \return the new tail of the picture chain
> + */
> +static inline picture_t * picture_chain_append(picture_t *chain, picture_t
> *pic) VLC_USED +{
> +    chain->p_next = pic;
> +    pic->p_next = NULL; // we're appending a picture, not a chain
> +    return pic;
> +}
> +
> +/**
> + * Add a picture at the front of a picture chain.
> + *
> + * The added picture becomes the front of the picture chain.
> + */
> +static inline void picture_chain_prepend(picture_t **chain, picture_t *pic)
> +{
> +    pic->p_next = *chain;
> +    *chain = pic;
> +}
> +
> +
>  /**
>   * This function will create a new picture.
>   * The picture created will implement a default release management
> compatible


-- 
Реми Дёни-Курмон
http://www.remlab.net/