[vlc-devel] jack output: patch for #8126

[Rémi Denis-Courmont]

On Wed, 10 Apr 2013 11:42:15 -0400, Tristan Matthews
<le.businessman at gmail.com> wrote:
>> I do not understand how this loop won't always fail if it ever iterates
>> more than once.
>>
> 
> jack_ringbuffer_write_space is called just before to see how much space
> the ringbuffer has. It is possible that we can't write all our samples
> right away,
> hence you might have to iterate more than once until enough write_space
is
> available
> in the ring buffer.

I know that the code should do that. The code does something else.

>> > +static void Flush(audio_output_t *p_aout, bool wait)
>> > +{
>> > +    struct aout_sys_t * p_sys = p_aout->sys;
>> > +    jack_ringbuffer_t *rb = p_sys->p_jack_ringbuffer;
>> > +    int i_error;
>> > +
>> > +    /* Sleep until the ring buffer has been emptied */
>> > +    if( wait )
>> > +    {
>> > +        const mtime_t wait_time =
>> > jack_frames_to_time(p_sys->p_jack_client,
>> > jack_get_buffer_size(p_sys->p_jack_client));
>> > +        while( jack_ringbuffer_write_space( rb ) != rb->size )
>> > +            msleep(wait_time);
>>
>> wait_time is probably wrong from the second iteration.
>>
> 
> The idea is to wait for a buffer's worth of time...I could wait
> for time == samples left in ringbuffer, is that what you mean?

I don't see why you'd wait more than once for total duration of the
buffer. And even that is longer than you should need to wait.


>> > @@ -345,7 +474,8 @@ static void Stop( audio_output_t *p_aout )
>> >      }
>> >      free( p_sys->p_jack_ports );
>> >      free( p_sys->p_jack_buffers );
>> > -    aout_PacketDestroy( p_aout );
>> > +    if( p_sys->p_jack_ringbuffer )
>>
>> Isn't this a tautology?
>>
> 
> If you call jack_ringbuffer_free on a NULL pointer, it will SEGFAULT.
> Are we guaranteed in Stop() that the ringbuffer has been created?

If you check it in Start(), yes.

-- 
Rémi Denis-Courmont
Sent from my collocated server