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

[Reimar Döffinger]

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...

> +#define _BSD_SOURCE 1

what exactly is that one needed for?

> +// 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.

> +    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...

> +    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.

> +    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?

> +            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.

> +            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?