<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">2013/4/10 Tristan Matthews <span dir="ltr"><<a href="mailto:le.businessman@gmail.com" target="_blank">le.businessman@gmail.com</a>></span><br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">2013/4/10 Rémi Denis-Courmont <span dir="ltr"><<a href="mailto:remi@remlab.net" target="_blank">remi@remlab.net</a>></span><br>
<div class="gmail_extra"><div class="gmail_quote"><div><div class="h5"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div>On Wed, 10 Apr 2013 11:42:15 -0400, Tristan Matthews<br>
<<a href="mailto:le.businessman@gmail.com" target="_blank">le.businessman@gmail.com</a>> wrote:<br>
>> I do not understand how this loop won't always fail if it ever iterates<br>
>> more than once.<br>
>><br>
><br>
> jack_ringbuffer_write_space is called just before to see how much space<br>
> the ringbuffer has. It is possible that we can't write all our samples<br>
> right away,<br>
> hence you might have to iterate more than once until enough write_space<br>
is<br>
> available<br>
> in the ring buffer.<br>
<br>
</div>I know that the code should do that. The code does something else.<br>
<div><br>
>> > +static void Flush(audio_output_t *p_aout, bool wait)<br>
>> > +{<br>
>> > +    struct aout_sys_t * p_sys = p_aout->sys;<br>
>> > +    jack_ringbuffer_t *rb = p_sys->p_jack_ringbuffer;<br>
>> > +    int i_error;<br>
>> > +<br>
>> > +    /* Sleep until the ring buffer has been emptied */<br>
>> > +    if( wait )<br>
>> > +    {<br>
>> > +        const mtime_t wait_time =<br>
>> > jack_frames_to_time(p_sys->p_jack_client,<br>
>> > jack_get_buffer_size(p_sys->p_jack_client));<br>
>> > +        while( jack_ringbuffer_write_space( rb ) != rb->size )<br>
>> > +            msleep(wait_time);<br>
>><br>
>> wait_time is probably wrong from the second iteration.<br>
>><br>
><br>
> The idea is to wait for a buffer's worth of time...I could wait<br>
> for time == samples left in ringbuffer, is that what you mean?<br>
<br>
</div>I don't see why you'd wait more than once for total duration of the<br>
buffer. And even that is longer than you should need to wait.<br>
<div><br>
<br>
>> > @@ -345,7 +474,8 @@ static void Stop( audio_output_t *p_aout )<br>
>> >      }<br>
>> >      free( p_sys->p_jack_ports );<br>
>> >      free( p_sys->p_jack_buffers );<br>
>> > -    aout_PacketDestroy( p_aout );<br>
>> > +    if( p_sys->p_jack_ringbuffer )<br>
>><br>
>> Isn't this a tautology?<br>
>><br>
><br>
> If you call jack_ringbuffer_free on a NULL pointer, it will SEGFAULT.<br>
> Are we guaranteed in Stop() that the ringbuffer has been created?<br>
<br>
</div>If you check it in Start(), yes.<br>
<div><div><br></div></div></blockquote></div></div><div><br><br>Here's the revised patch. It doesn't touch packet.c and hopefully it addresses all of the above comments.<br></div></div></div></div></blockquote><div>
<br></div><div>Please disregard the last patch, the sleep in Flush() was not being calculated correctly. Now it's just<br></div><div>sleeping for the delay returned by TimeGet()<br><br></div><div>Best,<br>Tristan<br></div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br>Best,<br></div><div>Tristan<br>
<br></div><div class="im"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div>
--<br>
Rémi Denis-Courmont<br>
Sent from my collocated server<br>
_______________________________________________<br>
vlc-devel mailing list<br>
To unsubscribe or modify your subscription options:<br>
<a href="http://mailman.videolan.org/listinfo/vlc-devel" target="_blank">http://mailman.videolan.org/listinfo/vlc-devel</a><br>
</div></div></blockquote></div></div><br><br clear="all"><div class="im"><br>-- <br>Tristan Matthews<br>web: <a href="http://tristanswork.blogspot.com" target="_blank">http://tristanswork.blogspot.com</a><br>
</div></div></div>
</blockquote></div><br><br clear="all"><br>-- <br>Tristan Matthews<br>web: <a href="http://tristanswork.blogspot.com">http://tristanswork.blogspot.com</a><br>
</div></div>