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

[Olivier Guilyardi]

Reimar D?ffinger wrote:
> On Fri, Feb 27, 2009 at 06:58:55PM +0100, Olivier Guilyardi wrote:
>> +jack_demuxer_extralibs="-ljack -lpthread -lrt"
> 
> just -ljack works just fine for MPlayer...

Okay, corrected.

> 
>> +#define _BSD_SOURCE 1
> 
> what exactly is that one needed for?

It's for usleep() from unistd.h. For some reason, it doesn't work if I add it
right above #include <unistd.h>

> 
>> +// Size of the internal ringbuffer as a number of jack buffers
>> +#define AV_JACK_RING_NBUFFERS 4
>> +
>> +typedef float AVJackSampleType;
> 
> The AV prefix are for things that are part of the public API,
> FF prefix for stuff that is part of the internal API, and these (and a
> lot of others) are neither.

Prefixes are gone.

> 
>> +    rb_size = self->nports * self->buffer_size * AV_JACK_RING_NBUFFERS *
>> sizeof(float);
> 
> Seems like that patch got mangled by your mailer...
> And I don't really like the AVJackSampleType typedef, but if you have it
> you should use it...

AVJackSampleType was a leftover.

> 
>> +    self->ports = av_malloc(self->nports * sizeof(jack_port_t *));
> 
> sizeof(*self->ports) usually is nicer.
> 
>> +    for (i = 0; i < self->nports; i++) {
>> +        snprintf(str, 16, "input_%d", i + 1);
> 
> Since str is used only here, I'd suggest moving the declaration into
> this block, too.
> Also sizeof(str) instead of 16 should be used.

Fixed

> 
>> +    jack_set_process_callback(self->client, av_jack_process_callback, (void *)
>> self);
>> +    jack_on_shutdown(self->client, av_jack_shutdown_callback, (void *) self);
>> +    jack_set_xrun_callback(self->client, av_jack_xrun_callback, (void *) self);
> 
> The casts should not be necessary?

Right, they're gone.

> 
>> +            info.pts = av_gettime() - (latency + cycle_delay) * self->frame_ms;
> 
> The system time should not be stored anywhere without explicit
> permission from the user, particularly not as an absolute value.

That's the way it's done in oss and x11grab demuxers. And to be honest with you,
after various discussions on this mailing-list and LAD, I got quite confused
about timestamping. There seem to be as many opinions as there are keyboards on
this topic.

>> +            jack_ringbuffer_write(self->ctl_rb, (char *) &info,
>> sizeof(AVJackPacketInfo));
> 
> sizeof(info) is much easier to verify for correctness (this applies many other places, too).
> 
>> +    for (i = 0; i < self->nports; i++) {
>> +        jack_ringbuffer_read(self->data_rb, (char *) self->channel_buffer,
>> +                             info.size * sizeof(float));
>> +        for (j = 0; j < info.size; j++) {
>> +            output_data[j * self->nports + i] = self->channel_buffer[j];
>> +        }
>> +    }
> 
> Isn't there already some function that can do this channel-interleaving,
> possibly even in-place?

I dunno, grep didn't help..

--
  Olivier
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ffmpeg-r17647-jack-0.4.patch
Type: text/x-patch
Size: 11872 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090227/9d120f60/attachment.bin>