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

[Tristan Matthews]

2013/4/10 Tristan Matthews <le.businessman at gmail.com>

> 2013/4/10 Rémi Denis-Courmont <remi at remlab.net>
>
>> 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.
>>
>>
>
> Here's the revised patch. It doesn't touch packet.c and hopefully it
> addresses all of the above comments.
>

Please disregard the last patch, the sleep in Flush() was not being
calculated correctly. Now it's just
sleeping for the delay returned by TimeGet()

Best,
Tristan


>
> Best,
> Tristan
>
>
>
>> --
>> Rémi Denis-Courmont
>> Sent from my collocated server
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> http://mailman.videolan.org/listinfo/vlc-devel
>>
>
>
>
> --
> Tristan Matthews
> web: http://tristanswork.blogspot.com
>



-- 
Tristan Matthews
web: http://tristanswork.blogspot.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20130410/be14d518/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-jack-don-t-use-legacy-packet-API.patch
Type: application/octet-stream
Size: 12954 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20130410/be14d518/attachment.obj>