From 4951b8a7d435baa831aae41e973dcf1405c18594 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Fri, 16 Sep 2022 20:00:17 +0100 Subject: [PATCH 01/19] It seems the RTSP link can go idle occasionally, so don't do anything drastic until it comes back, i.e. don't kill everything when it goes idle. --- rtsp.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rtsp.c b/rtsp.c index 3477bbe8..8188425e 100644 --- a/rtsp.c +++ b/rtsp.c @@ -1281,24 +1281,24 @@ ssize_t read_from_rtsp_connection(rtsp_conn_info *conn, void *buf, size_t count) debug(1, "Connection %d: RTSP connection is idle.", conn->connection_number); conn->rtsp_link_is_idle = 1; #ifdef CONFIG_AIRPLAY_2 - conn->last_anchor_info_is_valid = 0; clear_ptp_clock(); #endif - conn->anchor_remote_info_is_valid = 0; conn->udp_clock_sender_is_initialised = 0; conn->udp_clock_is_initialised = 0; } response = timed_read_from_rtsp_connection(conn, 0, buf, count); } if (conn->rtsp_link_is_idle == 1) { + conn->rtsp_link_is_idle = 0; debug(1, "Connection %d: RTSP connection traffic has resumed.", conn->connection_number); + conn->anchor_remote_info_is_valid = 0; conn->local_to_remote_time_difference_measurement_time = 0; conn->local_to_remote_time_difference = 0; - conn->rtsp_link_is_idle = 0; conn->first_packet_timestamp = 0; conn->input_frame_rate_starting_point_is_valid = 0; ab_resync(conn); #ifdef CONFIG_AIRPLAY_2 + conn->last_anchor_info_is_valid = 0; set_client_as_ptp_clock(conn); #endif } From cae9492d0aab66f7bd084e57d47244a50ad87bc4 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Fri, 16 Sep 2022 20:02:32 +0100 Subject: [PATCH 02/19] Add in, but comment out, some code to investigate discontinuities in timestamps of audio coming from iOS 16.0 devices. Also clean up code to supress initial AAC transients when skipping or scrubbing. There must be a nicer way to do this. --- rtp.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 48 insertions(+), 12 deletions(-) diff --git a/rtp.c b/rtp.c index ce1376c4..b4b6ad0f 100644 --- a/rtp.c +++ b/rtp.c @@ -2374,12 +2374,13 @@ void *rtp_buffered_audio_processor(void *arg) { int pcm_buffer_occupancy = 0; int pcm_buffer_read_point = 0; // offset to where the next buffer should come from uint32_t pcm_buffer_read_point_rtptime = 0; - // uint32_t expected_rtptime; + // uint32_t expected_pcm_buffer_read_point_rtptime = 0; uint64_t blocks_read = 0; uint64_t blocks_read_since_flush = 0; int flush_requested = 0; - + uint32_t expected_timestamp = 0; + // int expected_timesamp_is_reasonable = 0; uint32_t timestamp = 0; // initialised to avoid a "possibly uninitialised" warning. int streaming_has_started = 0; int play_enabled = 0; @@ -2455,15 +2456,17 @@ void *rtp_buffered_audio_processor(void *arg) { flushUntilSeq, seq_no, flushUntilTS, timestamp); else debug(2, - "flush request ended with flushUntilSeq, flushUntilTS: %u, incoming timestamp: %u", + "flush request ended with seqNo = flushUntilSeq at %u, flushUntilTS: %u, incoming timestamp: %u", flushUntilSeq, flushUntilTS, timestamp); conn->ap2_flush_requested = 0; flush_request_active = 0; flush_newly_requested = 0; } } - if ((flush_requested) && (flush_request_active == 0)) + if ((flush_requested) && (flush_request_active == 0)) { flush_newly_complete = 1; + blocks_read_since_flush = 1; // the last block always (?) becomes the first block after the flush + } flush_requested = flush_request_active; // flush_requested = conn->ap2_flush_requested; if ((play_enabled) && (conn->ap2_play_enabled == 0)) @@ -2474,7 +2477,6 @@ void *rtp_buffered_audio_processor(void *arg) { // do this outside the flush mutex if (flush_newly_complete) { debug(2, "Flush Complete."); - blocks_read_since_flush = 0; } if (play_newly_stopped != 0) @@ -2532,7 +2534,7 @@ void *rtp_buffered_audio_processor(void *arg) { // only accept them if they have sensible lead times if ((lead_time < (int64_t)30000000000L) && (lead_time >= 0)) { // if it's the very first block (thus no priming needed) - if ((blocks_read == 1) || (blocks_read_since_flush > 3)) { + //if ((blocks_read == 1) || (blocks_read_since_flush > 3)) { if ((lead_time >= (int64_t)(requested_lead_time * 1000000000L)) || (streaming_has_started != 0)) { if (streaming_has_started == 0) @@ -2565,13 +2567,34 @@ void *rtp_buffered_audio_processor(void *arg) { */ // clang-format on + // debug(1,"block timestamp: %u, packet timestamp: %u.", timestamp, pcm_buffer_read_point_rtptime); + +/* + if (streaming_has_started != 0) { + int32_t timestamp_difference = pcm_buffer_read_point_rtptime - expected_pcm_buffer_read_point_rtptime; + if (timestamp_difference != 0) + debug(1,"Unexpected time difference between packets -- actual: %u, expected: %u, difference: %d. Packets played: %d. Blocks played since flush: %d. ", + pcm_buffer_read_point_rtptime, expected_pcm_buffer_read_point_rtptime, timestamp_difference, streaming_has_started, blocks_read_since_flush); + } +*/ + + // if it's not the very first block of AAC, but is from the first few blocks of a new AAC sequence, + // it will contain noisy transients, so replace it with silence. + if ((blocks_read != 1) && (blocks_read_since_flush <= 3)) { + // debug(1,"muting packet %u from block %u because it's not from a true starting block and blocks_read_since_flush is %" PRIu64 ".", pcm_buffer_read_point_rtptime, timestamp, blocks_read_since_flush); + conn->previous_random_number = + generate_zero_frames((char *)(pcm_buffer + pcm_buffer_read_point), 352, config.output_format, conn->enable_dither, + conn->previous_random_number); + } + player_put_packet(0, 0, pcm_buffer_read_point_rtptime, pcm_buffer + pcm_buffer_read_point, 352, conn); streaming_has_started++; + // expected_pcm_buffer_read_point_rtptime = pcm_buffer_read_point_rtptime + 352; } - } + // } } else { - debug(2, + debug(1, "Dropping packet %u from block %u with out-of-range lead_time: %.3f seconds.", pcm_buffer_read_point_rtptime, seq_no, 0.000000001 * lead_time); } @@ -2586,11 +2609,13 @@ void *rtp_buffered_audio_processor(void *arg) { usleep(20000); // wait before asking if play is enabled again } } else { + // debug(1,"new buffer needed for buffer starting at %u because pcm_buffer_read_point (frames) is %u and pcm_buffer_occupancy (frames) is %u.", pcm_buffer_read_point_rtptime, pcm_buffer_read_point/conn->input_bytes_per_frame, + // pcm_buffer_occupancy/conn->input_bytes_per_frame); new_buffer_needed = 1; if (pcm_buffer_read_point != 0) { // debug(1,"pcm_buffer_read_point (frames): %u, pcm_buffer_occupancy (frames): %u", - // pcm_buffer_read_point/conn->input_bytes_per_frame, - // pcm_buffer_occupancy/conn->input_bytes_per_frame); if there is anything to move down + //pcm_buffer_read_point/conn->input_bytes_per_frame, + //pcm_buffer_occupancy/conn->input_bytes_per_frame); // if there is anything to move down // to the front of the buffer, do it now; if ((pcm_buffer_occupancy - pcm_buffer_read_point) > 0) { // move the remaining frames down to the start of the buffer @@ -2610,7 +2635,7 @@ void *rtp_buffered_audio_processor(void *arg) { if ((flush_requested) || (new_buffer_needed)) { // debug(1,"pcm_buffer_read_point (frames): %u, pcm_buffer_occupancy (frames): %u", - // pcm_buffer_read_point/4, pcm_buffer_occupancy/4); + // pcm_buffer_read_point/conn->input_bytes_per_frame, pcm_buffer_occupancy/conn->input_bytes_per_frame); // ok, so here we know we need material from the sender // do we will get in a packet of audio uint16_t data_len; @@ -2654,6 +2679,13 @@ void *rtp_buffered_audio_processor(void *arg) { */ seq_no = packet[1] * (1 << 16) + packet[2] * (1 << 8) + packet[3]; timestamp = nctohl(&packet[4]); + // debug(1,"New block timestamp: %u.", timestamp); + // int32_t timestamp_difference = timestamp - expected_timestamp; + // if ((timestamp_difference != 0) && (expected_timesamp_is_reasonable != 0)) + // debug(1, "Unexpected timestamp. Expected: %u, got: %u, difference: %d, blocks_read_since_flush: %" PRIu64 ".", expected_timestamp, timestamp, timestamp_difference, blocks_read_since_flush); + expected_timestamp = timestamp; + // expected_timesamp_is_reasonable = 0; // must be validated each time by decoding the frame + // debug(1, "immediately: block %u, rtptime %u", seq_no, timestamp); // uint32_t ssrc = nctohl(&packet[8]); // uint8_t marker = 0; @@ -2852,8 +2884,12 @@ void *rtp_buffered_audio_processor(void *arg) { pcm_buffer_size); } else { memcpy(pcm_buffer + pcm_buffer_occupancy, pcm_audio, dst_bufsize); + expected_timestamp += (dst_bufsize/conn->input_bytes_per_frame); + // expected_timesamp_is_reasonable = 1; pcm_buffer_occupancy += dst_bufsize; - } + // debug(1,"frames added: pcm_buffer_read_point (frames): %u, pcm_buffer_occupancy (frames): %u", + // pcm_buffer_read_point/conn->input_bytes_per_frame, pcm_buffer_occupancy/conn->input_bytes_per_frame); + } // debug(1,"decoded %d samples", decoded_frame->nb_samples); // memcpy(sampleBuffer,outputBuffer16,dst_bufsize); av_freep(&pcm_audio); From 853544f1648e62538d3b891cf5ec47fa3724a1be Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Mon, 19 Sep 2022 09:51:39 +0100 Subject: [PATCH 03/19] Update InitialConfiguration.md --- ADVANCED TOPICS/InitialConfiguration.md | 117 +++++++++++++++--------- 1 file changed, 76 insertions(+), 41 deletions(-) diff --git a/ADVANCED TOPICS/InitialConfiguration.md b/ADVANCED TOPICS/InitialConfiguration.md index 1e99e5ca..07781ab8 100644 --- a/ADVANCED TOPICS/InitialConfiguration.md +++ b/ADVANCED TOPICS/InitialConfiguration.md @@ -1,6 +1,55 @@ -# Initial Configuration -Shairport Sync reads settings from a configuration file at `/etc/shairport-sync.conf` (note that in FreeBSD it will be at `/usr/local/etc/shairport-sync.conf`). When you run `$sudo make install`, a sample configuration file is installed or updated at `/etc/shairport-sync.conf.sample` (`/usr/local/etc/shairport-sync.conf.sample` in FreeBSD). This contains all the setting groups and all the settings available, but they all are commented out (comments begin with `//`) so that default values are used. The file contains explanations of the settings, useful hints and suggestions. In addition, if the file doesn't already exist, a default configuration is installed, which should work in almost any system with a sound card. +# Finish Setting Up +When you complete the instructions in [BUILD.md](../BUILD.md), you have a basic functioning Shairport Sync installation. If you want more control -- for example, if you want to use a specific DAC, or if you want AirPlay to control the DAC's volume control -- you can use settings in the configuration file (recommended) or you can use command-line options. +## The Configuration File +Shairport Sync reads settings from a configuration file at `/etc/shairport-sync.conf` (note that in FreeBSD it will be at `/usr/local/etc/shairport-sync.conf`). When you run `$sudo make install`, a sample configuration file called `shairport-sync.conf.sample` is always installed or updated. This contains all the setting groups and all the settings available, but they all are commented out (comments begin with `//`) so that default values are used. The file contains explanations of the settings, useful hints and suggestions. + +## Specifying the Output Device and Mixer Control +If you have followed the [BUILD.md](../BUILD.md) instructions, audio received by Shairport Sync will be sent to the `default` device. Depending on the configuration of your system, you may be able to specify a specific hardware output DAC and use its built-in mixer to control volume levels. This would be desirable because (1) the `default` device may be doing further processing on the audio before sending it to the hardware output device, degrading its fidelity, and (2) using the real device's hardware mixer to control volume would give Shairport Sync complete control of the volume range. + +To get a list of the hardware DACs on your system, refer to the output of `$ shairport-sync -h`. Here is a sample from a Raspberry Pi 3B system: +``` +Usage: shairport-sync [options...] + or: shairport-sync [options...] -- [audio output-specific options] +... +[snip] +... +Settings and options for the audio backend "alsa": + -d output-device set the output device, default is "default". + -c mixer-control set the mixer control name, default is to use no mixer. + -m mixer-device set the mixer device, default is the output device. + -i mixer-index set the mixer index, default is 0. + hardware output devices: + "hw:Headphones" + "hw:sndrpihifiberry" + "hw:vc4hdmi" +... +[snip] +... +``` +In this system, you can see that there are three hardware output devices, `"hw:Headphones"`, `"hw:sndrpihifiberry"` and `"hw:vc4hdmi"`. + +Using a tool like `alsamixer`, an output device can be checked to find the name of the volume control mixer. For the first device, the name of the mixer is `"Headphone"`. + +These setting can be entered into the configuration file, in the `alsa` section, as follows: +``` +... +alsa = +{ + output_device = "hw:Headphones"; // the name of the alsa output device. Use "shairport-sync -h" to discover the names of ALSA hardware devices. Use "alsamixer" or "aplay" to find out the names of devices, mixers, etc. + mixer_control_name = "Headphone"; // the name of the mixer to use to adjust output volume. If not specified, volume in adjusted in software. +... +} +``` +Make sure to uncomment entries you wish to make active, and restart Shairport Sync after you have made changes. + +If you make a syntax error, Shairport Sync will not start and will leave a message in the log. More details below and also in the comments in the configuration file. + +Please note that if your system has a sound server such as PulseAudio or PipeWire (most desktop linuxes have one of these), you may not be able to access the sound hardware directly, so you may only be able to use the `default` output. + + + +## More about Configuration Settings Settings in the configuration file are grouped. For instance, there is a `general` group within which you can use the `name` tag to set the service name. Suppose you wanted to set the name of the service to `Front Room` and give the service the password `secret`, then you should do the following: ``` @@ -18,11 +67,11 @@ The password setting is only valid for classic Shairport Sync. **Important:** You should *never* use an important password as the AirPlay password for a Shairport Sync player -- the password is stored in Shairport Sync's configuration file in plain text and is thus completely vulnerable. -No backend is specified here, so it will default to the `alsa` backend if more than one back end has been compiled. To route the output to PulseAudio, add: +No backend is specified here, so it will default to the `alsa` backend if more than one back end has been compiled. To route the output to PulseAudio, set: ``` output_backend = "pa"; ``` -to the `general` group. +in the `general` group. The `alsa` group is used to specify properties of the output device. The most obvious setting is the name of the output device which you can set using the `output_device` tag. @@ -44,23 +93,14 @@ The `pa` group is used to specify settings relevant to the PulseAudio backend. Y Note: Shairport Sync can take configuration settings from command line options. This is mainly for backward compatibility, but sometimes still useful. For normal use, it is strongly recommended that you use the configuration file method. -**Raspberry Pi** +### Raspberry Pi -The Raspberry Pi Models A and B have a built-in audio DAC that is connected to the device's headphone jack. Apart from a loud click when used for the first time after power-up, it is now quite adequate for casual listening. - -To get the benefits of improvements in the Pi's software and firmware, you should update to the Raspian release of October 2018 or later, as a number of improvements have been made to the built-in DAC. - -Do the usual update and upgrade: -``` -# apt update -# apt upgrade -``` To make Shairport Sync output to the built-in audio DAC and use its hardware mixer, in the `alsa` section of the configuration file, set the output device and mixer as follows: ``` alsa = { - output_device = "hw:0"; // the name of the alsa output device. Use "alsamixer" or "aplay" to find out the names of devices, mixers, etc. - mixer_control_name = "PCM"; // the name of the mixer to use to adjust output volume. If not specified, volume in adjusted in software. + output_device = "hw:Headphones"; // the name of the alsa output device. Use "alsamixer" or "aplay" to find out the names of devices, mixers, etc. + mixer_control_name = "Headphone"; // the name of the mixer to use to adjust output volume. If not specified, volume in adjusted in software. // ... other alsa settings ``` (Remember to uncomment the lines by removing the `//` at the start of each.) When these changes have been made, reboot the machine. @@ -69,22 +109,7 @@ A problem with the built-in DAC that it declares itself to have a very large mix From a user's point of view, the effect of using this setting is to move the minimum usable volume all the way down to the bottom of the user's volume control, rather than have the minimum usable volume concentrated very close to the maximum volume. -*Command Line Arguments* - -As previously mentioned, you can use command line arguments to provide settings to Shairport Sync as before, though newer settings will only be available via the configuration file. For full information, please read the Shairport Sync `man` page, also available at http://htmlpreview.github.io/?https://github.com/mikebrady/shairport-sync/blob/master/man/shairport-sync.html. - -Apart from the following options, all command line options can be replaced by settings in the configuration file. Here is a brief description of command line options that are not replicated by settings in the settings file. - -* The `-c` option allows you to specify the location of the configuration file. -* The `-V` option gives you version information about Shairport Sync and then quits. -* The `-d` option causes Shairport Sync to properly daemonise itself, that is, to run in the background. You may need sudo privileges for this. -* The `-k` option causes Shairport Sync to kill an existing Shairport Sync daemon. You may need to have sudo privileges for this. - -The System V init script at `/etc/init.d/shairport-sync` has a bare minimum : -`-d`. Basically all it does is put the program in daemon mode. The program will read its settings from the configuration file. - -Examples --------- +### Examples Here are some examples of complete configuration files. @@ -110,7 +135,7 @@ general = { alsa = { output_device = "hw:0"; - mixer_control_name = "PCM"; + mixer_control_name = "Headphone"; }; ``` @@ -150,8 +175,6 @@ alsa = { }; ``` -For an NSLU2, which has no internal sound card, there appears to be a bug in ALSA — you can not specify a device other than "default". Thus: - On an NSLU2, to drive a first generation Griffin iMic: ``` general = { @@ -184,22 +207,34 @@ general = { ``` -Latency -------- +### Latency Latency is the exact time from a sound signal's original timestamp until that signal actually "appears" on the output of the audio output device, usually a Digital to Audio Converter (DAC), irrespective of any internal delays, processing times, etc. in the computer. Shairport Sync uses latencies supplied by the source, typically either 2 seconds or just over 2.25 seconds. You shouldn't need to change them. Problems can arise when you are trying to synchronise with speaker systems — typically surround-sound home theatre systems — that have their own inherent delays. You can compensate for an inherent delay using the appropriate backend (typically `alsa`) `audio_backend_latency_offset_in_seconds`. Set this offset (in frames) to compensate for a fixed delay in the audio back end; for example, if the output device delays by 100 ms, set this to -0.1. -Resynchronisation -------------- +### Resynchronisation + Shairport Sync actively maintains synchronisation with the source. If synchronisation is lost — say due to a busy source or a congested network — Shairport Sync will mute its output and resynchronise. The loss-of-sync threshold is a very conservative 0.050 seconds — i.e. the actual time and the expected time must differ by more than 50 ms to trigger a resynchronisation. Smaller disparities are corrected by insertions or deletions, as described above. * You can vary the resync threshold, or turn resync off completely, with the `general` `resync_threshold_in_seconds` setting. -Tolerance ---------- +### Tolerance Playback synchronisation is allowed to wander — to "drift" — a small amount before attempting to correct it. The default is 0.002 seconds, i.e. 2 ms. The smaller the tolerance, the more likely it is that overcorrection will occur. Overcorrection is when more corrections (insertions and deletions) are made than are strictly necessary to keep the stream in sync. Use the `statistics` setting to monitor correction levels. Corrections should not greatly exceed net corrections. * You can vary the tolerance with the `general` `drift_tolerance_in_seconds` setting. +## Command Line Arguments + +You can use command line arguments to provide settings to Shairport Sync, though newer settings will only be available via the configuration file. For full information, please read the Shairport Sync `man` page, also available at http://htmlpreview.github.io/?https://github.com/mikebrady/shairport-sync/blob/master/man/shairport-sync.html. + +Apart from the following options, all command line options can be replaced by settings in the configuration file. Here is a brief description of command line options that are not replicated by settings in the settings file. + +* The `-c` option allows you to specify the location of the configuration file. +* The `-V` option gives you version information about Shairport Sync and then quits. +* The `-d` option causes Shairport Sync to properly daemonise itself, that is, to run in the background. You may need sudo privileges for this. +* The `-k` option causes Shairport Sync to kill an existing Shairport Sync daemon. You may need to have sudo privileges for this. + +The System V init script at `/etc/init.d/shairport-sync` has a bare minimum : +`-d`. Basically all it does is put the program in daemon mode. The program will read its settings from the configuration file. + From a171f1b6b411adc22adcbabe66a1f61e27a6c7b8 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Mon, 19 Sep 2022 10:00:12 +0100 Subject: [PATCH 04/19] Update BUILD.md --- BUILD.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/BUILD.md b/BUILD.md index 9525ee27..67950b91 100644 --- a/BUILD.md +++ b/BUILD.md @@ -192,4 +192,8 @@ If a firewall is running (some systems, e.g. Fedora, run a firewall by default), ### Add to Home With AirPlay 2, you can follow the steps in [ADDINGTOHOME.md](https://github.com/mikebrady/shairport-sync/blob/development/ADDINGTOHOME.md) to add your device to the Apple Home system. ### Wait, there's more... -Shairport Sync has many extra [features](README.md#features) and [settings](https://github.com/mikebrady/shairport-sync/blob/development/scripts/shairport-sync.conf). For instance, you can [add an MQTT service](https://github.com/mikebrady/shairport-sync/blob/master/MQTT.md), change the AirPlay service name, adjust for delays in other equipment (many TVs and AVRs introduce slight delays in their audio processing chains) and more. Explore runtime settings in `/etc/shairport-sync.conf` or [here](https://github.com/mikebrady/shairport-sync/blob/development/scripts/shairport-sync.conf). Build configuration flags are [here](https://github.com/mikebrady/shairport-sync/blob/development/CONFIGURATION%20FLAGS.md). +Instead of using default values for everything, you can use the configuration file to get finer control over the setup, particularly the output device and mixer control -- see [Finish Setting Up](ADVANCED%20TOPICS/InitialConfiguration.md). + +Shairport Sync has many extra [features](README.md#features) and [settings](scripts/shairport-sync.conf). Build configuration flags are [here](CONFIGURATION%20FLAGS.md). + +Please take a look at [Advanced Topics](ADVANCED%20TOPICS/README.md) for some ideas about what else you can do to improve the operation of Shairport Sync. From 0423dde9dfa56e1f75fdc6b834c43de48feef4ac Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Mon, 19 Sep 2022 10:01:26 +0100 Subject: [PATCH 05/19] Update BUILD.md --- BUILD.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/BUILD.md b/BUILD.md index 67950b91..3caef3ce 100644 --- a/BUILD.md +++ b/BUILD.md @@ -194,6 +194,4 @@ With AirPlay 2, you can follow the steps in [ADDINGTOHOME.md](https://github.com ### Wait, there's more... Instead of using default values for everything, you can use the configuration file to get finer control over the setup, particularly the output device and mixer control -- see [Finish Setting Up](ADVANCED%20TOPICS/InitialConfiguration.md). -Shairport Sync has many extra [features](README.md#features) and [settings](scripts/shairport-sync.conf). Build configuration flags are [here](CONFIGURATION%20FLAGS.md). - -Please take a look at [Advanced Topics](ADVANCED%20TOPICS/README.md) for some ideas about what else you can do to improve the operation of Shairport Sync. +Please take a look at [Advanced Topics](ADVANCED%20TOPICS/README.md) for some ideas about what else you can do to enhance the operation of Shairport Sync. From 9f891b2d1e833a02b1d8cc924075edf1c85aca27 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Mon, 19 Sep 2022 10:03:42 +0100 Subject: [PATCH 06/19] Update BUILD.md --- BUILD.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/BUILD.md b/BUILD.md index 3caef3ce..36953b24 100644 --- a/BUILD.md +++ b/BUILD.md @@ -149,7 +149,7 @@ $ shairport-sync The AirPlay service should appear on the network and the audio you play should come through to the default ALSA device. (Use `alsamixer` or similar to adjust levels.) -If you have problems, please check the items in [Final Notes](final_notes) below, or in the [TROUBLESHOOTING.md](https://github.com/mikebrady/shairport-sync/blob/development/TROUBLESHOOTING.md) guide. +If you have problems, please check the items in [Final Notes](7._final_notes) below, or in the [TROUBLESHOOTING.md](https://github.com/mikebrady/shairport-sync/blob/development/TROUBLESHOOTING.md) guide. Note: Shairport Sync will run indefinitely -- use Control-C it to stop it. From 40654624fb893d8af4d62a58cc8eb92c6d49c0e3 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Mon, 19 Sep 2022 10:04:41 +0100 Subject: [PATCH 07/19] Update BUILD.md --- BUILD.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/BUILD.md b/BUILD.md index 36953b24..8d8e9352 100644 --- a/BUILD.md +++ b/BUILD.md @@ -149,7 +149,7 @@ $ shairport-sync The AirPlay service should appear on the network and the audio you play should come through to the default ALSA device. (Use `alsamixer` or similar to adjust levels.) -If you have problems, please check the items in [Final Notes](7._final_notes) below, or in the [TROUBLESHOOTING.md](https://github.com/mikebrady/shairport-sync/blob/development/TROUBLESHOOTING.md) guide. +If you have problems, please check the items in Final Notes below, or in the [TROUBLESHOOTING.md](TROUBLESHOOTING.md) guide. Note: Shairport Sync will run indefinitely -- use Control-C it to stop it. From a6e02449fb3e2e44465a76eedadd9cd0593c698f Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Thu, 22 Sep 2022 16:25:57 +0100 Subject: [PATCH 08/19] Increase the RTSP idle timeout to ten seconds, and confine its operation to realtime and classic AirPlay only.: --- rtsp.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/rtsp.c b/rtsp.c index 8188425e..370d083d 100644 --- a/rtsp.c +++ b/rtsp.c @@ -1275,14 +1275,11 @@ void clear_ptp_clock() { ssize_t read_from_rtsp_connection(rtsp_conn_info *conn, void *buf, size_t count) { // first try to read with a timeout, to see if there is any traffic... - ssize_t response = timed_read_from_rtsp_connection(conn, 4000000000L, buf, count); + ssize_t response = timed_read_from_rtsp_connection(conn, 10000000000L, buf, count); if ((response == -1) && ((errno == EAGAIN) || (errno == EWOULDBLOCK))) { if (conn->rtsp_link_is_idle == 0) { debug(1, "Connection %d: RTSP connection is idle.", conn->connection_number); conn->rtsp_link_is_idle = 1; -#ifdef CONFIG_AIRPLAY_2 - clear_ptp_clock(); -#endif conn->udp_clock_sender_is_initialised = 0; conn->udp_clock_is_initialised = 0; } @@ -1291,15 +1288,22 @@ ssize_t read_from_rtsp_connection(rtsp_conn_info *conn, void *buf, size_t count) if (conn->rtsp_link_is_idle == 1) { conn->rtsp_link_is_idle = 0; debug(1, "Connection %d: RTSP connection traffic has resumed.", conn->connection_number); +#ifdef CONFIG_AIRPLAY_2 + if (conn->airplay_stream_type == realtime_stream) { + conn->last_anchor_info_is_valid = 0; + conn->anchor_remote_info_is_valid = 0; + conn->first_packet_timestamp = 0; + conn->input_frame_rate_starting_point_is_valid = 0; + ab_resync(conn); + } +#else + conn->last_anchor_info_is_valid = 0; conn->anchor_remote_info_is_valid = 0; conn->local_to_remote_time_difference_measurement_time = 0; conn->local_to_remote_time_difference = 0; conn->first_packet_timestamp = 0; conn->input_frame_rate_starting_point_is_valid = 0; ab_resync(conn); -#ifdef CONFIG_AIRPLAY_2 - conn->last_anchor_info_is_valid = 0; - set_client_as_ptp_clock(conn); #endif } return response; From b19dd9e93c88d92a808e927128c6a6069f1b16fb Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Thu, 22 Sep 2022 16:28:51 +0100 Subject: [PATCH 09/19] Skip the out-of-date frames at the start of a session in AirPLay 2 Buffered Streams in iOS 16.0 / Ventura beta / iOS 16.1 beta. Only skip two blocks to avoid AAC transients and substitute silent for them. --- rtp.c | 60 ++++++++++++++++++++++++++++++++++------------------------- 1 file changed, 35 insertions(+), 25 deletions(-) diff --git a/rtp.c b/rtp.c index b4b6ad0f..567b5b9c 100644 --- a/rtp.c +++ b/rtp.c @@ -1328,6 +1328,7 @@ void reset_ptp_anchor_info(rtsp_conn_info *conn) { } int long_time_notifcation_done = 0; + int get_ptp_anchor_local_time_info(rtsp_conn_info *conn, uint32_t *anchorRTP, uint64_t *anchorLocalTime) { int response = clock_not_valid; @@ -2374,13 +2375,13 @@ void *rtp_buffered_audio_processor(void *arg) { int pcm_buffer_occupancy = 0; int pcm_buffer_read_point = 0; // offset to where the next buffer should come from uint32_t pcm_buffer_read_point_rtptime = 0; - // uint32_t expected_pcm_buffer_read_point_rtptime = 0; + uint32_t expected_pcm_buffer_read_point_rtptime = 0; uint64_t blocks_read = 0; uint64_t blocks_read_since_flush = 0; int flush_requested = 0; uint32_t expected_timestamp = 0; - // int expected_timesamp_is_reasonable = 0; + int expected_timesamp_is_reasonable = 0; uint32_t timestamp = 0; // initialised to avoid a "possibly uninitialised" warning. int streaming_has_started = 0; int play_enabled = 0; @@ -2569,28 +2570,37 @@ void *rtp_buffered_audio_processor(void *arg) { // debug(1,"block timestamp: %u, packet timestamp: %u.", timestamp, pcm_buffer_read_point_rtptime); -/* + int32_t timestamp_difference = pcm_buffer_read_point_rtptime - expected_pcm_buffer_read_point_rtptime;; if (streaming_has_started != 0) { - int32_t timestamp_difference = pcm_buffer_read_point_rtptime - expected_pcm_buffer_read_point_rtptime; if (timestamp_difference != 0) - debug(1,"Unexpected time difference between packets -- actual: %u, expected: %u, difference: %d. Packets played: %d. Blocks played since flush: %d. ", - pcm_buffer_read_point_rtptime, expected_pcm_buffer_read_point_rtptime, timestamp_difference, streaming_has_started, blocks_read_since_flush); - } -*/ - - // if it's not the very first block of AAC, but is from the first few blocks of a new AAC sequence, - // it will contain noisy transients, so replace it with silence. - if ((blocks_read != 1) && (blocks_read_since_flush <= 3)) { - // debug(1,"muting packet %u from block %u because it's not from a true starting block and blocks_read_since_flush is %" PRIu64 ".", pcm_buffer_read_point_rtptime, timestamp, blocks_read_since_flush); - conn->previous_random_number = - generate_zero_frames((char *)(pcm_buffer + pcm_buffer_read_point), 352, config.output_format, conn->enable_dither, - conn->previous_random_number); + if (streaming_has_started != 64) // don't log this situation because it's being dealt with below + debug(1,"Unexpected time difference between packets -- actual: %u, expected: %u, difference: %d. Packets played: %d. Blocks played since flush: %d. ", + pcm_buffer_read_point_rtptime, expected_pcm_buffer_read_point_rtptime, timestamp_difference, streaming_has_started, blocks_read_since_flush); } - player_put_packet(0, 0, pcm_buffer_read_point_rtptime, - pcm_buffer + pcm_buffer_read_point, 352, conn); - streaming_has_started++; - // expected_pcm_buffer_read_point_rtptime = pcm_buffer_read_point_rtptime + 352; + // Very specific code to get around an apparent bug in AirPlay 2 from iOS 16 / Ventura 13.0 + // It seems that the timestamp goes backwards by 2112 frames after the first 64 packets of 352 frames (64 * 352 = 22528 frames which is exactly 22 blocks) + // So, if the timestamp is before the expected timestamp when 64 packets have been played, then skip that packet. + + // Put an arbitrary last-ditch limit of 64 blocks_read_since_flush in case it all goes wrong... + + if ((timestamp_difference < 0) && (streaming_has_started == 64) && (blocks_read_since_flush < 64)) { + // debug(1,"skipping packet %u.", pcm_buffer_read_point_rtptime); + } else { + // if it's not the very first block of AAC, but is from the first few blocks of a new AAC sequence, + // it will contain noisy transients, so replace it with silence. + if ((blocks_read_since_flush <= 2) && (blocks_read_since_flush != blocks_read)) { + // debug(1,"Muting packet %u from block %u to avoid AAC transients because it's not from a true starting block. Blocks_read is %" PRIu64 ". Blocks_read_since_flush is %" PRIu64 ".", pcm_buffer_read_point_rtptime, timestamp, blocks_read, blocks_read_since_flush); + conn->previous_random_number = + generate_zero_frames((char *)(pcm_buffer + pcm_buffer_read_point), 352, config.output_format, conn->enable_dither, + conn->previous_random_number); + } + + player_put_packet(0, 0, pcm_buffer_read_point_rtptime, + pcm_buffer + pcm_buffer_read_point, 352, conn); + streaming_has_started++; + expected_pcm_buffer_read_point_rtptime = pcm_buffer_read_point_rtptime + 352; + } } // } } else { @@ -2680,11 +2690,11 @@ void *rtp_buffered_audio_processor(void *arg) { seq_no = packet[1] * (1 << 16) + packet[2] * (1 << 8) + packet[3]; timestamp = nctohl(&packet[4]); // debug(1,"New block timestamp: %u.", timestamp); - // int32_t timestamp_difference = timestamp - expected_timestamp; - // if ((timestamp_difference != 0) && (expected_timesamp_is_reasonable != 0)) - // debug(1, "Unexpected timestamp. Expected: %u, got: %u, difference: %d, blocks_read_since_flush: %" PRIu64 ".", expected_timestamp, timestamp, timestamp_difference, blocks_read_since_flush); + int32_t timestamp_difference = timestamp - expected_timestamp; + if ((timestamp_difference != 0) && (expected_timesamp_is_reasonable != 0)) + debug(1, "Block with unexpected timestamp. Expected: %u, got: %u, difference: %d, blocks_read_since_flush: %" PRIu64 ".", expected_timestamp, timestamp, timestamp_difference, blocks_read_since_flush); expected_timestamp = timestamp; - // expected_timesamp_is_reasonable = 0; // must be validated each time by decoding the frame + expected_timesamp_is_reasonable = 0; // must be validated each time by decoding the frame // debug(1, "immediately: block %u, rtptime %u", seq_no, timestamp); // uint32_t ssrc = nctohl(&packet[8]); @@ -2885,7 +2895,7 @@ void *rtp_buffered_audio_processor(void *arg) { } else { memcpy(pcm_buffer + pcm_buffer_occupancy, pcm_audio, dst_bufsize); expected_timestamp += (dst_bufsize/conn->input_bytes_per_frame); - // expected_timesamp_is_reasonable = 1; + expected_timesamp_is_reasonable = 1; pcm_buffer_occupancy += dst_bufsize; // debug(1,"frames added: pcm_buffer_read_point (frames): %u, pcm_buffer_occupancy (frames): %u", // pcm_buffer_read_point/conn->input_bytes_per_frame, pcm_buffer_occupancy/conn->input_bytes_per_frame); From 0a5b7113272269d1f27ab6cf9347c08064aececd Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Thu, 22 Sep 2022 17:03:53 +0100 Subject: [PATCH 10/19] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index e0ac0d8f..b0d8dbb5 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,12 @@ +Version 4.1-dev-543-g24f06b81 +==== +**Enhancements** +* Add a workaround for an apparent bug in AirPlay 2 Buffered Streams in iOS 16.0. After playing exactly 22,528 frames of audio, iOS 16.0 sends 2,112 frames of audio with the same timestamps as the previous 2,112 frames. This makes the frames that follow 47.9 ms (2112/44100 seconds) late. The workaround is to drop these extra 2,112 frames. +* If an AirPlay 2 Buffered Streams is being skipped or scrubbed, the audio that follows will generate an audible click due to an AAC decoding transient because preceding audio frames are missing. Shairport Sync now mutes the first 2,048 frames, down from 3,072 frames before. + +**Bug Fix** +* Give the RTSP idle checker a longer timeout -- 10 seconds -- and confine its operation to AirPlay 2 Runtime Streams and classic AirPlay only. This is to stop an occasional RTSP idle timeout silencing an AirPlay 2 Buffered Audio session. + Version 4.1-dev-532-g8dfebea2 ==== **Enhancements** From a980192a00abae009db3eef3423fcd0d467f0e01 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Thu, 22 Sep 2022 17:33:23 +0100 Subject: [PATCH 11/19] Fix a silly classic AirPlay build issue. --- rtsp.c | 1 - 1 file changed, 1 deletion(-) diff --git a/rtsp.c b/rtsp.c index 370d083d..d61ffeb1 100644 --- a/rtsp.c +++ b/rtsp.c @@ -1297,7 +1297,6 @@ ssize_t read_from_rtsp_connection(rtsp_conn_info *conn, void *buf, size_t count) ab_resync(conn); } #else - conn->last_anchor_info_is_valid = 0; conn->anchor_remote_info_is_valid = 0; conn->local_to_remote_time_difference_measurement_time = 0; conn->local_to_remote_time_difference = 0; From c6bb33dcaf65b041ca367dd6b5a7329496a00c1c Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Thu, 22 Sep 2022 18:19:46 +0100 Subject: [PATCH 12/19] Roll back fix for extra frames -- it fixes sync issues but introduces transients or glitches. Not nice. --- rtp.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/rtp.c b/rtp.c index 567b5b9c..55a70680 100644 --- a/rtp.c +++ b/rtp.c @@ -2584,9 +2584,13 @@ void *rtp_buffered_audio_processor(void *arg) { // Put an arbitrary last-ditch limit of 64 blocks_read_since_flush in case it all goes wrong... - if ((timestamp_difference < 0) && (streaming_has_started == 64) && (blocks_read_since_flush < 64)) { + //if ((timestamp_difference < 0) && (streaming_has_started == 64) && (blocks_read_since_flush < 64)) { // debug(1,"skipping packet %u.", pcm_buffer_read_point_rtptime); - } else { + //} else { + // if (streaming_has_started >= 59) && (streaming_has_started <= 64) { + // streaming_has_started++; + //} else { + { // if it's not the very first block of AAC, but is from the first few blocks of a new AAC sequence, // it will contain noisy transients, so replace it with silence. if ((blocks_read_since_flush <= 2) && (blocks_read_since_flush != blocks_read)) { From c433e2e87d532148e08760fa031e6498ed19b08e Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Fri, 23 Sep 2022 17:02:26 +0100 Subject: [PATCH 13/19] Add in code to detect the iOS 16 discontinuity but not to do anything about it. Delaying everything by 2112 frames after the discontinuity removes it, but the result seems to be that playback is delayed by 2112 frames. One approach would be to advance everything prior to the discontinuity by 2112 frames, but that would require checking what version of software the source was using, so hardly worth the trouble. --- rtp.c | 114 +++++++++++++++++++++++++++++++++++----------------------- 1 file changed, 69 insertions(+), 45 deletions(-) diff --git a/rtp.c b/rtp.c index 55a70680..e01c8730 100644 --- a/rtp.c +++ b/rtp.c @@ -2364,7 +2364,7 @@ void *rtp_buffered_audio_processor(void *arg) { unsigned char *data_to_process; ssize_t data_remaining; uint32_t seq_no = 0; // audio packet number. Initialised to avoid a "possibly uninitialised" warning. - // uint32_t previous_seq_no = 0; + uint32_t previous_seq_no = 0; int new_buffer_needed = 0; ssize_t nread; @@ -2375,15 +2375,16 @@ void *rtp_buffered_audio_processor(void *arg) { int pcm_buffer_occupancy = 0; int pcm_buffer_read_point = 0; // offset to where the next buffer should come from uint32_t pcm_buffer_read_point_rtptime = 0; + uint32_t pcm_buffer_read_point_rtptime_offset = 0; // hack uint32_t expected_pcm_buffer_read_point_rtptime = 0; uint64_t blocks_read = 0; - uint64_t blocks_read_since_flush = 0; + uint64_t blocks_read_in_sequence = 0; // since the start of this sequence -- reset by start or flush int flush_requested = 0; uint32_t expected_timestamp = 0; int expected_timesamp_is_reasonable = 0; uint32_t timestamp = 0; // initialised to avoid a "possibly uninitialised" warning. - int streaming_has_started = 0; + int packets_played_in_this_sequence = 0; int play_enabled = 0; uint32_t flush_from_timestamp = 0; // initialised to avoid a "possibly uninitialised" warning. double requested_lead_time = 0.0; // normal lead time minimum -- maybe it should be about 0.1 @@ -2464,15 +2465,35 @@ void *rtp_buffered_audio_processor(void *arg) { flush_newly_requested = 0; } } + + // flush_requested = conn->ap2_flush_requested; + if ((play_enabled) && (conn->ap2_play_enabled == 0)) { + play_newly_stopped = 1; + debug(2,"Play stopped."); + pcm_buffer_read_point_rtptime_offset = 0; + blocks_read_in_sequence = 0; // This may be set to 1 by a flush, so don't zero it during start. + packets_played_in_this_sequence = 0; + pcm_buffer_occupancy = 0; + pcm_buffer_read_point = 0; + } + + if ((play_enabled == 0) && (conn->ap2_play_enabled != 0)) { + // play newly started + debug(2,"Play started."); + } + + + if ((flush_requested) && (flush_request_active == 0)) { + if (play_enabled) + debug(1,"Flush completed while play_enabled is true."); flush_newly_complete = 1; - blocks_read_since_flush = 1; // the last block always (?) becomes the first block after the flush + blocks_read_in_sequence = 1; // the last block always (?) becomes the first block after the flush } flush_requested = flush_request_active; - // flush_requested = conn->ap2_flush_requested; - if ((play_enabled) && (conn->ap2_play_enabled == 0)) - play_newly_stopped = 1; + play_enabled = conn->ap2_play_enabled; + debug_mutex_unlock(&conn->flush_mutex, 3); // do this outside the flush mutex @@ -2489,15 +2510,16 @@ void *rtp_buffered_audio_processor(void *arg) { if (flush_is_delayed == 0) { debug(2, "Immediate Buffered Audio Flush Started."); // player_full_flush(conn); - streaming_has_started = 0; + packets_played_in_this_sequence = 0; pcm_buffer_occupancy = 0; pcm_buffer_read_point = 0; } else { debug(2, "Delayed Buffered Audio Flush Started."); - streaming_has_started = 0; + packets_played_in_this_sequence = 0; pcm_buffer_occupancy = 0; pcm_buffer_read_point = 0; } + pcm_buffer_read_point_rtptime_offset = 0; } // now, if a flush is not requested, we can do the normal stuff @@ -2535,10 +2557,10 @@ void *rtp_buffered_audio_processor(void *arg) { // only accept them if they have sensible lead times if ((lead_time < (int64_t)30000000000L) && (lead_time >= 0)) { // if it's the very first block (thus no priming needed) - //if ((blocks_read == 1) || (blocks_read_since_flush > 3)) { + //if ((blocks_read == 1) || (blocks_read_in_sequence > 3)) { if ((lead_time >= (int64_t)(requested_lead_time * 1000000000L)) || - (streaming_has_started != 0)) { - if (streaming_has_started == 0) + (packets_played_in_this_sequence != 0)) { + if (packets_played_in_this_sequence == 0) debug(2, "Connection %d: buffered audio starting frame: %u, lead time: %f " "seconds.", @@ -2571,46 +2593,43 @@ void *rtp_buffered_audio_processor(void *arg) { // debug(1,"block timestamp: %u, packet timestamp: %u.", timestamp, pcm_buffer_read_point_rtptime); int32_t timestamp_difference = pcm_buffer_read_point_rtptime - expected_pcm_buffer_read_point_rtptime;; - if (streaming_has_started != 0) { + if (packets_played_in_this_sequence != 0) { if (timestamp_difference != 0) - if (streaming_has_started != 64) // don't log this situation because it's being dealt with below - debug(1,"Unexpected time difference between packets -- actual: %u, expected: %u, difference: %d. Packets played: %d. Blocks played since flush: %d. ", - pcm_buffer_read_point_rtptime, expected_pcm_buffer_read_point_rtptime, timestamp_difference, streaming_has_started, blocks_read_since_flush); + debug(2,"Unexpected time difference between packets -- actual: %u, expected: %u, difference: %d. Packets played: %d. Blocks played since flush: %d. ", + pcm_buffer_read_point_rtptime, expected_pcm_buffer_read_point_rtptime, timestamp_difference, packets_played_in_this_sequence, blocks_read_in_sequence); } + // Very specific code to get around an apparent bug in AirPlay 2 from iOS 16 / Ventura 13.0 - // It seems that the timestamp goes backwards by 2112 frames after the first 64 packets of 352 frames (64 * 352 = 22528 frames which is exactly 22 blocks) - // So, if the timestamp is before the expected timestamp when 64 packets have been played, then skip that packet. - - // Put an arbitrary last-ditch limit of 64 blocks_read_since_flush in case it all goes wrong... + // It seems that the timestamp goes backwards by 2112 frames not later than the 65th packet of 352 frames (64 * 352 = 22528 frames which is exactly 22 blocks) + // So, if that happens, we'll add 2112 to the timstamp passed to the player - //if ((timestamp_difference < 0) && (streaming_has_started == 64) && (blocks_read_since_flush < 64)) { - // debug(1,"skipping packet %u.", pcm_buffer_read_point_rtptime); - //} else { - // if (streaming_has_started >= 59) && (streaming_has_started <= 64) { - // streaming_has_started++; - //} else { - { - // if it's not the very first block of AAC, but is from the first few blocks of a new AAC sequence, - // it will contain noisy transients, so replace it with silence. - if ((blocks_read_since_flush <= 2) && (blocks_read_since_flush != blocks_read)) { - // debug(1,"Muting packet %u from block %u to avoid AAC transients because it's not from a true starting block. Blocks_read is %" PRIu64 ". Blocks_read_since_flush is %" PRIu64 ".", pcm_buffer_read_point_rtptime, timestamp, blocks_read, blocks_read_since_flush); - conn->previous_random_number = - generate_zero_frames((char *)(pcm_buffer + pcm_buffer_read_point), 352, config.output_format, conn->enable_dither, - conn->previous_random_number); - } - - player_put_packet(0, 0, pcm_buffer_read_point_rtptime, - pcm_buffer + pcm_buffer_read_point, 352, conn); - streaming_has_started++; - expected_pcm_buffer_read_point_rtptime = pcm_buffer_read_point_rtptime + 352; + if ((timestamp_difference == -2112) && (packets_played_in_this_sequence <= 64)) { + debug(1,"iOS 16.0 discontinuity detected with %d packets played in this sequence. Nothing done.", packets_played_in_this_sequence); + // pcm_buffer_read_point_rtptime_offset = 2112; // this pretends the timestamps after the discontinuity are 2112 frames later, but this just delays everything by 2112 frames, pushing stuff out of sync, and i think you can hear it. } + + + // if it's not the very first block of AAC, but is from the first few blocks of a new AAC sequence, + // it will contain noisy transients, so replace it with silence. + if ((blocks_read_in_sequence <= 2) && (blocks_read_in_sequence != blocks_read)) { + // debug(1,"Muting packet %u from block %u to avoid AAC transients because it's not from a true starting block. Blocks_read is %" PRIu64 ". blocks_read_in_sequence is %" PRIu64 ".", pcm_buffer_read_point_rtptime, timestamp, blocks_read, blocks_read_in_sequence); + conn->previous_random_number = + generate_zero_frames((char *)(pcm_buffer + pcm_buffer_read_point), 352, config.output_format, conn->enable_dither, + conn->previous_random_number); + } + + player_put_packet(0, 0, pcm_buffer_read_point_rtptime + pcm_buffer_read_point_rtptime_offset, + pcm_buffer + pcm_buffer_read_point, 352, conn); + packets_played_in_this_sequence++; + expected_pcm_buffer_read_point_rtptime = pcm_buffer_read_point_rtptime + 352; } // } } else { - debug(1, + debug(3, "Dropping packet %u from block %u with out-of-range lead_time: %.3f seconds.", pcm_buffer_read_point_rtptime, seq_no, 0.000000001 * lead_time); + expected_pcm_buffer_read_point_rtptime = pcm_buffer_read_point_rtptime + 352; } pcm_buffer_read_point_rtptime += 352; @@ -2682,7 +2701,7 @@ void *rtp_buffered_audio_processor(void *arg) { errno, errorstring); } else if (nread > 0) { blocks_read++; // note, this doesn't mean they are valid audio blocks - blocks_read_since_flush++; + blocks_read_in_sequence++; // debug(1, "Realtime Audio Receiver Packet of length %d received.", nread); // now get hold of its various bits and pieces /* @@ -2691,12 +2710,17 @@ void *rtp_buffered_audio_processor(void *arg) { uint8_t extension = (packet[0] & 0b00010000) >> 4; uint8_t csrc_count = packet[0] & 0b00001111; */ + previous_seq_no = seq_no; + previous_seq_no++; seq_no = packet[1] * (1 << 16) + packet[2] * (1 << 8) + packet[3]; + if (previous_seq_no != seq_no) { + debug(2,"block sequence number changed from expected %u to actual %u.", previous_seq_no, seq_no); + } timestamp = nctohl(&packet[4]); // debug(1,"New block timestamp: %u.", timestamp); int32_t timestamp_difference = timestamp - expected_timestamp; if ((timestamp_difference != 0) && (expected_timesamp_is_reasonable != 0)) - debug(1, "Block with unexpected timestamp. Expected: %u, got: %u, difference: %d, blocks_read_since_flush: %" PRIu64 ".", expected_timestamp, timestamp, timestamp_difference, blocks_read_since_flush); + debug(2, "Block with unexpected timestamp. Expected: %u, got: %u, difference: %d, blocks_read_in_sequence: %" PRIu64 ".", expected_timestamp, timestamp, timestamp_difference, blocks_read_in_sequence); expected_timestamp = timestamp; expected_timesamp_is_reasonable = 0; // must be validated each time by decoding the frame @@ -2848,8 +2872,8 @@ void *rtp_buffered_audio_processor(void *arg) { if (ret < 0) { debug(1, "error sending frame %d of size %d to decoder, blocks_read: %u, " - "blocks_read_since_flush: %u.", - frame_within_block, pkt->size, blocks_read, blocks_read_since_flush); + "blocks_read_in_sequence: %u.", + frame_within_block, pkt->size, blocks_read, blocks_read_in_sequence); } else { while (ret >= 0) { ret = avcodec_receive_frame(codec_context, decoded_frame); From 7dc077e2603563f5eae12ec1f7bcdfa0f698b779 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Fri, 23 Sep 2022 17:05:26 +0100 Subject: [PATCH 14/19] Clang format --- rtp.c | 121 +++++++++++++++++++++++++++++++++++----------------------- 1 file changed, 74 insertions(+), 47 deletions(-) diff --git a/rtp.c b/rtp.c index e01c8730..7b53df74 100644 --- a/rtp.c +++ b/rtp.c @@ -2588,48 +2588,66 @@ void *rtp_buffered_audio_processor(void *arg) { not_first_time_out = 1; } */ - // clang-format on + // clang-format on - // debug(1,"block timestamp: %u, packet timestamp: %u.", timestamp, pcm_buffer_read_point_rtptime); + // debug(1,"block timestamp: %u, packet timestamp: %u.", timestamp, + // pcm_buffer_read_point_rtptime); - int32_t timestamp_difference = pcm_buffer_read_point_rtptime - expected_pcm_buffer_read_point_rtptime;; - if (packets_played_in_this_sequence != 0) { - if (timestamp_difference != 0) - debug(2,"Unexpected time difference between packets -- actual: %u, expected: %u, difference: %d. Packets played: %d. Blocks played since flush: %d. ", - pcm_buffer_read_point_rtptime, expected_pcm_buffer_read_point_rtptime, timestamp_difference, packets_played_in_this_sequence, blocks_read_in_sequence); - } - - - // Very specific code to get around an apparent bug in AirPlay 2 from iOS 16 / Ventura 13.0 - // It seems that the timestamp goes backwards by 2112 frames not later than the 65th packet of 352 frames (64 * 352 = 22528 frames which is exactly 22 blocks) - // So, if that happens, we'll add 2112 to the timstamp passed to the player - - if ((timestamp_difference == -2112) && (packets_played_in_this_sequence <= 64)) { - debug(1,"iOS 16.0 discontinuity detected with %d packets played in this sequence. Nothing done.", packets_played_in_this_sequence); - // pcm_buffer_read_point_rtptime_offset = 2112; // this pretends the timestamps after the discontinuity are 2112 frames later, but this just delays everything by 2112 frames, pushing stuff out of sync, and i think you can hear it. - } - - - // if it's not the very first block of AAC, but is from the first few blocks of a new AAC sequence, - // it will contain noisy transients, so replace it with silence. - if ((blocks_read_in_sequence <= 2) && (blocks_read_in_sequence != blocks_read)) { - // debug(1,"Muting packet %u from block %u to avoid AAC transients because it's not from a true starting block. Blocks_read is %" PRIu64 ". blocks_read_in_sequence is %" PRIu64 ".", pcm_buffer_read_point_rtptime, timestamp, blocks_read, blocks_read_in_sequence); - conn->previous_random_number = - generate_zero_frames((char *)(pcm_buffer + pcm_buffer_read_point), 352, config.output_format, conn->enable_dither, - conn->previous_random_number); - } - - player_put_packet(0, 0, pcm_buffer_read_point_rtptime + pcm_buffer_read_point_rtptime_offset, - pcm_buffer + pcm_buffer_read_point, 352, conn); - packets_played_in_this_sequence++; - expected_pcm_buffer_read_point_rtptime = pcm_buffer_read_point_rtptime + 352; + int32_t timestamp_difference = + pcm_buffer_read_point_rtptime - expected_pcm_buffer_read_point_rtptime; + ; + if (packets_played_in_this_sequence != 0) { + if (timestamp_difference != 0) + debug( + 2, + "Unexpected time difference between packets -- actual: %u, expected: %u, " + "difference: %d. Packets played: %d. Blocks played since flush: %d. ", + pcm_buffer_read_point_rtptime, expected_pcm_buffer_read_point_rtptime, + timestamp_difference, packets_played_in_this_sequence, + blocks_read_in_sequence); } + + // Very specific code to get around an apparent bug in AirPlay 2 from iOS 16 / + // Ventura 13.0 It seems that the timestamp goes backwards by 2112 frames not + // later than the 65th packet of 352 frames (64 * 352 = 22528 frames which is + // exactly 22 blocks) So, if that happens, we'll add 2112 to the timstamp passed + // to the player + + if ((timestamp_difference == -2112) && (packets_played_in_this_sequence <= 64)) { + debug(1, + "iOS 16.0 discontinuity detected with %d packets played in this " + "sequence. Nothing done.", + packets_played_in_this_sequence); + // pcm_buffer_read_point_rtptime_offset = 2112; // this pretends the timestamps + // after the discontinuity are 2112 frames later, but this just delays + // everything by 2112 frames, pushing stuff out of sync, and i think you can + // hear it. + } + + // if it's not the very first block of AAC, but is from the first few blocks of a + // new AAC sequence, it will contain noisy transients, so replace it with silence. + if ((blocks_read_in_sequence <= 2) && (blocks_read_in_sequence != blocks_read)) { + // debug(1,"Muting packet %u from block %u to avoid AAC transients because it's + // not from a true starting block. Blocks_read is %" PRIu64 ". + // blocks_read_in_sequence is %" PRIu64 ".", pcm_buffer_read_point_rtptime, + // timestamp, blocks_read, blocks_read_in_sequence); + conn->previous_random_number = generate_zero_frames( + (char *)(pcm_buffer + pcm_buffer_read_point), 352, config.output_format, + conn->enable_dither, conn->previous_random_number); + } + + player_put_packet( + 0, 0, pcm_buffer_read_point_rtptime + pcm_buffer_read_point_rtptime_offset, + pcm_buffer + pcm_buffer_read_point, 352, conn); + packets_played_in_this_sequence++; + expected_pcm_buffer_read_point_rtptime = pcm_buffer_read_point_rtptime + 352; + } // } } else { debug(3, "Dropping packet %u from block %u with out-of-range lead_time: %.3f seconds.", pcm_buffer_read_point_rtptime, seq_no, 0.000000001 * lead_time); - expected_pcm_buffer_read_point_rtptime = pcm_buffer_read_point_rtptime + 352; + expected_pcm_buffer_read_point_rtptime = pcm_buffer_read_point_rtptime + 352; } pcm_buffer_read_point_rtptime += 352; @@ -2642,13 +2660,16 @@ void *rtp_buffered_audio_processor(void *arg) { usleep(20000); // wait before asking if play is enabled again } } else { - // debug(1,"new buffer needed for buffer starting at %u because pcm_buffer_read_point (frames) is %u and pcm_buffer_occupancy (frames) is %u.", pcm_buffer_read_point_rtptime, pcm_buffer_read_point/conn->input_bytes_per_frame, + // debug(1,"new buffer needed for buffer starting at %u because pcm_buffer_read_point + // (frames) is %u and pcm_buffer_occupancy (frames) is %u.", + // pcm_buffer_read_point_rtptime, pcm_buffer_read_point/conn->input_bytes_per_frame, // pcm_buffer_occupancy/conn->input_bytes_per_frame); new_buffer_needed = 1; if (pcm_buffer_read_point != 0) { // debug(1,"pcm_buffer_read_point (frames): %u, pcm_buffer_occupancy (frames): %u", - //pcm_buffer_read_point/conn->input_bytes_per_frame, - //pcm_buffer_occupancy/conn->input_bytes_per_frame); // if there is anything to move down + // pcm_buffer_read_point/conn->input_bytes_per_frame, + // pcm_buffer_occupancy/conn->input_bytes_per_frame); // if there is anything to move + // down // to the front of the buffer, do it now; if ((pcm_buffer_occupancy - pcm_buffer_read_point) > 0) { // move the remaining frames down to the start of the buffer @@ -2668,9 +2689,9 @@ void *rtp_buffered_audio_processor(void *arg) { if ((flush_requested) || (new_buffer_needed)) { // debug(1,"pcm_buffer_read_point (frames): %u, pcm_buffer_occupancy (frames): %u", - // pcm_buffer_read_point/conn->input_bytes_per_frame, pcm_buffer_occupancy/conn->input_bytes_per_frame); - // ok, so here we know we need material from the sender - // do we will get in a packet of audio + // pcm_buffer_read_point/conn->input_bytes_per_frame, + // pcm_buffer_occupancy/conn->input_bytes_per_frame); ok, so here we know we need material + // from the sender do we will get in a packet of audio uint16_t data_len; // here we read from the buffer that our thread has been reading size_t bytes_remaining_in_buffer; @@ -2714,13 +2735,17 @@ void *rtp_buffered_audio_processor(void *arg) { previous_seq_no++; seq_no = packet[1] * (1 << 16) + packet[2] * (1 << 8) + packet[3]; if (previous_seq_no != seq_no) { - debug(2,"block sequence number changed from expected %u to actual %u.", previous_seq_no, seq_no); + debug(2, "block sequence number changed from expected %u to actual %u.", previous_seq_no, + seq_no); } timestamp = nctohl(&packet[4]); // debug(1,"New block timestamp: %u.", timestamp); int32_t timestamp_difference = timestamp - expected_timestamp; - if ((timestamp_difference != 0) && (expected_timesamp_is_reasonable != 0)) - debug(2, "Block with unexpected timestamp. Expected: %u, got: %u, difference: %d, blocks_read_in_sequence: %" PRIu64 ".", expected_timestamp, timestamp, timestamp_difference, blocks_read_in_sequence); + if ((timestamp_difference != 0) && (expected_timesamp_is_reasonable != 0)) + debug(2, + "Block with unexpected timestamp. Expected: %u, got: %u, difference: %d, " + "blocks_read_in_sequence: %" PRIu64 ".", + expected_timestamp, timestamp, timestamp_difference, blocks_read_in_sequence); expected_timestamp = timestamp; expected_timesamp_is_reasonable = 0; // must be validated each time by decoding the frame @@ -2922,12 +2947,14 @@ void *rtp_buffered_audio_processor(void *arg) { pcm_buffer_size); } else { memcpy(pcm_buffer + pcm_buffer_occupancy, pcm_audio, dst_bufsize); - expected_timestamp += (dst_bufsize/conn->input_bytes_per_frame); + expected_timestamp += (dst_bufsize / conn->input_bytes_per_frame); expected_timesamp_is_reasonable = 1; pcm_buffer_occupancy += dst_bufsize; - // debug(1,"frames added: pcm_buffer_read_point (frames): %u, pcm_buffer_occupancy (frames): %u", - // pcm_buffer_read_point/conn->input_bytes_per_frame, pcm_buffer_occupancy/conn->input_bytes_per_frame); - } + // debug(1,"frames added: pcm_buffer_read_point (frames): %u, + // pcm_buffer_occupancy (frames): %u", + // pcm_buffer_read_point/conn->input_bytes_per_frame, + // pcm_buffer_occupancy/conn->input_bytes_per_frame); + } // debug(1,"decoded %d samples", decoded_frame->nb_samples); // memcpy(sampleBuffer,outputBuffer16,dst_bufsize); av_freep(&pcm_audio); From 0c8b2200f72482917cdeda3a18181dacc14384bd Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Fri, 23 Sep 2022 17:11:15 +0100 Subject: [PATCH 15/19] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index b0d8dbb5..66c2135c 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,7 @@ +Version 4.1-dev-548-g7dc077e2 +==== +* Removed the workaround for the apparent bug in AirPlay 2 Buffered Streams in iOS 16.0. A number of approaches were tried. The most aurally successful is left in the code but is disabled. Basically, it delays the timing of the frames following the 2,122 frame discontinuity by 2,112 frames. This eliminates the timing discontinuity but delays the AirPlay stream by 47.9 ms, and this can be heard in some situations. So it looks as if the bug -- if that's what it is -- can't easily be worked around. It has not been fixed by iOS 16.0.2, unfortunately. + Version 4.1-dev-543-g24f06b81 ==== **Enhancements** From 0f476da0eef51820d175473a13373154aa723e5e Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Mon, 26 Sep 2022 17:24:04 +0100 Subject: [PATCH 16/19] Update the pair_ap library. Thanks again to ejurgensen for this code. --- pair_ap/client-example.c | 11 +++++++---- pair_ap/pair-internal.h | 4 ++-- pair_ap/pair-tlv.c | 3 +++ pair_ap/pair.h | 2 +- pair_ap/pair_fruit.c | 20 ++++++++------------ pair_ap/pair_homekit.c | 39 +++++++++++++++++---------------------- 6 files changed, 38 insertions(+), 41 deletions(-) diff --git a/pair_ap/client-example.c b/pair_ap/client-example.c index c5736ed5..31f46c62 100644 --- a/pair_ap/client-example.c +++ b/pair_ap/client-example.c @@ -45,18 +45,21 @@ static char * prompt_pin(void) { char *pin = NULL; - size_t len; + ssize_t len; + size_t sz; printf ("Enter pin: "); fflush (stdout); - len = getline(&pin, &len, stdin); - if (len != 5) // Includes EOL + len = getline(&pin, &sz, stdin); + if (len < 1) { - printf ("Bad pin length %zu\n", len); + printf("Bad pin entered (len=%zd)\n", len); return NULL; } + pin[len - 1] = '\0'; // Zero the EOL + return pin; } diff --git a/pair_ap/pair-internal.h b/pair_ap/pair-internal.h index 3de74daa..29302f1f 100644 --- a/pair_ap/pair-internal.h +++ b/pair_ap/pair-internal.h @@ -15,7 +15,7 @@ struct pair_client_setup_context { struct SRPUser *user; - uint8_t pin[4]; + char *pin; char device_id[PAIR_AP_DEVICE_ID_LEN_MAX]; pair_cb add_cb; @@ -50,7 +50,7 @@ struct pair_server_setup_context { struct SRPVerifier *verifier; - uint8_t pin[4]; + char *pin; char device_id[PAIR_AP_DEVICE_ID_LEN_MAX]; pair_cb add_cb; diff --git a/pair_ap/pair-tlv.c b/pair_ap/pair-tlv.c index ea1a7614..ef457407 100644 --- a/pair_ap/pair-tlv.c +++ b/pair_ap/pair-tlv.c @@ -70,6 +70,9 @@ pair_tlv_new() { void pair_tlv_free(pair_tlv_values_t *values) { + if (!values) + return; + pair_tlv_t *t = values->head; while (t) { pair_tlv_t *t2 = t; diff --git a/pair_ap/pair.h b/pair_ap/pair.h index 35a5aeb8..ec6c1a89 100644 --- a/pair_ap/pair.h +++ b/pair_ap/pair.h @@ -4,7 +4,7 @@ #include #define PAIR_AP_VERSION_MAJOR 0 -#define PAIR_AP_VERSION_MINOR 12 +#define PAIR_AP_VERSION_MINOR 14 #define PAIR_AP_DEVICE_ID_LEN_MAX 64 diff --git a/pair_ap/pair_fruit.c b/pair_ap/pair_fruit.c index 86f23636..5b81a56a 100644 --- a/pair_ap/pair_fruit.c +++ b/pair_ap/pair_fruit.c @@ -347,7 +347,7 @@ srp_user_process_challenge(struct SRPUser *usr, const unsigned char *bytes_s, in bnum u, x; *len_M = 0; - *bytes_M = 0; + *bytes_M = NULL; bnum_bin2bn(s, bytes_s, len_s); bnum_bin2bn(B, bytes_B, len_B); @@ -384,14 +384,7 @@ srp_user_process_challenge(struct SRPUser *usr, const unsigned char *bytes_s, in calculate_H_AMK(usr->alg, usr->H_AMK, usr->A, usr->M, usr->session_key, usr->session_key_len); *bytes_M = usr->M; - if (len_M) - *len_M = hash_length(usr->alg); - } - else - { - *bytes_M = NULL; - if (len_M) - *len_M = 0; + *len_M = hash_length(usr->alg); } cleanup2: @@ -607,10 +600,12 @@ client_setup_new(struct pair_setup_context *handle, const char *pin, pair_cb add if (!is_initialized()) return -1; - if (!pin || strlen(pin) < 4) + if (!pin) return -1; - memcpy(sctx->pin, pin, sizeof(sctx->pin)); + sctx->pin = strdup(pin); + if (!sctx->pin) + return -1; return 0; } @@ -627,6 +622,7 @@ client_setup_free(struct pair_setup_context *handle) free(sctx->salt); free(sctx->epk); free(sctx->authtag); + free(sctx->pin); } static uint8_t * @@ -639,7 +635,7 @@ client_setup_request1(size_t *len, struct pair_setup_context *handle) uint32_t uint32; char *data = NULL; // Necessary to initialize because plist_to_bin() uses value - sctx->user = srp_user_new(HASH_SHA1, SRP_NG_2048, USERNAME, (unsigned char *)sctx->pin, sizeof(sctx->pin), 0, 0); + sctx->user = srp_user_new(HASH_SHA1, SRP_NG_2048, USERNAME, (unsigned char *)sctx->pin, strlen(sctx->pin), 0, 0); dict = plist_new_dict(); diff --git a/pair_ap/pair_homekit.c b/pair_ap/pair_homekit.c index d08baca2..6452fb8c 100644 --- a/pair_ap/pair_homekit.c +++ b/pair_ap/pair_homekit.c @@ -472,7 +472,7 @@ srp_user_process_challenge(struct SRPUser *usr, const unsigned char *bytes_s, in bnum u, x; *len_M = 0; - *bytes_M = 0; + *bytes_M = NULL; bnum_bin2bn(s, bytes_s, len_s); bnum_bin2bn(B, bytes_B, len_B); @@ -512,14 +512,7 @@ srp_user_process_challenge(struct SRPUser *usr, const unsigned char *bytes_s, in calculate_H_AMK(usr->alg, usr->H_AMK, usr->A, usr->M, usr->session_key, usr->session_key_len); *bytes_M = usr->M; - if (len_M) - *len_M = hash_length(usr->alg); - } - else - { - *bytes_M = NULL; - if (len_M) - *len_M = 0; + *len_M = hash_length(usr->alg); } cleanup2: @@ -1180,20 +1173,17 @@ client_setup_new(struct pair_setup_context *handle, const char *pin, pair_cb add if (!is_initialized()) return -1; - if (handle->type == &pair_client_homekit_normal) - { - if (!pin || strlen(pin) < 4) - return -1; - } - else if (handle->type == &pair_client_homekit_transient && !pin) - { - pin = "3939"; - } + if (!pin && handle->type == &pair_client_homekit_transient) + pin = "3939"; + else if (!pin) + return -1; if (device_id && strlen(device_id) >= PAIR_AP_DEVICE_ID_LEN_MAX) return -1; - memcpy(sctx->pin, pin, sizeof(sctx->pin)); + sctx->pin = strdup(pin); + if (!sctx->pin) + return -1; sctx->add_cb = add_cb; sctx->add_cb_arg = cb_arg; @@ -1223,6 +1213,7 @@ client_setup_free(struct pair_setup_context *handle) free(sctx->salt); free(sctx->epk); free(sctx->authtag); + free(sctx->pin); } static uint8_t * @@ -1248,7 +1239,7 @@ client_setup_request1(size_t *len, struct pair_setup_context *handle) goto error; } - sctx->user = srp_user_new(HASH_SHA512, SRP_NG_3072, USERNAME, (unsigned char *)sctx->pin, sizeof(sctx->pin), 0, 0); + sctx->user = srp_user_new(HASH_SHA512, SRP_NG_3072, USERNAME, (unsigned char *)sctx->pin, strlen(sctx->pin), 0, 0); if (!sctx->user) { handle->errmsg = "Setup request 1: Create SRP user failed"; @@ -1949,6 +1940,7 @@ client_verify_response2(struct pair_verify_context *handle, const uint8_t *data, handle->status = PAIR_STATUS_COMPLETED; + pair_tlv_free(response); return 0; } @@ -2010,7 +2002,9 @@ server_setup_new(struct pair_setup_context *handle, const char *pin, pair_cb add if (!device_id || strlen(device_id) >= PAIR_AP_DEVICE_ID_LEN_MAX) return -1; - memcpy(sctx->pin, pin, sizeof(sctx->pin)); + sctx->pin = strdup(pin); + if (!sctx->pin) + return -1; sctx->add_cb = add_cb; sctx->add_cb_arg = cb_arg; @@ -2035,6 +2029,7 @@ server_setup_free(struct pair_setup_context *handle) free(sctx->M1); free(sctx->v); free(sctx->salt); + free(sctx->pin); } static int @@ -2063,7 +2058,7 @@ server_setup_request1(struct pair_setup_context *handle, const uint8_t *data, si sctx->is_transient = (type && type->size == 1 && type->value[0] == PairingFlagsTransient); // Note this is modified to return a 16 byte salt - ret = srp_create_salted_verification_key(HASH_SHA512, SRP_NG_3072, USERNAME, (unsigned char *)sctx->pin, sizeof(sctx->pin), + ret = srp_create_salted_verification_key(HASH_SHA512, SRP_NG_3072, USERNAME, (unsigned char *)sctx->pin, strlen(sctx->pin), &sctx->salt, &sctx->salt_len, &sctx->v, &sctx->v_len, NULL, NULL); if (ret < 0) { From 30aaabc47ec1ba30aec1865dae28be0e23901d12 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Mon, 26 Sep 2022 17:42:13 +0100 Subject: [PATCH 17/19] Remove the need for the gcrypt library; use openssl instead, as it's mandatory elsewhere in Shairport Sync. --- Makefile.am | 2 +- configure.ac | 5 ----- shairport.c | 23 ----------------------- 3 files changed, 1 insertion(+), 29 deletions(-) diff --git a/Makefile.am b/Makefile.am index 666047ea..b681ed3e 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1,7 +1,7 @@ SUBDIRS = man ARFLAGS = cr -lib_pair_ap_a_CFLAGS = -Wall -g -DCONFIG_GCRYPT -pthread +lib_pair_ap_a_CFLAGS = -Wall -g -DCONFIG_OPENSSL -pthread lib_tinyhttp_a_CFLAGS = -pthread lib_dbus_interface_a_CFLAGS = -pthread lib_mpris_interface_a_CFLAGS = -pthread diff --git a/configure.ac b/configure.ac index b0f53cd9..fd233952 100644 --- a/configure.ac +++ b/configure.ac @@ -412,11 +412,6 @@ if test "x$with_airplay_2" = "xyes" ; then ]) ]) PKG_CHECK_MODULES([libsodium], [libsodium],[CFLAGS="${libsodium_CFLAGS} ${CFLAGS}" LIBS="${libsodium_LIBS} ${LIBS}"],[AC_MSG_ERROR(AirPlay 2 support requires libsodium -- libsodium-dev suggested)]) - PKG_CHECK_MODULES([libgcrypt], [libgcrypt],[CFLAGS="${libgcrypt_CFLAGS} ${CFLAGS}" LIBS="${libgcrypt_LIBS} ${LIBS}"],[ - AC_CHECK_LIB([gcrypt], [gcry_control], [], [ - AC_MSG_ERROR([Airplay 2 support requires libgcrypt -- libgcrypt-dev suggested]) - ]) - ]) PKG_CHECK_MODULES([libavutil], [libavutil],[CFLAGS="${libavutil_CFLAGS} ${CFLAGS}" LIBS="${libavutil_LIBS} ${LIBS}"],[ AC_MSG_ERROR(AirPlay 2 support requires libavutil -- libavutil-dev suggested)]) PKG_CHECK_MODULES([libavcodec], [libavcodec],[CFLAGS="${libavcodec_CFLAGS} ${CFLAGS}" LIBS="${libavcodec_LIBS} ${LIBS}"],[ diff --git a/shairport.c b/shairport.c index 9d1657f0..aec61bc7 100644 --- a/shairport.c +++ b/shairport.c @@ -45,7 +45,6 @@ #ifdef CONFIG_AIRPLAY_2 #include "ptp-utilities.h" -#include #include #include #include @@ -2104,28 +2103,6 @@ int main(int argc, char **argv) { debug(1, "libsodium initialised."); } - // this code is based on - // https://www.gnupg.org/documentation/manuals/gcrypt/Initializing-the-library.html - - /* Version check should be the very first call because it - makes sure that important subsystems are initialized. - #define NEED_LIBGCRYPT_VERSION to the minimum required version. */ - -#define NEED_LIBGCRYPT_VERSION "1.5.4" - - if (!gcry_check_version(NEED_LIBGCRYPT_VERSION)) { - die("libgcrypt is too old (need %s, have %s).", NEED_LIBGCRYPT_VERSION, - gcry_check_version(NULL)); - } - - /* Disable secure memory. */ - gcry_control(GCRYCTL_DISABLE_SECMEM, 0); - - /* ... If required, other initialization goes here. */ - - /* Tell Libgcrypt that initialization has completed. */ - gcry_control(GCRYCTL_INITIALIZATION_FINISHED, 0); - #endif /* Mess around with the latency options */ From 85f9bb373b42446a243e55e137099a8d76651d12 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Mon, 26 Sep 2022 17:53:34 +0100 Subject: [PATCH 18/19] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index 66c2135c..c90367b9 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,11 @@ +Version 4.1-dev-551-g30aaabc4 +==== +**Update** +* Updated to the latest version of the [`pair_ap`](https://github.com/ejurgensen/pair_ap) library. Thanks again to [ejurgensen](https://github.com/ejurgensen/pair_ap/commits?author=ejurgensen) for this invaluable resource. + +**Enhancement** +* Removed the requirement for the [`libgcrypt`](https://gnupg.org/software/libgcrypt/index.html) library. OpenSSL is mandatory for other parts of Shairport Sync when operating in the AirPlay 2 mode and is now used in place of `libgcrypt`. + Version 4.1-dev-548-g7dc077e2 ==== * Removed the workaround for the apparent bug in AirPlay 2 Buffered Streams in iOS 16.0. A number of approaches were tried. The most aurally successful is left in the code but is disabled. Basically, it delays the timing of the frames following the 2,122 frame discontinuity by 2,112 frames. This eliminates the timing discontinuity but delays the AirPlay stream by 47.9 ms, and this can be heard in some situations. So it looks as if the bug -- if that's what it is -- can't easily be worked around. It has not been fixed by iOS 16.0.2, unfortunately. From cda00e95301df0a7366faaf57a02e86566316f0d Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Mon, 26 Sep 2022 17:56:02 +0100 Subject: [PATCH 19/19] Update BUILD.md --- BUILD.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/BUILD.md b/BUILD.md index 8d8e9352..f787b60a 100644 --- a/BUILD.md +++ b/BUILD.md @@ -39,7 +39,7 @@ Okay, now let's get the tools and libraries for building and installing Shairpor # apt upgrade # this is optional but recommended # apt install --no-install-recommends build-essential git xmltoman autoconf automake libtool \ libpopt-dev libconfig-dev libasound2-dev avahi-daemon libavahi-client-dev libssl-dev libsoxr-dev \ - libplist-dev libsodium-dev libavutil-dev libavcodec-dev libavformat-dev uuid-dev libgcrypt-dev xxd + libplist-dev libsodium-dev libavutil-dev libavcodec-dev libavformat-dev uuid-dev xxd ``` If you are building classic Shairport Sync, the list of packages is shorter: ``` @@ -54,7 +54,7 @@ For AirPlay 2 operation, _before you install the libraries_, please ensure the y # yum update # yum install make automake gcc gcc-c++ \ git xmltoman autoconf automake avahi-devel libconfig-devel openssl-devel popt-devel soxr-devel \ - ffmpeg ffmpeg-devel libplist-devel libsodium-devel libgcrypt-devel libuuid-devel vim-common \ + ffmpeg ffmpeg-devel libplist-devel libsodium-devel libuuid-devel vim-common \ alsa-lib-devel ``` If you are building classic Shairport Sync, the list of packages is shorter: