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

[Steve Lhomme]

On 2020-09-21 9:54, Thomas Guillem wrote:
> The simplification is disputable. If was simple before, and it is still simple now, but there is more code now.

This becomes one line in patch [24/24].

I consider the switch from pointer of pointer to a pointer a 
simplification. Plus it's the same code pattern already used elsewhere.

> On Fri, Sep 18, 2020, at 16:45, Steve Lhomme wrote:
>> 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 dbab0bf9bf7..f9e67a67663 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;
>>   };
>>   
>>   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;
>>   }
>>   
>> -- 
>> 2.26.2
>>
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>