[vlc-devel] [PATCH v4 08/24] picture_fifo: simplify the picture fifo tail handling

[Rémi Denis-Courmont]

Le maanantaina 21. syyskuuta 2020, 14.48.06 EEST Steve Lhomme a écrit :
> Rather than a pointer on the last picture pointer, we use either NULL (no
> tail/chain empty) or a pointer to the last picture in the chain.
> 
> Make sure that the pictures we queue don't have a p_next otherwise it would
> screw the computation of the last. (the issue existed before this patch)
> ---
>  src/misc/picture_fifo.c | 29 ++++++++++++++++++-----------
>  1 file changed, 18 insertions(+), 11 deletions(-)
> 
> diff --git a/src/misc/picture_fifo.c b/src/misc/picture_fifo.c
> index f999ec1a3f6..46838669e21 100644
> --- a/src/misc/picture_fifo.c
> +++ b/src/misc/picture_fifo.c
> @@ -39,30 +39,37 @@
>  struct picture_fifo_t {
>      vlc_mutex_t lock;
>      picture_t   *first;
> -    picture_t   **last_ptr;
> +    picture_t   *tail;

The current code is a very common pattern for FIFOs, that fits very well with 
C. It avoids making a bogus special case of the empty list. And indeed, this 
patch actually increases code size. This does not look like an improvement to 
me.

>  };
> 
>  static void PictureFifoReset(picture_fifo_t *fifo)
>  {
>      fifo->first    = NULL;
> -    fifo->last_ptr = &fifo->first;
> +    fifo->tail     = NULL;
>  }
>  static void PictureFifoPush(picture_fifo_t *fifo, picture_t *picture)
>  {
>      assert(!picture->p_next);
> -    *fifo->last_ptr = picture;
> -    fifo->last_ptr  = &picture->p_next;
> +    if (fifo->first == NULL)
> +    {
> +        fifo->first = picture;
> +        fifo->tail  = picture;
> +    }
> +    else
> +    {
> +        fifo->tail->p_next = picture;
> +        fifo->tail =  picture;
> +        picture->p_next = NULL; // we're appending a picture, not a chain
> +    }
>  }
>  static picture_t *PictureFifoPop(picture_fifo_t *fifo)
>  {
> -    picture_t *picture = fifo->first;
> +    if (fifo->first == NULL)
> +        return NULL;
> 
> -    if (picture) {
> -        fifo->first = picture->p_next;
> -        if (!fifo->first)
> -            fifo->last_ptr = &fifo->first;
> -        picture->p_next = NULL;
> -    }
> +    picture_t *picture = fifo->first;
> +    fifo->first = picture->p_next;
> +    picture->p_next = NULL;
>      return picture;
>  }


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