[vlc-devel] [PATCH] opensles: protect buffer array with a spinlock

[Dominique Martinet]

Hi,

Appending the buffer to the array before playing is nice and a good
idea, _except_ when the array is full.
Because when it's full, you write the new buffer that would be cleaned
when the old one is done playing, i.e. i_toappend_buffer ==
i_toclean_buffer.
I was never doing this check, because if the case came up then we'd get
a BUFFER_INSUFFICIENT error and just try again without writing to the
buffer array, but now it explains your error:

the buffer's full (opensles' internal queue is of fixed size BUFF_QUEUE):
> W/libOpenSLES(13045): Leaving BufferQueue::Enqueue
> (SL_RESULT_BUFFER_INSUFFICIENT)
> E/vlc     (13045): buffer insufficient
the current buffer is done being played, PlayCallback is called and clears
p_buffer_array[p_sys->i_toclean_buffer] ==
p_buffer_array[p_sys->i_toappend_buffer] == p_buffer, you try to enqueue
again with a buffer that's just been cleared... Which isn't good.
> W/libOpenSLES(13045): Leaving BufferQueue::Enqueue
> (SL_RESULT_PARAMETER_INVALID)
> W/vlc     (13045): Error 2, dropping buffer

A possible fix would be to make opensles' queue one element shorter than
our p_buffer_array, so it would still work just as it did up till now.
(A better fix would be to replace the whole thing by a FIFO queue)



By the way, I just noticed that there was a number of tries added for
when we get a buffer_insufficient error, I don't know if we really want
that one - did you have many "buffer insufficient" errors in your logs?
By that I mean before your patch :-)
(it should be ok anyway since there is no break, it should print an
"error 7 dropping buffer" message and free it.. actually we probably
want it if playback is too slow for cpu reasons)

If you have too much, I see two possible reasons:
 - the buffers are too small given the playrate and it really needs a
 bigger queue size
and/or
 - the buffers aren't played as fast as we would like to queue them,
 i.e. there's a problem with the playrate/some parameters


I think I'll give all this another try soonish (i.e. tonight or
tomorrow) and see if I can reproduce your problems on my phone, it was
playing alright a few months ago..

Regards,
-- 
Dominique Martinet | Asmadeus