[FFmpeg-devel] [PATCH] libavdevice: JACK demuxer

[Olivier Guilyardi]

M?ns Rullg?rd a ?crit :
> Olivier Guilyardi <list at samalyse.com> writes:
> 
>> +typedef struct {
>> +    jack_client_t *     client;
>> +    int                 activated;
>> +    sem_t               mutex;
> 
> That is a confusing name.  A semaphore is not a mutex.

Okay, I'll correct this in my next version of the patch.

>> +            jack_ringbuffer_write(self->ctl_rb, (char *) &info, sizeof(info));
>> +            sem_post(&self->mutex);
> 
> This will post the semaphore once each time this function is called
> without overrun.

Indeed

>> +static int audio_read_packet(AVFormatContext *context, AVPacket *pkt)
>> +{
>> +    PrivateData *self = context->priv_data;
>> +    PacketControlInfo info;
>> +    int pkt_size, i, j;
>> +    float *output_data;
>> +    struct timespec timeout = {0, 0};
>> +
>> +    if (!self->activated && activate_jack(self, context))
>> +        return AVERROR(EIO);
>> +
>> +    timeout.tv_sec = av_gettime() / 1000000 + 2;
>> +    if (sem_timedwait(&self->mutex, &timeout)) {
> 
> This will wait on the semaphore once each time this function is
> called.  If these calls are less frequent than the calls to
> process_callback(), the semaphore count will accumulate until it is
> positive even though the buffer has no data, and an underrun will be
> reported.

No it won't necessarily wait every time the function is called. Say that the 
process callback (and thus sem_post()) has been called twice before 
audio_read_packet(). Then sem_timedwait() won't wait for the next two calls.

If the calls to audio_read_packet() are less frequent than the calls to 
process_callback() (which is likely to happen under heavy load, since the 
process callback runs at realtime priority), then the lock-free ringbuffer will 
get full, and process_callback() will detect an overrun, thus not calling 
sem_post() until some free space is available for writing in the ringbuffer.

When audio_read_packet() eventually gets called, it will read some data from the 
ringbuffer, thus freeing such space.

Each increment of the semaphore value means a new packet of data is available. I 
do not understand your last statement.

> I think you should be using a mutex and condition variable instead.

I would then need to implement a counting mechanism as the one provided by the 
semaphore in such a simple manner.

--
   Olivier