From: Mike Brady Date: Sat, 26 Mar 2016 21:40:59 +0000 (+0000) Subject: Add no-sync option, don't try to sync if delay() returns -1, check and comment some... X-Git-Tag: 2.9.5.2~10 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=cca403c26072e023ad67372f3d68cd1b4ec70675;p=thirdparty%2Fshairport-sync.git Add no-sync option, don't try to sync if delay() returns -1, check and comment some integer and sign promotion, update stats printing, (but more to do there). --- diff --git a/player.c b/player.c index ebd7cf19..5f9127b5 100644 --- a/player.c +++ b/player.c @@ -168,8 +168,8 @@ static inline seq_t PREDECESSOR(seq_t x) { // anything with ORDINATE in it must be proctected by the ab_mutex static inline int32_t ORDINATE(seq_t x) { - int32_t p = x & 0xffff; - int32_t q = ab_read & 0x0ffff; + int32_t p = x; // int32_t from seq_t, i.e. uint16_t, so okay + int32_t q = ab_read; // int32_t from seq_t, i.e. uint16_t, so okay int32_t t = (p + 0x10000 - q) & 0xffff; // we definitely will get a positive number in t at this point, but it might be a // positive alias of a negative number, i.e. x might actually be "before" ab_read @@ -935,9 +935,9 @@ static void *player_thread_func(void *arg) { int64_t tsum_of_sync_errors, tsum_of_corrections, tsum_of_insertions_and_deletions, tsum_of_drifts; int64_t previous_sync_error, previous_correction; - int64_t minimum_dac_queue_size = 1000000; - int32_t minimum_buffer_occupancy = BUFFER_FRAMES; - int32_t maximum_buffer_occupancy = 0; + int64_t minimum_dac_queue_size = INT64_MAX; + int32_t minimum_buffer_occupancy = INT32_MAX; + int32_t maximum_buffer_occupancy = INT32_MIN; buffer_occupancy = 0; @@ -992,25 +992,28 @@ static void *player_thread_func(void *arg) { uint32_t reference_timestamp; uint64_t reference_timestamp_time,remote_reference_timestamp_time; - get_reference_timestamp_stuff(&reference_timestamp, &reference_timestamp_time, &remote_reference_timestamp_time); + get_reference_timestamp_stuff(&reference_timestamp, &reference_timestamp_time, &remote_reference_timestamp_time); // types okay int64_t rt, nt; - rt = reference_timestamp; - nt = inframe->timestamp; + rt = reference_timestamp; // uint32_t to int64_t + nt = inframe->timestamp; // uint32_t to int64_t - uint64_t local_time_now = get_absolute_time_in_fp(); + uint64_t local_time_now = get_absolute_time_in_fp(); // types okay // struct timespec tn; // clock_gettime(CLOCK_MONOTONIC,&tn); // uint64_t // local_time_now=((uint64_t)tn.tv_sec<<32)+((uint64_t)tn.tv_nsec<<32)/1000000000; - int64_t td_in_frames; - int64_t td = local_time_now - reference_timestamp_time; + int64_t td = 0; + int64_t td_in_frames = 0; + if (local_time_now >= reference_timestamp_time) { // debug(1,"td is %lld.",td); - if (td >= 0) { + td = local_time_now - reference_timestamp_time; // this is the positive value. Conversion is positive uint64_t to int64_t, thus okay td_in_frames = (td * 44100) >> 32; } else { - td_in_frames = -((-td * 44100) >> 32); + td = reference_timestamp_time - local_time_now; // this is the absolute value, which should be negated. Conversion is positive uint64_t to int64_t, thus okay. + td_in_frames = -((td * 44100) >> 32); //use the absolute td value for the present. Types okay + td = -td; // should be okay, as the range of values should be very small w.r.t 64 bits } // This is the timing error for the next audio frame in the DAC, if applicable @@ -1020,10 +1023,10 @@ static void *player_thread_func(void *arg) { // check sequencing if (last_seqno_read == -1) - last_seqno_read = inframe->sequence_number; + last_seqno_read = inframe->sequence_number; //int32_t from seq_t, i.e. uint16_t, so okay. else { - last_seqno_read = (SUCCESSOR(last_seqno_read) & 0xffff); - if (inframe->sequence_number != last_seqno_read) { + last_seqno_read = SUCCESSOR(last_seqno_read); // int32_t from seq_t, i.e. uint16_t, so okay. + if (inframe->sequence_number != last_seqno_read) { //seq_t, ei.e. uint16_t and int32_t, so okay debug( 1, "Player: packets out of sequence: expected: %d, got: %d, sync error: %d frames.", @@ -1032,7 +1035,7 @@ static void *player_thread_func(void *arg) { } } - buffer_occupancy = seq_diff(ab_read, ab_write); + buffer_occupancy = seq_diff(ab_read, ab_write); // int32_t from int32 if (buffer_occupancy < minimum_buffer_occupancy) minimum_buffer_occupancy = buffer_occupancy; @@ -1040,26 +1043,39 @@ static void *player_thread_func(void *arg) { if (buffer_occupancy > maximum_buffer_occupancy) maximum_buffer_occupancy = buffer_occupancy; - if (config.output->delay) { - current_delay = config.output->delay(); - if (current_delay == -1) { + // here, we want to check (a) if we are meant to do synchronisation, (b) if we have a delay procedure, (b) if we can get the delay. + // If any of these are false, we don't do any synchronisation stuff + + current_delay = -1; // use this as a failure flag + + if (config.output->delay) { + current_delay = config.output->delay(); // int64_t from int32_t, so okay; + if (current_delay >= 0) { + if (current_delay < minimum_dac_queue_size) { + minimum_dac_queue_size = current_delay; // update for display later + } + } else { debug(1, "Delay error when checking running latency."); - current_delay = 0; } - if (current_delay < minimum_dac_queue_size) - minimum_dac_queue_size = current_delay; + } + + if (current_delay >= 0) { // this is the actual delay, including the latency we actually want, which will // fluctuate a good bit about a potentially rising or falling trend. - int64_t delay = td_in_frames + rt - (nt - current_delay); + int64_t delay = td_in_frames + rt - (nt - current_delay); // all int64_t + // This is the timing error for the next audio frame in the DAC. - sync_error = delay - config.latency; + sync_error = delay - config.latency; // int64_t from int64_t - int32_t, so okay + + // if (llabs(sync_error)>352*512) + // debug(1,"Very large sync error: %lld",sync_error); // before we finally commit to this frame, check its sequencing and timing // require a certain error before bothering to fix it... - if (sync_error > config.tolerance) { + if (sync_error > config.tolerance) { // int64_t > int, okay amount_to_stuff = -1; } if (sync_error < -config.tolerance) { @@ -1079,7 +1095,7 @@ static void *player_thread_func(void *arg) { if (amount_to_stuff) { if ((local_time_now) && (first_packet_time_to_play) && (local_time_now >= first_packet_time_to_play)) { - int64_t tp = (local_time_now - first_packet_time_to_play)>>32; // seconds + int64_t tp = (local_time_now - first_packet_time_to_play)>>32; // seconds int64_t from uint64_t which is always positive, so ok if (tp<5) amount_to_stuff = 0; // wait at least five seconds @@ -1089,6 +1105,9 @@ static void *player_thread_func(void *arg) { } } } + + if (config.no_sync!=0) + amount_to_stuff = 0 ; // no stuffing if it's been disabled if ((amount_to_stuff == 0) && (fix_volume == 0x10000)) { // if no stuffing needed and no volume adjustment, then @@ -1132,8 +1151,14 @@ static void *player_thread_func(void *arg) { // check for loss of sync // timestamp of zero means an inserted silent frame in place of a missing frame - if ((inframe->timestamp != 0) && (!please_stop) && (config.resyncthreshold != 0) && - (abs(sync_error) > config.resyncthreshold)) { + + // not too sure if abs() is implemented for int64_t, so we'll do it manually + int64_t abs_sync_error = sync_error; + if (abs_sync_error<0) + abs_sync_error = -abs_sync_error; + + if ((config.no_sync==0) && (inframe->timestamp != 0) && (!please_stop) && (config.resyncthreshold != 0) && + (abs_sync_error > config.resyncthreshold)) { sync_error_out_of_bounds++; // debug(1,"Sync error out of bounds: Error: %lld; previous error: %lld; DAC: %lld; // timestamp: %llx, time now @@ -1149,11 +1174,12 @@ static void *player_thread_func(void *arg) { sync_error_out_of_bounds = 0; } } else { - // if there is no delay procedure, there can be no synchronising + // if there is no delay procedure, or it's not working or not allowed, there can be no synchronising + if (fix_volume == 0x10000) config.output->play(inbuf, frame_size); else { - play_samples = stuff_buffer_basic(inbuf, outbuf, 0); + play_samples = stuff_buffer_basic(inbuf, outbuf, 0); // no stuffing, but volume adjustment config.output->play(outbuf, frame_size); } } @@ -1166,45 +1192,49 @@ static void *player_thread_func(void *arg) { // new stats calculation. We want a running average of sync error, drift, adjustment, // number of additions+subtractions + + // this is a misleading hack -- the statistics should include some data on the number of valid samples and the number of times sync wasn't checked due to non availability of a delay figure. + // for the present, stats are only updated when sync has been checked + if (sync_error!=-1) { + if (number_of_statistics == trend_interval) { + // here we remove the oldest statistical data and take it from the summaries as well + tsum_of_sync_errors -= statistics[oldest_statistic].sync_error; + tsum_of_drifts -= statistics[oldest_statistic].drift; + if (statistics[oldest_statistic].correction > 0) + tsum_of_insertions_and_deletions -= statistics[oldest_statistic].correction; + else + tsum_of_insertions_and_deletions += statistics[oldest_statistic].correction; + tsum_of_corrections -= statistics[oldest_statistic].correction; + oldest_statistic = (oldest_statistic + 1) % trend_interval; + number_of_statistics--; + } - if (number_of_statistics == trend_interval) { - // here we remove the oldest statistical data and take it from the summaries as well - tsum_of_sync_errors -= statistics[oldest_statistic].sync_error; - tsum_of_drifts -= statistics[oldest_statistic].drift; - if (statistics[oldest_statistic].correction > 0) - tsum_of_insertions_and_deletions -= statistics[oldest_statistic].correction; - else - tsum_of_insertions_and_deletions += statistics[oldest_statistic].correction; - tsum_of_corrections -= statistics[oldest_statistic].correction; - oldest_statistic = (oldest_statistic + 1) % trend_interval; - number_of_statistics--; - } - - statistics[newest_statistic].sync_error = sync_error; - statistics[newest_statistic].correction = amount_to_stuff; + statistics[newest_statistic].sync_error = sync_error; + statistics[newest_statistic].correction = amount_to_stuff; - if (number_of_statistics == 0) - statistics[newest_statistic].drift = 0; - else - statistics[newest_statistic].drift = - sync_error - previous_sync_error - previous_correction; + if (number_of_statistics == 0) + statistics[newest_statistic].drift = 0; + else + statistics[newest_statistic].drift = + sync_error - previous_sync_error - previous_correction; - previous_sync_error = sync_error; - previous_correction = amount_to_stuff; + previous_sync_error = sync_error; + previous_correction = amount_to_stuff; - tsum_of_sync_errors += sync_error; - tsum_of_drifts += statistics[newest_statistic].drift; - if (amount_to_stuff > 0) { - tsum_of_insertions_and_deletions += amount_to_stuff; - } else { - tsum_of_insertions_and_deletions -= amount_to_stuff; - } - tsum_of_corrections += amount_to_stuff; - session_corrections += amount_to_stuff; + tsum_of_sync_errors += sync_error; + tsum_of_drifts += statistics[newest_statistic].drift; + if (amount_to_stuff > 0) { + tsum_of_insertions_and_deletions += amount_to_stuff; + } else { + tsum_of_insertions_and_deletions -= amount_to_stuff; + } + tsum_of_corrections += amount_to_stuff; + session_corrections += amount_to_stuff; - newest_statistic = (newest_statistic + 1) % trend_interval; - number_of_statistics++; + newest_statistic = (newest_statistic + 1) % trend_interval; + number_of_statistics++; + } } if (play_number % print_interval == 0) { // we can now calculate running averages for sync error (frames), corrections (ppm), @@ -1217,29 +1247,39 @@ static void *player_thread_func(void *arg) { // if ((play_number/print_interval)%20==0) if (config.statistics_requested) { if (at_least_one_frame_seen) { - if (config.output->delay) - inform("Sync error: %.1f (frames); net correction: %.1f (ppm); corrections: %.1f " - "(ppm); missing packets %llu; late packets %llu; too late packets %llu; " - "resend requests %llu; min DAC queue size %lli, min and max buffer occupancy " - "%d and %d.", - moving_average_sync_error, moving_average_correction * 1000000 / 352, - moving_average_insertions_plus_deletions * 1000000 / 352, missing_packets, - late_packets, too_late_packets, resend_requests, minimum_dac_queue_size, - minimum_buffer_occupancy, maximum_buffer_occupancy); - else + if ((config.output->delay)) { + if (config.no_sync==0) { + inform("Sync error: %.1f (frames); net correction: %.1f (ppm); corrections: %.1f " + "(ppm); missing packets %llu; late packets %llu; too late packets %llu; " + "resend requests %llu; min DAC queue size %lli, min and max buffer occupancy " + "%d and %d.", + moving_average_sync_error, moving_average_correction * 1000000 / 352, + moving_average_insertions_plus_deletions * 1000000 / 352, missing_packets, + late_packets, too_late_packets, resend_requests, minimum_dac_queue_size, + minimum_buffer_occupancy, maximum_buffer_occupancy); + } else { + inform("Synchronisation disabled. Sync error: %.1f (frames); missing packets %llu; late packets %llu; too late packets %llu; " + "resend requests %llu; min DAC queue size %lli, min and max buffer occupancy " + "%d and %d.", + moving_average_sync_error, missing_packets, + late_packets, too_late_packets, resend_requests, minimum_dac_queue_size, + minimum_buffer_occupancy, maximum_buffer_occupancy); + } + } else { inform("Synchronisation disabled. Missing packets %llu; late packets %llu; too late packets %llu; " "resend requests %llu; min and max buffer occupancy " "%d and %d.", missing_packets, late_packets, too_late_packets, resend_requests, - minimum_buffer_occupancy, maximum_buffer_occupancy); + minimum_buffer_occupancy, maximum_buffer_occupancy); + } } else { inform("No frames received in the last sampling interval."); } } - minimum_dac_queue_size = 1000000; // hack reset - maximum_buffer_occupancy = 0; // can't be less than this - minimum_buffer_occupancy = BUFFER_FRAMES; // can't be more than this + minimum_dac_queue_size = INT64_MAX; // hack reset + maximum_buffer_occupancy = INT32_MIN; // can't be less than this + minimum_buffer_occupancy = INT32_MAX; // can't be more than this at_least_one_frame_seen = 0; } }