Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for targeting iOS #65

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 69 additions & 19 deletions pm_mac/pmmacosxcm.c
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,15 @@

#include <CoreServices/CoreServices.h>
#include <CoreMIDI/MIDIServices.h>
#include <CoreAudio/HostTime.h>
#include <unistd.h>
#include <libkern/OSAtomic.h>
#include <TargetConditionals.h>

#if TARGET_OS_OSX
#include <CoreAudio/HostTime.h>
#else
#include <mach/mach_time.h>
#endif

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be doing what PortTime was intended to do: provide a system-independent way to access time functions.
There are actually 2 porttime implementations for Mac: ptmacosx_cf.c based on core foundation calls and ptmacosx_mach.c based on Mach calls, but it looks like ptmacosx_mach.c still uses core foundation calls for time functions.
I'd much prefer to add system-independent functions in ptmacosx_mach.c and porttime.h than here for getting and translating nanos to portmidi (milliseconds).
A critical question though, is have you established that the nanos you get from Mach calls are the same nanos you get from CoreAudio? It's important because CoreMidi uses CoreAudio timestamps (undocumented but they match in tests) and it won't work to use just any nanosecond timer. I have no idea if mach gets the same nano values.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hence #65 (comment):

Ideally, we should probably share these utility methods between porttime and pm_mac. The right place for that would likely be porttime, but we'd also create a leaky abstraction by exposing these implementation details, even if only to the (also platform-specific) pm_mac. Ideas welcome.

The question would be how pmmacosxcm.c would consume those functions (or whether there's a way to model these conversions in a fully platform-agnostic way, so they can be part of the main porttime interface. Haven't dug deep enough into porttime to understand the architectural decisions there though).

A critical question though, is have you established that the nanos you get from Mach calls are the same nanos you get from CoreAudio? It's important because CoreMidi uses CoreAudio timestamps (undocumented but they match in tests) and it won't work to use just any nanosecond timer. I have no idea if mach gets the same nano values.

That would indeed be interesting to check, I haven't done so yet. I would be surprised though if they would differ.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think your additional functions to get host time and convert host time to micros and nanos is fine. At this point, only OSX and IOS would use them, and I think it's fine to make them Apple-only functions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm working to integrate a lot of changes into PortMidi. Returning to this, the whole reason for incorporating CoreAudio time into PortMidi was that the absolute time values are important because they are used as Midi timestamps by MacOS (I don't think this is documented). I believe many PortMidi tests would work fine with bad timestamps, so it's important to look at some MIDI packets in iOS and compare their timestamps to time sources to see what time source is being used (or find it in documentation if it is stated anywhere). Similarly, output timing should be tested to see if absolute timestamps based on mach_absolute_time() or whatever can be used to produce correctly timed output. If you can verify the time source is correct (at least usable for timing and seems correct), then this would be a nice addition to PortMidi.

#define PACKET_BUFFER_SIZE 1024
/* maximum overall data rate (OS X limits MIDI rate in case there
Expand Down Expand Up @@ -222,6 +228,10 @@ typedef struct coremidi_info_struct {
/* private function declarations */
MIDITimeStamp timestamp_pm_to_cm(PmTimestamp timestamp); // returns host time
PmTimestamp timestamp_cm_to_pm(MIDITimeStamp timestamp); // returns ms
static UInt64 current_host_time(void);
static UInt64 nanos_to_host_time(UInt64 nanos);
static UInt64 host_time_to_nanos(UInt64 host_time);
static UInt64 micros_per_host_tick(void);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

portmidi may be overoptimized to use host_time when possible rather than always using nanos (which would give a simpler, smaller set of time functions), but OK, let's extend porttime. These additional functions can exist for apple only (since they are not called from other platforms). Names should be, e.g. Pt_Current_Host_Time -- now I kind of hate this style, but it was originally supposed to be consistent with PortAudio, and better to be consistent that not (or to break everything by changing naming conventions now).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Names should be, e.g. Pt_Current_Host_Time

Pt_CurrentHostTime? That would be consistent with the PortMidi naming style:

PMEXPORT int Pm_HasHostError(PortMidiStream * stream);

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've moved these functions to a new header, ptmacosx.h, since they are Apple-specific and so they don't leak into the porttime interface on other platforms:

UInt64 Pt_CurrentHostTime(void);
UInt64 Pt_NanosToHostTime(UInt64 nanos);
UInt64 Pt_HostTimeToNanos(UInt64 host_time);
UInt64 Pt_MicrosPerHostTick(void);

char* cm_get_full_endpoint_name(MIDIEndpointRef endpoint, int *iac_flag);

Expand Down Expand Up @@ -259,7 +269,7 @@ static PmTimestamp midi_synchronize(PmInternal *midi)
{
coremidi_info_type info = (coremidi_info_type) midi->api_info;
UInt64 pm_stream_time_2 = // current time in ns
AudioConvertHostTimeToNanos(AudioGetCurrentHostTime());
host_time_to_nanos(current_host_time());
PmTimestamp real_time; // in ms
UInt64 pm_stream_time; // in ns
/* if latency is zero and this is an output, there is no
Expand All @@ -270,8 +280,7 @@ static PmTimestamp midi_synchronize(PmInternal *midi)
/* read real_time between two reads of stream time */
pm_stream_time = pm_stream_time_2;
real_time = (*midi->time_proc)(midi->time_info);
pm_stream_time_2 = AudioConvertHostTimeToNanos(
AudioGetCurrentHostTime());
pm_stream_time_2 = host_time_to_nanos(current_host_time());
/* repeat if more than 0.5 ms has elapsed */
} while (pm_stream_time_2 > pm_stream_time + 500000);
info->delta = pm_stream_time - ((UInt64) real_time * (UInt64) 1000000);
Expand Down Expand Up @@ -415,19 +424,19 @@ static void read_callback(const MIDIPacketList *newPackets, PmInternal *midi)
*/
CM_DEBUG printf("read_callback packet @ %lld ns (host %lld) "
"status %x length %d\n",
AudioConvertHostTimeToNanos(AudioGetCurrentHostTime()),
AudioGetCurrentHostTime(),
host_time_to_nanos(current_host_time()),
current_host_time(),
packet->data[0], packet->length);
for (packetIndex = 0; packetIndex < newPackets->numPackets; packetIndex++) {
/* Set the timestamp and dispatch this message */
CM_DEBUG printf(" packet->timeStamp %lld ns %lld host\n",
packet->timeStamp,
AudioConvertHostTimeToNanos(packet->timeStamp));
host_time_to_nanos(packet->timeStamp));
if (packet->timeStamp == 0) {
event.timestamp = now;
} else {
event.timestamp = (PmTimestamp) /* explicit conversion */ (
(AudioConvertHostTimeToNanos(packet->timeStamp) - info->delta) /
(host_time_to_nanos(packet->timeStamp) - info->delta) /
(UInt64) 1000000);
}
status = packet->data[0];
Expand Down Expand Up @@ -505,7 +514,7 @@ static coremidi_info_type create_macosxcm_info(int is_virtual, int is_input)
info->last_msg_length = 0;
info->min_next_time = 0;
info->isIACdevice = FALSE;
info->us_per_host_tick = 1000000.0 / AudioGetHostClockFrequency();
info->us_per_host_tick = micros_per_host_tick();
info->host_ticks_per_byte =
(UInt64) (1000000.0 / (info->us_per_host_tick * MAX_BYTES_PER_S));
info->packetList = (is_input ? NULL :
Expand Down Expand Up @@ -754,7 +763,7 @@ static PmError midi_write_flush(PmInternal *midi, PmTimestamp timestamp)
if (info->packet != NULL) {
/* out of space, send the buffer and start refilling it */
/* update min_next_time each flush to support rate limit */
UInt64 host_now = AudioGetCurrentHostTime();
UInt64 host_now = current_host_time();
if (host_now > info->min_next_time)
info->min_next_time = host_now;
if (info->is_virtual) {
Expand All @@ -780,8 +789,8 @@ static PmError send_packet(PmInternal *midi, Byte *message,
CM_DEBUG printf("add %d to packet %p len %d timestamp %lld @ %lld ns "
"(host %lld)\n",
message[0], info->packet, messageLength, timestamp,
AudioConvertHostTimeToNanos(AudioGetCurrentHostTime()),
AudioGetCurrentHostTime());
host_time_to_nanos(current_host_time()),
current_host_time());
info->packet = MIDIPacketListAdd(info->packetList,
sizeof(info->packetBuffer), info->packet,
timestamp, messageLength, message);
Expand Down Expand Up @@ -842,11 +851,11 @@ static PmError midi_write_short(PmInternal *midi, PmEvent *event)
* latency is zero. Both mean no timing and send immediately.
*/
if (when == 0 || midi->latency == 0) {
timestamp = AudioGetCurrentHostTime();
timestamp = current_host_time();
} else { /* translate PortMidi time + latency to CoreMIDI time */
timestamp = ((UInt64) (when + midi->latency) * (UInt64) 1000000) +
info->delta;
timestamp = AudioConvertNanosToHostTime(timestamp);
timestamp = nanos_to_host_time(timestamp);
}

message[0] = Pm_MessageStatus(what);
Expand Down Expand Up @@ -888,10 +897,10 @@ static PmError midi_begin_sysex(PmInternal *midi, PmTimestamp when)
when_ns = ((UInt64) (when + midi->latency) * (UInt64) 1000000) +
info->delta;
info->sysex_timestamp =
(MIDITimeStamp) AudioConvertNanosToHostTime(when_ns);
(MIDITimeStamp) nanos_to_host_time(when_ns);
UInt64 now; /* only make system time call when writing a virtual port */
if (info->is_virtual && info->sysex_timestamp <
(now = AudioGetCurrentHostTime())) {
(now = current_host_time())) {
info->sysex_timestamp = now;
}

Expand Down Expand Up @@ -963,6 +972,47 @@ static unsigned int midi_check_host_error(PmInternal *midi)
return FALSE;
}

UInt64 current_host_time(void)
{
#if TARGET_OS_OSX
return AudioGetCurrentHostTime();
#else
return mach_absolute_time();
#endif
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The mach methods are available on macOS too, so we could probably drop the macOS-specific AudioGetCurrentHostTime, AudioGetHostClockFrequency etc. I've kept them around under a TARGET_OS_OSX conditional for now to avoid accidentally introducing a regression if there's a bug in the (albeit quite short) mach-based implementation for iOS. Since it introduces an additional, likely not very heavily used code path, I would also be open to removing the macOS branch again. Thoughts welcome.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there some reason to choose mach_absolute_time()?

It seems to be the officially recommended drop-in replacement: https://developer.apple.com/library/archive/qa/qa1643/_index.html

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clock_gettime_nsec_np(CLOCK_UPTIME_RAW) seems to be a wrapper around mach_absolute_time:

https://github.com/apple-oss-distributions/Libc/blob/c5a3293354e22262702a3add5b2dfc9bb0b93b85/gen/clock_gettime.c#L117-L119

So it's probably a question of style which one to choose? mach_absolute_time seems pretty widely used, so I'm sure they won't deprecate that anytime soon.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, because documentation for mach_absolute_time says use clock_gettime_nsec_np(CLOCK_UPTIME_RAW) (!). But OK, if Apple can't decide, I'll go with your choice.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the link, I must have missed that note. Interesting that they mention fingerprinting. But regardless, mach_absolute_time is the lower level API and almost all projects I've seen that use Core Audio/Core MIDI on iOS went with mach_absolute_time too.


UInt64 nanos_to_host_time(UInt64 nanos)
{
#if TARGET_OS_OSX
return AudioConvertNanosToHostTime(nanos);
#else
mach_timebase_info_data_t clock_timebase;
mach_timebase_info(&clock_timebase);
return (nanos * clock_timebase.denom) / clock_timebase.numer;
#endif
}

UInt64 host_time_to_nanos(UInt64 host_time)
{
#if TARGET_OS_OSX
return AudioConvertHostTimeToNanos(host_time);
#else
mach_timebase_info_data_t clock_timebase;
mach_timebase_info(&clock_timebase);
return (host_time * clock_timebase.numer) / clock_timebase.denom;
#endif
}

UInt64 micros_per_host_tick(void)
{
#if TARGET_OS_OSX
return 1000000.0 / AudioGetHostClockFrequency();
#else
mach_timebase_info_data_t clock_timebase;
mach_timebase_info(&clock_timebase);
return clock_timebase.numer / (clock_timebase.denom * 1000.0);
#endif
}

MIDITimeStamp timestamp_pm_to_cm(PmTimestamp timestamp)
{
Expand All @@ -971,15 +1021,15 @@ MIDITimeStamp timestamp_pm_to_cm(PmTimestamp timestamp)
return (MIDITimeStamp)0;
} else {
nanos = (UInt64)timestamp * (UInt64)1000000;
return (MIDITimeStamp)AudioConvertNanosToHostTime(nanos);
return (MIDITimeStamp)nanos_to_host_time(nanos);
}
}


PmTimestamp timestamp_cm_to_pm(MIDITimeStamp timestamp)
{
UInt64 nanos;
nanos = AudioConvertHostTimeToNanos(timestamp);
nanos = host_time_to_nanos(timestamp);
return (PmTimestamp)(nanos / (UInt64)1000000);
}

Expand Down Expand Up @@ -1096,7 +1146,7 @@ static CFStringRef ConnectedEndpointName(MIDIEndpointRef endpoint,
if (nConnected) {
const SInt32 *pid = (const SInt32 *)(CFDataGetBytePtr(connections));
for (i = 0; i < nConnected; ++i, ++pid) {
MIDIUniqueID id = EndianS32_BtoN(*pid);
MIDIUniqueID id = CFSwapInt32BigToHost(*pid);
MIDIObjectRef connObject;
MIDIObjectType connObjectType;
err = MIDIObjectFindByUniqueID(id, &connObject,
Expand Down
55 changes: 49 additions & 6 deletions porttime/ptmacosx_mach.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,21 @@

#include <stdlib.h>
#include <stdio.h>
#include <CoreAudio/HostTime.h>

#import <mach/mach.h>
#import <mach/mach_error.h>
#import <mach/mach_time.h>
#import <mach/clock.h>
#include <unistd.h>
#include <AvailabilityMacros.h>
#include <MacTypes.h>
fwcd marked this conversation as resolved.
Show resolved Hide resolved
#include <TargetConditionals.h>

#if TARGET_OS_OSX
#include <CoreAudio/HostTime.h>
#else
#include <mach/mach_time.h>
#endif

#include "porttime.h"
#include "sys/time.h"
Expand All @@ -32,6 +39,11 @@ static int time_started_flag = FALSE;
static UInt64 start_time;
static pthread_t pt_thread_pid;

/* private function declarations */
static UInt64 current_host_time(void);
static UInt64 nanos_to_host_time(UInt64 nanos);
static UInt64 host_time_to_nanos(UInt64 host_time);
fwcd marked this conversation as resolved.
Show resolved Hide resolved

/* note that this is static data -- we only need one copy */
typedef struct {
int id;
Expand Down Expand Up @@ -119,8 +131,8 @@ static void *Pt_CallbackProc(void *p)
int delay = mytime++ * parameters->resolution - Pt_Time();
PtTimestamp timestamp;
if (delay < 0) delay = 0;
wait_time = AudioConvertNanosToHostTime((UInt64)delay * NSEC_PER_MSEC);
wait_time += AudioGetCurrentHostTime();
wait_time = nanos_to_host_time((UInt64)delay * NSEC_PER_MSEC);
wait_time += current_host_time();
mach_wait_until(wait_time);
timestamp = Pt_Time();
(*(parameters->callback))(timestamp, parameters->userData);
Expand All @@ -133,7 +145,7 @@ static void *Pt_CallbackProc(void *p)
PtError Pt_Start(int resolution, PtCallback *callback, void *userData)
{
if (time_started_flag) return ptAlreadyStarted;
start_time = AudioGetCurrentHostTime();
start_time = current_host_time();

if (callback) {
int res;
Expand Down Expand Up @@ -192,8 +204,8 @@ int Pt_Started(void)
PtTimestamp Pt_Time(void)
{
UInt64 clock_time, nsec_time;
clock_time = AudioGetCurrentHostTime() - start_time;
nsec_time = AudioConvertHostTimeToNanos(clock_time);
clock_time = current_host_time() - start_time;
nsec_time = host_time_to_nanos(clock_time);
return (PtTimestamp)(nsec_time / NSEC_PER_MSEC);
}

Expand All @@ -202,3 +214,34 @@ void Pt_Sleep(int32_t duration)
{
usleep(duration * 1000);
}

UInt64 current_host_time(void)
{
#if TARGET_OS_OSX
return AudioGetCurrentHostTime();
#else
return mach_absolute_time();
#endif
}

UInt64 nanos_to_host_time(UInt64 nanos)
{
#if TARGET_OS_OSX
return AudioConvertNanosToHostTime(nanos);
#else
mach_timebase_info_data_t clock_timebase;
mach_timebase_info(&clock_timebase);
return (nanos * clock_timebase.denom) / clock_timebase.numer;
#endif
}

UInt64 host_time_to_nanos(UInt64 host_time)
{
#if TARGET_OS_OSX
return AudioConvertHostTimeToNanos(host_time);
#else
mach_timebase_info_data_t clock_timebase;
mach_timebase_info(&clock_timebase);
return (host_time * clock_timebase.numer) / clock_timebase.denom;
#endif
}