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

[Tristan Matthews]

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.

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20130410/7b4445ce/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-jack-don-t-use-legacy-packet-API.patch
Type: application/octet-stream
Size: 13170 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20130410/7b4445ce/attachment.obj>