From e2683cb02435b91f43dfbc943c31831a55881bf5 Mon Sep 17 00:00:00 2001 From: Devin Anderson Date: Wed, 23 Mar 2011 02:44:24 -0700 Subject: [PATCH] Fix 'alsarawmidi' driver so that it actually works. Add functionality to 'midi_latency_test'. Fix bug in raw write queue implementation. Output error message when a source MIDI port isn't valid during mixdown. Output error messages for error conditions detected in buffer read and write queues. --- common/JackMidiBufferReadQueue.cpp | 16 ++++- common/JackMidiBufferWriteQueue.cpp | 18 ++++-- common/JackMidiPort.cpp | 7 +- common/JackMidiRawInputWriteQueue.cpp | 2 +- example-clients/midi_latency_test.c | 39 +++++++---- linux/alsarawmidi/JackALSARawMidiDriver.cpp | 64 +++++++++---------- linux/alsarawmidi/JackALSARawMidiDriver.h | 2 +- .../alsarawmidi/JackALSARawMidiInputPort.cpp | 1 + .../alsarawmidi/JackALSARawMidiOutputPort.cpp | 7 +- linux/alsarawmidi/JackALSARawMidiPort.cpp | 12 ++-- linux/alsarawmidi/JackALSARawMidiPort.h | 4 +- 11 files changed, 106 insertions(+), 66 deletions(-) diff --git a/common/JackMidiBufferReadQueue.cpp b/common/JackMidiBufferReadQueue.cpp index 66777eb0..c53e596c 100644 --- a/common/JackMidiBufferReadQueue.cpp +++ b/common/JackMidiBufferReadQueue.cpp @@ -46,12 +46,22 @@ JackMidiBufferReadQueue::DequeueEvent() void JackMidiBufferReadQueue::ResetMidiBuffer(JackMidiBuffer *buffer) { + event_count = 0; index = 0; - if (buffer) { + if (! buffer) { + jack_error("JackMidiBufferReadQueue::ResetMidiBuffer - buffer reset " + "to NULL"); + } else if (! buffer->IsValid()) { + jack_error("JackMidiBufferReadQueue::ResetMidiBuffer - buffer reset " + "to invalid buffer"); + } else { + uint32_t lost_events = buffer->lost_events; + if (lost_events) { + jack_error("JackMidiBufferReadQueue::ResetMidiBuffer - %d events " + "lost during mixdown", lost_events); + } this->buffer = buffer; event_count = buffer->event_count; last_frame_time = GetLastFrame(); - } else { - event_count = 0; } } diff --git a/common/JackMidiBufferWriteQueue.cpp b/common/JackMidiBufferWriteQueue.cpp index 5caab49d..7e6ae67f 100644 --- a/common/JackMidiBufferWriteQueue.cpp +++ b/common/JackMidiBufferWriteQueue.cpp @@ -49,9 +49,17 @@ void JackMidiBufferWriteQueue::ResetMidiBuffer(JackMidiBuffer *buffer, jack_nframes_t frames) { - this->buffer = buffer; - buffer->Reset(frames); - last_frame_time = GetLastFrame(); - max_bytes = buffer->MaxEventSize(); - next_frame_time = last_frame_time + frames; + if (! buffer) { + jack_error("JackMidiBufferWriteQueue::ResetMidiBuffer - buffer reset " + "to NULL"); + } else if (! buffer->IsValid()) { + jack_error("JackMidiBufferWriteQueue::ResetMidiBuffer - buffer reset " + "to invalid buffer"); + } else { + this->buffer = buffer; + buffer->Reset(frames); + last_frame_time = GetLastFrame(); + max_bytes = buffer->MaxEventSize(); + next_frame_time = last_frame_time + frames; + } } diff --git a/common/JackMidiPort.cpp b/common/JackMidiPort.cpp index 42ee245c..5bd82341 100644 --- a/common/JackMidiPort.cpp +++ b/common/JackMidiPort.cpp @@ -55,7 +55,6 @@ SERVER_EXPORT jack_midi_data_t* JackMidiBuffer::ReserveEvent(jack_nframes_t time lost_events++; return 0; } - JackMidiEvent* event = &events[event_count++]; event->time = time; event->size = size; @@ -90,7 +89,7 @@ static void MidiBufferMixdown(void* mixbuffer, void** src_buffers, int src_count { JackMidiBuffer* mix = static_cast(mixbuffer); if (!mix->IsValid()) { - jack_error("MIDI: invalid mix buffer"); + jack_error("Jack::MidiBufferMixdown - invalid mix buffer"); return; } mix->Reset(nframes); @@ -98,8 +97,10 @@ static void MidiBufferMixdown(void* mixbuffer, void** src_buffers, int src_count int event_count = 0; for (int i = 0; i < src_count; ++i) { JackMidiBuffer* buf = static_cast(src_buffers[i]); - if (!buf->IsValid()) + if (!buf->IsValid()) { + jack_error("Jack::MidiBufferMixdown - invalid source buffer"); return; + } buf->mix_index = 0; event_count += buf->event_count; mix->lost_events += buf->lost_events; diff --git a/common/JackMidiRawInputWriteQueue.cpp b/common/JackMidiRawInputWriteQueue.cpp index e2ff4453..f2853160 100644 --- a/common/JackMidiRawInputWriteQueue.cpp +++ b/common/JackMidiRawInputWriteQueue.cpp @@ -282,7 +282,7 @@ JackMidiRawInputWriteQueue::RecordByte(jack_midi_data_t byte) bool JackMidiRawInputWriteQueue::WriteEvent(jack_nframes_t boundary_frame) { - if (event.time < boundary_frame) { + if ((! boundary_frame) || (event.time < boundary_frame)) { switch (write_queue->EnqueueEvent(&event)) { case BUFFER_TOO_SMALL: HandleEventLoss(&event); diff --git a/example-clients/midi_latency_test.c b/example-clients/midi_latency_test.c index 27c651c8..732fe2ab 100644 --- a/example-clients/midi_latency_test.c +++ b/example-clients/midi_latency_test.c @@ -85,7 +85,8 @@ jack_nframes_t lowest_latency; jack_time_t lowest_latency_time; jack_midi_data_t *message_1; jack_midi_data_t *message_2; -size_t messages_processed; +size_t messages_received; +size_t messages_sent; size_t message_size; jack_latency_range_t out_latency_range; jack_port_t *out_port; @@ -100,6 +101,7 @@ int timeout; jack_nframes_t total_latency; jack_time_t total_latency_time; size_t unexpected_messages; +size_t xrun_count; static void output_error(const char *source, const char *message); @@ -156,11 +158,13 @@ handle_process(jack_nframes_t frames, void *arg) } highest_latency = 0; lowest_latency = 0; - messages_processed = 0; + messages_received = 0; + messages_sent = 0; process_state = 1; total_latency = 0; total_latency_time = 0; unexpected_messages = 0; + xrun_count = 0; jack_port_get_latency_range(remote_in_port, JackCaptureLatency, &in_latency_range); jack_port_get_latency_range(remote_out_port, JackPlaybackLatency, @@ -175,7 +179,7 @@ handle_process(jack_nframes_t frames, void *arg) last_frame_time = jack_last_frame_time(client); for (i = 0; i < event_count; i++) { jack_midi_event_get(&event, port_buffer, i); - message = (messages_processed % 2) ? message_2 : message_1; + message = (messages_received % 2) ? message_2 : message_1; if ((event.size == message_size) && (! memcmp(message, event.buffer, message_size * sizeof(jack_midi_data_t)))) { @@ -201,20 +205,20 @@ handle_process(jack_nframes_t frames, void *arg) lowest_latency = frame; lowest_latency_time = time; } - latency_time_values[messages_processed] = time; - latency_values[messages_processed] = frame; + latency_time_values[messages_received] = time; + latency_values[messages_received] = frame; total_latency += frame; total_latency_time += time; - messages_processed++; - if (messages_processed == samples) { + messages_received++; + if (messages_received == samples) { process_state = 2; sem_post(&semaphore); break; } send_message: frame = (jack_nframes_t) ((((double) rand()) / RAND_MAX) * frames); - if (frame == frames) { - frame--; + if (frame >= frames) { + frame = frames - 1; } port_buffer = jack_port_get_buffer(out_port, frames); jack_midi_clear_buffer(port_buffer); @@ -223,10 +227,11 @@ handle_process(jack_nframes_t frames, void *arg) set_process_error(SOURCE_EVENT_RESERVE, ERROR_RESERVE); break; } - message = (messages_processed % 2) ? message_2 : message_1; + message = (messages_sent % 2) ? message_2 : message_1; memcpy(buffer, message, message_size * sizeof(jack_midi_data_t)); last_activity = jack_last_frame_time(client) + frame; last_activity_time = jack_frames_to_time(client, last_activity); + messages_sent++; case 2: /* State: finished - do nothing */ @@ -245,6 +250,12 @@ handle_shutdown(void *arg) set_process_error("handle_shutdown", "The JACK server has been shutdown"); } +static void +handle_xrun(void *arg) +{ + xrun_count++; +} + static void output_error(const char *source, const char *message) { @@ -551,8 +562,14 @@ main(int argc, char **argv) printf(" > 10 ms: %u\n", jitter_plot[100]); } } + printf("\nMessages sent: %d\n" + "Messages received: %d\n", + messages_sent, messages_received); if (unexpected_messages) { - printf("\nUnexpected messages received: %d\n", unexpected_messages); + printf("Unexpected messages received: %d\n", unexpected_messages); + } + if (xrun_count) { + printf("Xruns: %d (messages may have been lost)", xrun_count); } deactivate_client: jack_deactivate(client); diff --git a/linux/alsarawmidi/JackALSARawMidiDriver.cpp b/linux/alsarawmidi/JackALSARawMidiDriver.cpp index adbb89fb..8e564c76 100755 --- a/linux/alsarawmidi/JackALSARawMidiDriver.cpp +++ b/linux/alsarawmidi/JackALSARawMidiDriver.cpp @@ -39,8 +39,12 @@ JackALSARawMidiDriver::Attach() { jack_nframes_t buffer_size = fEngineControl->fBufferSize; jack_port_id_t index; + jack_nframes_t latency = buffer_size; + jack_latency_range_t latency_range; const char *name; JackPort *port; + latency_range.max = latency; + latency_range.min = latency; for (int i = 0; i < fCaptureChannels; i++) { JackALSARawMidiInputPort *input_port = input_ports[i]; name = input_port->GetName(); @@ -55,9 +59,14 @@ JackALSARawMidiDriver::Attach() } port = fGraphManager->GetPort(index); port->SetAlias(input_port->GetAlias()); - port->SetLatency(buffer_size); + port->SetLatencyRange(JackCaptureLatency, &latency_range); fCapturePortList[i] = index; } + if (! fEngineControl->fSyncMode) { + latency += buffer_size; + latency_range.max = latency; + latency_range.min = latency; + } for (int i = 0; i < fPlaybackChannels; i++) { JackALSARawMidiOutputPort *output_port = output_ports[i]; name = output_port->GetName(); @@ -72,7 +81,7 @@ JackALSARawMidiDriver::Attach() } port = fGraphManager->GetPort(index); port->SetAlias(output_port->GetAlias()); - port->SetLatency(buffer_size); + port->SetLatencyRange(JackPlaybackLatency, &latency_range); fPlaybackPortList[i] = index; } return 0; @@ -103,23 +112,19 @@ JackALSARawMidiDriver::Execute() { jack_nframes_t timeout_frame = 0; for (;;) { + jack_nframes_t process_frame; jack_time_t wait_time; + jack_time_t *wait_time_ptr; unsigned short revents; if (! timeout_frame) { - wait_time = 0; + wait_time_ptr = 0; } else { jack_time_t next_time = GetTimeFromFrames(timeout_frame); jack_time_t now = GetMicroSeconds(); - - if (next_time <= now) { - goto handle_ports; - } - wait_time = next_time - now; + wait_time = next_time <= now ? 0 : next_time - now; + wait_time_ptr = &wait_time; } - - jack_info("Calling 'Poll' with wait time '%d'.", wait_time); - - if (Poll(wait_time) == -1) { + if (Poll(wait_time_ptr) == -1) { if (errno == EINTR) { continue; } @@ -139,8 +144,7 @@ JackALSARawMidiDriver::Execute() break; } handle_ports: - jack_nframes_t process_frame; - jack_nframes_t timeout_frame = 0; + timeout_frame = 0; for (int i = 0; i < fCaptureChannels; i++) { process_frame = input_ports[i]->ProcessALSA(); if (process_frame && ((! timeout_frame) || @@ -156,9 +160,6 @@ JackALSARawMidiDriver::Execute() } } } - - jack_info("ALSA thread is exiting."); - return false; } @@ -362,15 +363,15 @@ JackALSARawMidiDriver::Open(bool capturing, bool playing, int in_channels, #ifdef HAVE_PPOLL int -JackALSARawMidiDriver::Poll(jack_time_t wait_time) +JackALSARawMidiDriver::Poll(const jack_time_t *wait_time) { struct timespec timeout; struct timespec *timeout_ptr; if (! wait_time) { timeout_ptr = 0; } else { - timeout.tv_sec = wait_time / 1000000; - timeout.tv_nsec = (wait_time % 1000000) * 1000; + timeout.tv_sec = (*wait_time) / 1000000; + timeout.tv_nsec = ((*wait_time) % 1000000) * 1000; timeout_ptr = &timeout; } return ppoll(poll_fds, poll_fd_count, timeout_ptr, 0); @@ -379,15 +380,15 @@ JackALSARawMidiDriver::Poll(jack_time_t wait_time) #else int -JackALSARawMidiDriver::Poll(jack_time_t wait_time) +JackALSARawMidiDriver::Poll(const jack_time_t *wait_time) { int result = poll(poll_fds, poll_fd_count, - ! wait_time ? -1 : (int) (wait_time / 1000)); + ! wait_time ? -1 : (int) ((*wait_time) / 1000)); if ((! result) && wait_time) { - wait_time %= 1000; - if (wait_time) { + jack_time_t time_left = (*wait_time) % 1000; + if (time_left) { // Cheap hack. - usleep(wait_time); + usleep(time_left); result = poll(poll_fds, poll_fd_count, 0); } } @@ -399,9 +400,9 @@ JackALSARawMidiDriver::Poll(jack_time_t wait_time) int JackALSARawMidiDriver::Read() { + jack_nframes_t buffer_size = fEngineControl->fBufferSize; for (int i = 0; i < fCaptureChannels; i++) { - input_ports[i]->ProcessJack(GetInputBuffer(i), - fEngineControl->fBufferSize); + input_ports[i]->ProcessJack(GetInputBuffer(i), buffer_size); } return 0; } @@ -412,8 +413,7 @@ JackALSARawMidiDriver::Start() jack_info("JackALSARawMidiDriver::Start - Starting 'alsarawmidi' driver."); - // JackMidiDriver::Start(); - + JackMidiDriver::Start(); poll_fd_count = 1; for (int i = 0; i < fCaptureChannels; i++) { poll_fd_count += input_ports[i]->GetPollDescriptorCount(); @@ -471,11 +471,11 @@ JackALSARawMidiDriver::Start() poll_fd_iter += output_port->GetPollDescriptorCount(); } - jack_info("Starting ALSA thread ..."); + jack_info("JackALSARawMidiDriver::Start - starting ALSA thread ..."); if (! thread->StartSync()) { - jack_info("Started ALSA thread."); + jack_info("JackALSARawMidiDriver::Start - started ALSA thread."); return 0; } @@ -496,7 +496,7 @@ int JackALSARawMidiDriver::Stop() { - jack_info("Stopping 'alsarawmidi' driver."); + jack_info("JackALSARawMidiDriver::Stop - stopping 'alsarawmidi' driver."); if (fds[1] != -1) { close(fds[1]); diff --git a/linux/alsarawmidi/JackALSARawMidiDriver.h b/linux/alsarawmidi/JackALSARawMidiDriver.h index 5a793ebb..0ffc4e52 100755 --- a/linux/alsarawmidi/JackALSARawMidiDriver.h +++ b/linux/alsarawmidi/JackALSARawMidiDriver.h @@ -34,7 +34,7 @@ namespace Jack { int code); int - Poll(jack_time_t wait_time); + Poll(const jack_time_t *wait_time); public: diff --git a/linux/alsarawmidi/JackALSARawMidiInputPort.cpp b/linux/alsarawmidi/JackALSARawMidiInputPort.cpp index fe0d7911..94ae8c4a 100755 --- a/linux/alsarawmidi/JackALSARawMidiInputPort.cpp +++ b/linux/alsarawmidi/JackALSARawMidiInputPort.cpp @@ -101,6 +101,7 @@ JackALSARawMidiInputPort::ProcessJack(JackMidiBuffer *port_buffer, jack_event = thread_queue->DequeueEvent(); } for (; jack_event; jack_event = thread_queue->DequeueEvent()) { + // We add `frames` so that MIDI events align with audio as closely as // possible. switch (write_queue->EnqueueEvent(jack_event->time + frames, diff --git a/linux/alsarawmidi/JackALSARawMidiOutputPort.cpp b/linux/alsarawmidi/JackALSARawMidiOutputPort.cpp index 640d13f6..df1fc268 100755 --- a/linux/alsarawmidi/JackALSARawMidiOutputPort.cpp +++ b/linux/alsarawmidi/JackALSARawMidiOutputPort.cpp @@ -101,6 +101,10 @@ JackALSARawMidiOutputPort::ProcessALSA(int read_fd) jack_nframes_t next_frame = raw_queue->Process(); blocked = send_queue->IsBlocked(); if (blocked) { + + jack_info("JackALSARawMidiOutputPort::ProcessALSA - MIDI port is " + "blocked"); + SetPollEventMask(POLLERR | POLLNVAL | POLLOUT); return 0; } @@ -122,9 +126,6 @@ JackALSARawMidiOutputPort::ProcessJack(JackMidiBuffer *port_buffer, continue; } char c = 1; - - jack_info("Attempting to write to file descriptor '%d'", write_fd); - ssize_t result = write(write_fd, &c, 1); assert(result <= 1); if (! result) { diff --git a/linux/alsarawmidi/JackALSARawMidiPort.cpp b/linux/alsarawmidi/JackALSARawMidiPort.cpp index db476003..a2d3d8fc 100755 --- a/linux/alsarawmidi/JackALSARawMidiPort.cpp +++ b/linux/alsarawmidi/JackALSARawMidiPort.cpp @@ -49,7 +49,7 @@ JackALSARawMidiPort::JackALSARawMidiPort(snd_rawmidi_info_t *info, if (code) { error_message = snd_strerror(code); func = "snd_rawmidi_params_current"; - goto close; + goto free_params; } code = snd_rawmidi_params_set_avail_min(rawmidi, params, 1); if (code) { @@ -129,22 +129,22 @@ JackALSARawMidiPort::PopulatePollDescriptors(struct pollfd *poll_fd) int JackALSARawMidiPort::ProcessPollEvents() { - unsigned short revents; + unsigned short revents = 0; int code = snd_rawmidi_poll_descriptors_revents(rawmidi, poll_fds, num_fds, &revents); if (code) { - jack_error("JackALSARawMidiInputPort::ProcessPollEvents - " + jack_error("JackALSARawMidiPort::ProcessPollEvents - " "snd_rawmidi_poll_descriptors_revents: %s", snd_strerror(code)); return 0; } if (revents & POLLNVAL) { - jack_error("JackALSARawMidiInputPort::ProcessPollEvents - the file " + jack_error("JackALSARawMidiPort::ProcessPollEvents - the file " "descriptor is invalid."); } if (revents & POLLERR) { - jack_error("JackALSARawMidiInputPort::ProcessPollEvents - an error " - "has occurred on the device or stream."); + jack_error("JackALSARawMidiPort::ProcessPollEvents - an error has " + "occurred on the device or stream."); } return revents; } diff --git a/linux/alsarawmidi/JackALSARawMidiPort.h b/linux/alsarawmidi/JackALSARawMidiPort.h index 31ffd078..564e5b7b 100755 --- a/linux/alsarawmidi/JackALSARawMidiPort.h +++ b/linux/alsarawmidi/JackALSARawMidiPort.h @@ -30,7 +30,9 @@ namespace Jack { public: JackALSARawMidiPort(snd_rawmidi_info_t *info, size_t index); - virtual ~JackALSARawMidiPort(); + + virtual + ~JackALSARawMidiPort(); const char * GetAlias();