Thread: [Linuxptp-devel] [PATCH RFC] Replace moving average with moving median for delay filtering.
PTP IEEE 1588 stack for Linux
Brought to you by:
rcochran
From: Miroslav L. <mli...@re...> - 2013-10-09 10:19:18
|
The moving average used in the path delay calculation is very sensitive to outliers. Replace it with moving median. This, for instance, allows much faster recovery from an external clock time step which happened between receiving sync message and sending delay_req message. The measured delay includes a large error, but the median is still a good estimate of the delay and the first step correction applied by the servo is right. In this implementation the median update has linear time complexity. Signed-off-by: Miroslav Lichvar <mli...@re...> --- clock.c | 18 ++++++++-------- makefile | 2 +- mave.c => mmedian.c | 60 +++++++++++++++++++++++++++++++++++++++-------------- mave.h => mmedian.h | 14 ++++++------- port.c | 16 +++++++------- 5 files changed, 70 insertions(+), 40 deletions(-) rename mave.c => mmedian.c (52%) rename mave.h => mmedian.h (77%) diff --git a/clock.c b/clock.c index 419a6b7..8e7fc56 100644 --- a/clock.c +++ b/clock.c @@ -26,7 +26,7 @@ #include "clock.h" #include "clockadj.h" #include "foreign.h" -#include "mave.h" +#include "mmedian.h" #include "missing.h" #include "msg.h" #include "phc.h" @@ -40,7 +40,7 @@ #define CLK_N_PORTS (MAX_PORTS + 1) /* plus one for the UDS interface */ #define N_CLOCK_PFD (N_POLLFD + 1) /* one extra per port, for the fault timer */ -#define MAVE_LENGTH 10 +#define MMEDIAN_LENGTH 10 #define POW2_41 ((double)(1ULL << 41)) #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0])) @@ -85,7 +85,7 @@ struct clock { enum servo_state servo_state; tmv_t master_offset; tmv_t path_delay; - struct mave *avg_delay; + struct mmedian *med_delay; struct freq_estimator fest; struct time_status_np status; double nrr; @@ -119,7 +119,7 @@ void clock_destroy(struct clock *c) phc_close(c->clkid); } servo_destroy(c->servo); - mave_destroy(c->avg_delay); + mmedian_destroy(c->med_delay); stats_destroy(c->stats.offset); stats_destroy(c->stats.freq); stats_destroy(c->stats.delay); @@ -630,9 +630,9 @@ struct clock *clock_create(int phc_index, struct interface *iface, int count, return NULL; } c->servo_state = SERVO_UNLOCKED; - c->avg_delay = mave_create(MAVE_LENGTH); - if (!c->avg_delay) { - pr_err("Failed to create moving average"); + c->med_delay = mmedian_create(MMEDIAN_LENGTH); + if (!c->med_delay) { + pr_err("Failed to create moving median"); return NULL; } c->stats_interval = dds->stats_interval; @@ -983,7 +983,7 @@ void clock_path_delay(struct clock *c, struct timespec req, struct timestamp rx, pr_warning("c3 %10lld", c3); } - c->path_delay = mave_accumulate(c->avg_delay, pd); + c->path_delay = mmedian_accumulate(c->med_delay, pd); c->cur.meanPathDelay = tmv_to_TimeInterval(c->path_delay); @@ -1154,7 +1154,7 @@ static void handle_state_decision_event(struct clock *c) if (!cid_eq(&best_id, &c->best_id)) { clock_freq_est_reset(c); - mave_reset(c->avg_delay); + mmedian_reset(c->med_delay); c->path_delay = 0; fresh_best = 1; } diff --git a/makefile b/makefile index d79a1df..71119a1 100644 --- a/makefile +++ b/makefile @@ -32,7 +32,7 @@ VER = -DVER=$(version) CFLAGS = -Wall $(VER) $(INC) $(DEBUG) $(FEAT_CFLAGS) $(EXTRA_CFLAGS) LDLIBS = -lm -lrt $(EXTRA_LDFLAGS) PRG = ptp4l pmc phc2sys hwstamp_ctl -OBJ = bmc.o clock.o clockadj.o config.o fault.o fsm.o ptp4l.o mave.o \ +OBJ = bmc.o clock.o clockadj.o config.o fault.o fsm.o ptp4l.o mmedian.o \ msg.o phc.o pi.o port.o print.o raw.o servo.o sk.o stats.o tlv.o tmtab.o \ transport.o udp.o udp6.o uds.o util.o version.o diff --git a/mave.c b/mmedian.c similarity index 52% rename from mave.c rename to mmedian.c index 7742f05..f55dad0 100644 --- a/mave.c +++ b/mmedian.c @@ -1,5 +1,5 @@ /** - * @file mave.c + * @file mmedian.c * @note Copyright (C) 2011 Richard Cochran <ric...@gm...> * * This program is free software; you can redistribute it and/or modify @@ -19,25 +19,33 @@ #include <stdlib.h> #include <string.h> -#include "mave.h" +#include "mmedian.h" -struct mave { +struct mmedian { int cnt; int len; int index; - tmv_t sum; + /* Indices sorted by value. */ + int *order; + /* Values stored in circular buffer. */ tmv_t *val; }; -struct mave *mave_create(int length) +struct mmedian *mmedian_create(int length) { - struct mave *m; + struct mmedian *m; m = calloc(1, sizeof(*m)); - if (!m) { + if (!m || length < 1) { + return NULL; + } + m->order = calloc(1, length * sizeof(*m->order)); + if (!m->order) { + free(m); return NULL; } m->val = calloc(1, length * sizeof(*m->val)); if (!m->val) { + free(m->order); free(m); return NULL; } @@ -45,28 +53,50 @@ struct mave *mave_create(int length) return m; } -void mave_destroy(struct mave *m) +void mmedian_destroy(struct mmedian *m) { + free(m->order); free(m->val); free(m); } -tmv_t mave_accumulate(struct mave *m, tmv_t val) +tmv_t mmedian_accumulate(struct mmedian *m, tmv_t val) { - m->sum = tmv_sub(m->sum, m->val[m->index]); + int i; + m->val[m->index] = val; - m->index = (1 + m->index) % m->len; - m->sum = tmv_add(m->sum, val); if (m->cnt < m->len) { m->cnt++; + } else { + /* Remove index of the replaced value from order. */ + for (i = 0; i < m->cnt; i++) + if (m->order[i] == m->index) + break; + for (; i + 1 < m->cnt; i++) + m->order[i] = m->order[i + 1]; } - return tmv_div(m->sum, m->cnt); + + /* Insert index of the new value to order. */ + for (i = m->cnt - 1; i > 0; i--) { + if (m->val[m->order[i - 1]] <= m->val[m->index]) + break; + m->order[i] = m->order[i - 1]; + } + m->order[i] = m->index; + + m->index = (1 + m->index) % m->len; + + if (m->cnt % 2) + return m->val[m->order[m->cnt / 2]]; + else + return tmv_div(tmv_add(m->val[m->order[m->cnt / 2 - 1]], + m->val[m->order[m->cnt / 2]]), 2); } -void mave_reset(struct mave *m) +void mmedian_reset(struct mmedian *m) { m->cnt = 0; m->index = 0; - m->sum = 0; + memset(m->order, 0, m->len * sizeof(*m->order)); memset(m->val, 0, m->len * sizeof(*m->val)); } diff --git a/mave.h b/mmedian.h similarity index 77% rename from mave.h rename to mmedian.h index 84241d4..f0bd8ac 100644 --- a/mave.h +++ b/mmedian.h @@ -1,6 +1,6 @@ /** - * @file mave.h - * @brief Implements a moving average. + * @file mmedian.h + * @brief Implements a moving median. * @note Copyright (C) 2011 Richard Cochran <ric...@gm...> * * This program is free software; you can redistribute it and/or modify @@ -22,14 +22,14 @@ #include "tmv.h" -struct mave; +struct mmedian; -struct mave *mave_create(int length); +struct mmedian *mmedian_create(int length); -void mave_destroy(struct mave *m); +void mmedian_destroy(struct mmedian *m); -tmv_t mave_accumulate(struct mave *m, tmv_t val); +tmv_t mmedian_accumulate(struct mmedian *m, tmv_t val); -void mave_reset(struct mave *m); +void mmedian_reset(struct mmedian *m); #endif diff --git a/port.c b/port.c index 3fc262a..e6ef5b1 100644 --- a/port.c +++ b/port.c @@ -25,7 +25,7 @@ #include "bmc.h" #include "clock.h" -#include "mave.h" +#include "mmedian.h" #include "missing.h" #include "msg.h" #include "port.h" @@ -37,7 +37,7 @@ #include "util.h" #define ALLOWED_LOST_RESPONSES 3 -#define PORT_MAVE_LENGTH 10 +#define PORT_MMEDIAN_LENGTH 10 enum syfu_state { SF_EMPTY, @@ -82,7 +82,7 @@ struct port { } seqnum; struct tmtab tmtab; tmv_t peer_delay; - struct mave *avg_delay; + struct mmedian *med_delay; int log_sync_interval; struct nrate_estimator nrate; unsigned int pdr_missing; @@ -1712,7 +1712,7 @@ calc: pd = tmv_sub(pd, c2); pd = tmv_div(pd, 2); - p->peer_delay = mave_accumulate(p->avg_delay, pd); + p->peer_delay = mmedian_accumulate(p->med_delay, pd); p->peerMeanPathDelay = tmv_to_TimeInterval(p->peer_delay); @@ -1841,7 +1841,7 @@ void port_close(struct port *p) port_disable(p); } transport_destroy(p->trp); - mave_destroy(p->avg_delay); + mmedian_destroy(p->med_delay); free(p); } @@ -2299,9 +2299,9 @@ struct port *port_open(int phc_index, p->delayMechanism = interface->dm; p->versionNumber = PTP_VERSION; - p->avg_delay = mave_create(PORT_MAVE_LENGTH); - if (!p->avg_delay) { - pr_err("Failed to create moving average"); + p->med_delay = mmedian_create(PORT_MMEDIAN_LENGTH); + if (!p->med_delay) { + pr_err("Failed to create moving median"); transport_destroy(p->trp); free(p); return NULL; -- 1.8.3.1 |
From: Richard C. <ric...@gm...> - 2013-10-09 19:48:28
|
On Wed, Oct 09, 2013 at 12:19:06PM +0200, Miroslav Lichvar wrote: > The moving average used in the path delay calculation is very sensitive > to outliers. Replace it with moving median. I like this idea. > This, for instance, allows much faster recovery from an external clock > time step which happened between receiving sync message and sending > delay_req message. The measured delay includes a large error, but the > median is still a good estimate of the delay and the first step > correction applied by the servo is right. I wonder what the effect might be in the case running PTP over normal (non-transparent) networks. These introduce a variable path delay, with outliers that keep repeating at some rate. Letting the longer paths into the estimate (in proportion to their occurrence, ie in the average) might perform better in this case than possibly filtering them out. Just thinking out loud... > -tmv_t mave_accumulate(struct mave *m, tmv_t val); > +tmv_t mmedian_accumulate(struct mmedian *m, tmv_t val); "Accumulate" seems like the wrong word for this, but other than that, the patch looks okay to me. Maybe we should offer both options in the manner of the servo interface? Thanks, Richard |
From: Miroslav L. <mli...@re...> - 2013-10-10 12:38:21
|
On Wed, Oct 09, 2013 at 09:48:07PM +0200, Richard Cochran wrote: > On Wed, Oct 09, 2013 at 12:19:06PM +0200, Miroslav Lichvar wrote: > > This, for instance, allows much faster recovery from an external clock > > time step which happened between receiving sync message and sending > > delay_req message. The measured delay includes a large error, but the > > median is still a good estimate of the delay and the first step > > correction applied by the servo is right. > > I wonder what the effect might be in the case running PTP over normal > (non-transparent) networks. These introduce a variable path delay, > with outliers that keep repeating at some rate. In the simulations with exponentially distributed delays the RMS time and frequency error seem to be the same as with the moving average. If there is a difference, it will be smaller than few percent. The servo performance tests from the test suite pass without any changes to the limits. > Letting the longer paths into the estimate (in proportion to their > occurrence, ie in the average) might perform better in this case than > possibly filtering them out. Just thinking out loud... If you have a specific distribution in mind, we can try it in the simulator. I guess it would have to be extremely asymmetric to see a difference in the offsets passed to the servo when median or average is used. > > -tmv_t mave_accumulate(struct mave *m, tmv_t val); > > +tmv_t mmedian_accumulate(struct mmedian *m, tmv_t val); > > "Accumulate" seems like the wrong word for this, but other than that, > the patch looks okay to me. Ok, I can change it to mmedian_update. > > Maybe we should offer both options in the manner of the servo > interface? >From the results I've seen so far it doesn't seem to be necessary. -- Miroslav Lichvar |
From: Miroslav L. <mli...@re...> - 2013-10-29 10:58:29
|
Changes in v3: - fixed declaration of filter_sample() - renamed accepted configuration options for delay_filter - removed forgotten MAVE_LENGTH and PORT_MAVE_LENGTH macros Miroslav Lichvar (3): Add modular filter interface. Add median filter. Add options to configure delay filter. clock.c | 18 ++++----- config.c | 33 +++++++++++++++++ config.h | 3 ++ default.cfg | 2 + ds.h | 3 ++ filter.c | 49 +++++++++++++++++++++++++ filter.h | 64 ++++++++++++++++++++++++++++++++ filter_private.h | 33 +++++++++++++++++ gPTP.cfg | 2 + makefile | 5 ++- mave.c | 48 ++++++++++++++---------- mave.h | 12 +----- mmedian.c | 110 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ mmedian.h | 27 ++++++++++++++ port.c | 16 ++++---- ptp4l.8 | 9 +++++ ptp4l.c | 2 + 17 files changed, 388 insertions(+), 48 deletions(-) create mode 100644 filter.c create mode 100644 filter.h create mode 100644 filter_private.h create mode 100644 mmedian.c create mode 100644 mmedian.h -- 1.8.3.1 |
From: Miroslav L. <mli...@re...> - 2013-10-29 10:58:31
|
Similarly to the servo interface, allow multiple filters to be used for delay filtering. Convert mave to the new interface. Signed-off-by: Miroslav Lichvar <mli...@re...> --- clock.c | 16 +++++++------- filter.c | 46 +++++++++++++++++++++++++++++++++++++++++ filter.h | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ filter_private.h | 33 +++++++++++++++++++++++++++++ makefile | 5 +++-- mave.c | 48 +++++++++++++++++++++++++----------------- mave.h | 12 ++--------- port.c | 14 ++++++------- 8 files changed, 191 insertions(+), 46 deletions(-) create mode 100644 filter.c create mode 100644 filter.h create mode 100644 filter_private.h diff --git a/clock.c b/clock.c index 74d5c77..cf2f752 100644 --- a/clock.c +++ b/clock.c @@ -27,7 +27,7 @@ #include "clockadj.h" #include "clockcheck.h" #include "foreign.h" -#include "mave.h" +#include "filter.h" #include "missing.h" #include "msg.h" #include "phc.h" @@ -86,7 +86,7 @@ struct clock { enum servo_state servo_state; tmv_t master_offset; tmv_t path_delay; - struct mave *avg_delay; + struct filter *delay_filter; struct freq_estimator fest; struct time_status_np status; double nrr; @@ -121,7 +121,7 @@ void clock_destroy(struct clock *c) phc_close(c->clkid); } servo_destroy(c->servo); - mave_destroy(c->avg_delay); + filter_destroy(c->delay_filter); stats_destroy(c->stats.offset); stats_destroy(c->stats.freq); stats_destroy(c->stats.delay); @@ -632,9 +632,9 @@ struct clock *clock_create(int phc_index, struct interface *iface, int count, return NULL; } c->servo_state = SERVO_UNLOCKED; - c->avg_delay = mave_create(MAVE_LENGTH); - if (!c->avg_delay) { - pr_err("Failed to create moving average"); + c->delay_filter = filter_create(FILTER_MOVING_AVERAGE, MAVE_LENGTH); + if (!c->delay_filter) { + pr_err("Failed to create delay filter"); return NULL; } c->stats_interval = dds->stats_interval; @@ -992,7 +992,7 @@ void clock_path_delay(struct clock *c, struct timespec req, struct timestamp rx, pr_warning("c3 %10lld", c3); } - c->path_delay = mave_accumulate(c->avg_delay, pd); + c->path_delay = filter_sample(c->delay_filter, pd); c->cur.meanPathDelay = tmv_to_TimeInterval(c->path_delay); @@ -1170,7 +1170,7 @@ static void handle_state_decision_event(struct clock *c) if (!cid_eq(&best_id, &c->best_id)) { clock_freq_est_reset(c); - mave_reset(c->avg_delay); + filter_reset(c->delay_filter); c->t1 = tmv_zero(); c->t2 = tmv_zero(); c->path_delay = 0; diff --git a/filter.c b/filter.c new file mode 100644 index 0000000..65eb003 --- /dev/null +++ b/filter.c @@ -0,0 +1,46 @@ +/** + * @file filter.c + * @note Copyright (C) 2013 Miroslav Lichvar <mli...@re...> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include "filter_private.h" +#include "mave.h" + +struct filter *filter_create(enum filter_type type, int length) +{ + switch (type) { + case FILTER_MOVING_AVERAGE: + return mave_create(length); + default: + return NULL; + } +} + +void filter_destroy(struct filter *filter) +{ + filter->destroy(filter); +} + +tmv_t filter_sample(struct filter *filter, tmv_t sample) +{ + return filter->sample(filter, sample); +} + +void filter_reset(struct filter *filter) +{ + filter->reset(filter); +} diff --git a/filter.h b/filter.h new file mode 100644 index 0000000..4d7b370 --- /dev/null +++ b/filter.h @@ -0,0 +1,63 @@ +/** + * @file filter.h + * @brief Implements a generic filter interface. + * @note Copyright (C) 2013 Miroslav Lichvar <mli...@re...> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ +#ifndef HAVE_FILTER_H +#define HAVE_FILTER_H + +#include "tmv.h" + +/** Opaque type */ +struct filter; + +/** + * Defines the available filters. + */ +enum filter_type { + FILTER_MOVING_AVERAGE, +}; + +/** + * Create a new instance of a filter. + * @param type The type of the filter to create. + * @param length The filter's length. + * @return A pointer to a new filter on success, NULL otherwise. + */ +struct filter *filter_create(enum filter_type type, int length); + +/** + * Destroy an instance of a filter. + * @param filter Pointer to a filter obtained via @ref filter_create(). + */ +void filter_destroy(struct filter *filter); + +/** + * Feed a sample into a filter. + * @param filter Pointer to a filter obtained via @ref filter_create(). + * @param sample The input sample. + * @return The output value. + */ +tmv_t filter_sample(struct filter *filter, tmv_t sample); + +/** + * Reset a filter. + * @param filter Pointer to a filter obtained via @ref filter_create(). + */ +void filter_reset(struct filter *filter); + +#endif diff --git a/filter_private.h b/filter_private.h new file mode 100644 index 0000000..26062b4 --- /dev/null +++ b/filter_private.h @@ -0,0 +1,33 @@ +/** + * @file filter_private.h + * @note Copyright (C) 2013 Miroslav Lichvar <mli...@re...> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ +#ifndef HAVE_FILTER_PRIVATE_H +#define HAVE_FILTER_PRIVATE_H + +#include "tmv.h" +#include "contain.h" + +struct filter { + void (*destroy)(struct filter *filter); + + tmv_t (*sample)(struct filter *filter, tmv_t sample); + + void (*reset)(struct filter *filter); +}; + +#endif diff --git a/makefile b/makefile index 3f7ec90..5e9b009 100644 --- a/makefile +++ b/makefile @@ -24,8 +24,9 @@ CFLAGS = -Wall $(VER) $(incdefs) $(DEBUG) $(EXTRA_CFLAGS) LDLIBS = -lm -lrt $(EXTRA_LDFLAGS) PRG = ptp4l pmc phc2sys hwstamp_ctl OBJ = bmc.o clock.o clockadj.o clockcheck.o config.o fault.o \ - fsm.o ptp4l.o mave.o msg.o phc.o pi.o port.o print.o raw.o servo.o \ - sk.o stats.o tlv.o tmtab.o transport.o udp.o udp6.o uds.o util.o version.o + filter.o fsm.o mave.o msg.o phc.o pi.o port.o print.o \ + ptp4l.o raw.o servo.o sk.o stats.o tlv.o tmtab.o transport.o udp.o \ + udp6.o uds.o util.o version.o OBJECTS = $(OBJ) hwstamp_ctl.o phc2sys.o pmc.o pmc_common.o sysoff.o SRC = $(OBJECTS:.o=.c) diff --git a/mave.c b/mave.c index 7742f05..fd09e5a 100644 --- a/mave.c +++ b/mave.c @@ -20,8 +20,10 @@ #include <string.h> #include "mave.h" +#include "filter_private.h" struct mave { + struct filter filter; int cnt; int len; int index; @@ -29,30 +31,17 @@ struct mave { tmv_t *val; }; -struct mave *mave_create(int length) -{ - struct mave *m; - m = calloc(1, sizeof(*m)); - if (!m) { - return NULL; - } - m->val = calloc(1, length * sizeof(*m->val)); - if (!m->val) { - free(m); - return NULL; - } - m->len = length; - return m; -} - -void mave_destroy(struct mave *m) +static void mave_destroy(struct filter *filter) { + struct mave *m = container_of(filter, struct mave, filter); free(m->val); free(m); } -tmv_t mave_accumulate(struct mave *m, tmv_t val) +static tmv_t mave_accumulate(struct filter *filter, tmv_t val) { + struct mave *m = container_of(filter, struct mave, filter); + m->sum = tmv_sub(m->sum, m->val[m->index]); m->val[m->index] = val; m->index = (1 + m->index) % m->len; @@ -63,10 +52,31 @@ tmv_t mave_accumulate(struct mave *m, tmv_t val) return tmv_div(m->sum, m->cnt); } -void mave_reset(struct mave *m) +static void mave_reset(struct filter *filter) { + struct mave *m = container_of(filter, struct mave, filter); + m->cnt = 0; m->index = 0; m->sum = 0; memset(m->val, 0, m->len * sizeof(*m->val)); } + +struct filter *mave_create(int length) +{ + struct mave *m; + m = calloc(1, sizeof(*m)); + if (!m) { + return NULL; + } + m->filter.destroy = mave_destroy; + m->filter.sample = mave_accumulate; + m->filter.reset = mave_reset; + m->val = calloc(1, length * sizeof(*m->val)); + if (!m->val) { + free(m); + return NULL; + } + m->len = length; + return &m->filter; +} diff --git a/mave.h b/mave.h index 84241d4..8f170e4 100644 --- a/mave.h +++ b/mave.h @@ -20,16 +20,8 @@ #ifndef HAVE_MAVE_H #define HAVE_MAVE_H -#include "tmv.h" +#include "filter.h" -struct mave; - -struct mave *mave_create(int length); - -void mave_destroy(struct mave *m); - -tmv_t mave_accumulate(struct mave *m, tmv_t val); - -void mave_reset(struct mave *m); +struct filter *mave_create(int length); #endif diff --git a/port.c b/port.c index bd10d0a..e8448b8 100644 --- a/port.c +++ b/port.c @@ -25,7 +25,7 @@ #include "bmc.h" #include "clock.h" -#include "mave.h" +#include "filter.h" #include "missing.h" #include "msg.h" #include "port.h" @@ -82,7 +82,7 @@ struct port { } seqnum; struct tmtab tmtab; tmv_t peer_delay; - struct mave *avg_delay; + struct filter *delay_filter; int log_sync_interval; struct nrate_estimator nrate; unsigned int pdr_missing; @@ -1716,7 +1716,7 @@ calc: pd = tmv_sub(pd, c2); pd = tmv_div(pd, 2); - p->peer_delay = mave_accumulate(p->avg_delay, pd); + p->peer_delay = filter_sample(p->delay_filter, pd); p->peerMeanPathDelay = tmv_to_TimeInterval(p->peer_delay); @@ -1845,7 +1845,7 @@ void port_close(struct port *p) port_disable(p); } transport_destroy(p->trp); - mave_destroy(p->avg_delay); + filter_destroy(p->delay_filter); free(p); } @@ -2306,9 +2306,9 @@ struct port *port_open(int phc_index, p->delayMechanism = interface->dm; p->versionNumber = PTP_VERSION; - p->avg_delay = mave_create(PORT_MAVE_LENGTH); - if (!p->avg_delay) { - pr_err("Failed to create moving average"); + p->delay_filter = filter_create(FILTER_MOVING_AVERAGE, PORT_MAVE_LENGTH); + if (!p->delay_filter) { + pr_err("Failed to create delay filter"); transport_destroy(p->trp); free(p); return NULL; -- 1.8.3.1 |
From: Miroslav L. <mli...@re...> - 2013-10-29 10:58:31
|
Median filter has an advantage over moving average that it is much less sensitive to outliers. For instance, it allows much faster recovery from an external clock time step which happened between receiving sync message and sending delay_req message. The measured delay includes a large error, but the median is still a good estimate of the delay and the first step correction applied by the servo is right. In this implementation the median update has linear time complexity. Signed-off-by: Miroslav Lichvar <mli...@re...> --- filter.c | 3 ++ filter.h | 1 + makefile | 2 +- mmedian.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ mmedian.h | 27 +++++++++++++++ 5 files changed, 142 insertions(+), 1 deletion(-) create mode 100644 mmedian.c create mode 100644 mmedian.h diff --git a/filter.c b/filter.c index 65eb003..fceb29a 100644 --- a/filter.c +++ b/filter.c @@ -19,12 +19,15 @@ #include "filter_private.h" #include "mave.h" +#include "mmedian.h" struct filter *filter_create(enum filter_type type, int length) { switch (type) { case FILTER_MOVING_AVERAGE: return mave_create(length); + case FILTER_MOVING_MEDIAN: + return mmedian_create(length); default: return NULL; } diff --git a/filter.h b/filter.h index 4d7b370..5a196bc 100644 --- a/filter.h +++ b/filter.h @@ -30,6 +30,7 @@ struct filter; */ enum filter_type { FILTER_MOVING_AVERAGE, + FILTER_MOVING_MEDIAN, }; /** diff --git a/makefile b/makefile index 5e9b009..37cbaf9 100644 --- a/makefile +++ b/makefile @@ -24,7 +24,7 @@ CFLAGS = -Wall $(VER) $(incdefs) $(DEBUG) $(EXTRA_CFLAGS) LDLIBS = -lm -lrt $(EXTRA_LDFLAGS) PRG = ptp4l pmc phc2sys hwstamp_ctl OBJ = bmc.o clock.o clockadj.o clockcheck.o config.o fault.o \ - filter.o fsm.o mave.o msg.o phc.o pi.o port.o print.o \ + filter.o fsm.o mave.o mmedian.o msg.o phc.o pi.o port.o print.o \ ptp4l.o raw.o servo.o sk.o stats.o tlv.o tmtab.o transport.o udp.o \ udp6.o uds.o util.o version.o diff --git a/mmedian.c b/mmedian.c new file mode 100644 index 0000000..9185cbb --- /dev/null +++ b/mmedian.c @@ -0,0 +1,110 @@ +/** + * @file mmedian.c + * @note Copyright (C) 2013 Miroslav Lichvar <mli...@re...> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ +#include <stdlib.h> +#include <string.h> + +#include "mmedian.h" +#include "filter_private.h" + +struct mmedian { + struct filter filter; + int cnt; + int len; + int index; + /* Indices sorted by value. */ + int *order; + /* Values stored in circular buffer. */ + tmv_t *samples; +}; + +static void mmedian_destroy(struct filter *filter) +{ + struct mmedian *m = container_of(filter, struct mmedian, filter); + free(m->order); + free(m->samples); + free(m); +} + +static tmv_t mmedian_sample(struct filter *filter, tmv_t sample) +{ + struct mmedian *m = container_of(filter, struct mmedian, filter); + int i; + + m->samples[m->index] = sample; + if (m->cnt < m->len) { + m->cnt++; + } else { + /* Remove index of the replaced value from order. */ + for (i = 0; i < m->cnt; i++) + if (m->order[i] == m->index) + break; + for (; i + 1 < m->cnt; i++) + m->order[i] = m->order[i + 1]; + } + + /* Insert index of the new value to order. */ + for (i = m->cnt - 1; i > 0; i--) { + if (m->samples[m->order[i - 1]] <= m->samples[m->index]) + break; + m->order[i] = m->order[i - 1]; + } + m->order[i] = m->index; + + m->index = (1 + m->index) % m->len; + + if (m->cnt % 2) + return m->samples[m->order[m->cnt / 2]]; + else + return tmv_div(tmv_add(m->samples[m->order[m->cnt / 2 - 1]], + m->samples[m->order[m->cnt / 2]]), 2); +} + +static void mmedian_reset(struct filter *filter) +{ + struct mmedian *m = container_of(filter, struct mmedian, filter); + m->cnt = 0; + m->index = 0; +} + +struct filter *mmedian_create(int length) +{ + struct mmedian *m; + + if (length < 1) + return NULL; + m = calloc(1, sizeof(*m)); + if (!m) + return NULL; + m->filter.destroy = mmedian_destroy; + m->filter.sample = mmedian_sample; + m->filter.reset = mmedian_reset; + m->order = calloc(1, length * sizeof(*m->order)); + if (!m->order) { + free(m); + return NULL; + } + m->samples = calloc(1, length * sizeof(*m->samples)); + if (!m->samples) { + free(m->order); + free(m); + return NULL; + } + m->len = length; + return &m->filter; +} diff --git a/mmedian.h b/mmedian.h new file mode 100644 index 0000000..8fee0dc --- /dev/null +++ b/mmedian.h @@ -0,0 +1,27 @@ +/** + * @file mmedian.h + * @brief Implements a moving median. + * @note Copyright (C) 2013 Miroslav Lichvar <mli...@re...> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ +#ifndef HAVE_MMEDIAN_H +#define HAVE_MMEDIAN_H + +#include "filter.h" + +struct filter *mmedian_create(int length); + +#endif -- 1.8.3.1 |
From: Miroslav L. <mli...@re...> - 2013-10-29 10:58:32
|
Add new options delay_filter and delay_filter_length to select the filter and its length. They set both the clock delay filter and the port peer delay filter. The default is now moving median with 10 samples. Signed-off-by: Miroslav Lichvar <mli...@re...> --- clock.c | 4 ++-- config.c | 33 +++++++++++++++++++++++++++++++++ config.h | 3 +++ default.cfg | 2 ++ ds.h | 3 +++ gPTP.cfg | 2 ++ port.c | 4 ++-- ptp4l.8 | 9 +++++++++ ptp4l.c | 2 ++ 9 files changed, 58 insertions(+), 4 deletions(-) diff --git a/clock.c b/clock.c index cf2f752..1563ba9 100644 --- a/clock.c +++ b/clock.c @@ -41,7 +41,6 @@ #define CLK_N_PORTS (MAX_PORTS + 1) /* plus one for the UDS interface */ #define N_CLOCK_PFD (N_POLLFD + 1) /* one extra per port, for the fault timer */ -#define MAVE_LENGTH 10 #define POW2_41 ((double)(1ULL << 41)) #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0])) @@ -632,7 +631,8 @@ struct clock *clock_create(int phc_index, struct interface *iface, int count, return NULL; } c->servo_state = SERVO_UNLOCKED; - c->delay_filter = filter_create(FILTER_MOVING_AVERAGE, MAVE_LENGTH); + c->delay_filter = filter_create(dds->delay_filter, + dds->delay_filter_length); if (!c->delay_filter) { pr_err("Failed to create delay filter"); return NULL; diff --git a/config.c b/config.c index 7b7534f..c86ab01 100644 --- a/config.c +++ b/config.c @@ -159,6 +159,7 @@ static enum parser_result parse_port_setting(const char *option, int p) { enum parser_result r; + int val; r = parse_pod_setting(option, value, &cfg->iface[p].pod); if (r != NOT_PARSED) @@ -183,6 +184,21 @@ static enum parser_result parse_port_setting(const char *option, cfg->iface[p].dm = DM_P2P; else return BAD_VALUE; + + } else if (!strcmp(option, "delay_filter")) { + if (!strcasecmp("moving_average", value)) + cfg->iface[p].delay_filter = FILTER_MOVING_AVERAGE; + else if (!strcasecmp("moving_median", value)) + cfg->iface[p].delay_filter = FILTER_MOVING_MEDIAN; + else + return BAD_VALUE; + + } else if (!strcmp(option, "delay_filter_length")) { + r = get_ranged_int(value, &val, 1, INT_MAX); + if (r != PARSED_OK) + return r; + cfg->iface[p].delay_filter_length = val; + } else return NOT_PARSED; @@ -510,6 +526,20 @@ static enum parser_result parse_global_setting(const char *option, return r; cfg->dds.time_source = val; + } else if (!strcmp(option, "delay_filter")) { + if (!strcasecmp("moving_average", value)) + cfg->dds.delay_filter = FILTER_MOVING_AVERAGE; + else if (!strcasecmp("moving_median", value)) + cfg->dds.delay_filter = FILTER_MOVING_MEDIAN; + else + return BAD_VALUE; + + } else if (!strcmp(option, "delay_filter_length")) { + r = get_ranged_int(value, &val, 1, INT_MAX); + if (r != PARSED_OK) + return r; + cfg->dds.delay_filter_length = val; + } else return NOT_PARSED; @@ -667,6 +697,9 @@ int config_create_interface(char *name, struct config *cfg) sk_get_ts_info(name, &iface->ts_info); + iface->delay_filter = cfg->dds.delay_filter; + iface->delay_filter_length = cfg->dds.delay_filter_length; + cfg->nports++; return i; diff --git a/config.h b/config.h index 6c57bc9..a380037 100644 --- a/config.h +++ b/config.h @@ -22,6 +22,7 @@ #include "ds.h" #include "dm.h" +#include "filter.h" #include "transport.h" #include "servo.h" #include "sk.h" @@ -36,6 +37,8 @@ struct interface { enum transport_type transport; struct port_defaults pod; struct sk_ts_info ts_info; + enum filter_type delay_filter; + int delay_filter_length; }; #define CFG_IGNORE_DM (1 << 0) diff --git a/default.cfg b/default.cfg index 8794596..80c20b9 100644 --- a/default.cfg +++ b/default.cfg @@ -67,6 +67,8 @@ uds_address /var/run/ptp4l network_transport UDPv4 delay_mechanism E2E time_stamping hardware +delay_filter moving_median +delay_filter_length 10 # # Clock description # diff --git a/ds.h b/ds.h index e8e94ad..6e2ac61 100644 --- a/ds.h +++ b/ds.h @@ -22,6 +22,7 @@ #include "ddt.h" #include "fault.h" +#include "filter.h" /* clock data sets */ @@ -57,6 +58,8 @@ struct default_ds { int sanity_freq_limit; int time_source; struct clock_description clock_desc; + enum filter_type delay_filter; + int delay_filter_length; }; struct dataset { diff --git a/gPTP.cfg b/gPTP.cfg index a94bd01..3c0842c 100644 --- a/gPTP.cfg +++ b/gPTP.cfg @@ -65,3 +65,5 @@ uds_address /var/run/ptp4l network_transport L2 delay_mechanism P2P time_stamping hardware +delay_filter moving_median +delay_filter_length 10 diff --git a/port.c b/port.c index e8448b8..94c5cac 100644 --- a/port.c +++ b/port.c @@ -37,7 +37,6 @@ #include "util.h" #define ALLOWED_LOST_RESPONSES 3 -#define PORT_MAVE_LENGTH 10 enum syfu_state { SF_EMPTY, @@ -2306,7 +2305,8 @@ struct port *port_open(int phc_index, p->delayMechanism = interface->dm; p->versionNumber = PTP_VERSION; - p->delay_filter = filter_create(FILTER_MOVING_AVERAGE, PORT_MAVE_LENGTH); + p->delay_filter = filter_create(interface->delay_filter, + interface->delay_filter_length); if (!p->delay_filter) { pr_err("Failed to create delay filter"); transport_destroy(p->trp); diff --git a/ptp4l.8 b/ptp4l.8 index 01438b3..c59b6e7 100644 --- a/ptp4l.8 +++ b/ptp4l.8 @@ -192,6 +192,15 @@ The default is UDPv4. .B neighborPropDelayThresh Upper limit for peer delay in nanoseconds. If the estimated peer delay is greater than this value the port is marked as not 802.1AS capable. +.TP +.B delay_filter +Select the algorithm used to filter the measured delay and peer delay. Possible +values are moving_average and moving_median. +The default is moving_median. +.TP +.B delay_filter_length +The length of the delay filter in samples. +The default is 10. .SH PROGRAM AND CLOCK OPTIONS diff --git a/ptp4l.c b/ptp4l.c index c828bb4..5d8749a 100644 --- a/ptp4l.c +++ b/ptp4l.c @@ -71,6 +71,8 @@ static struct config cfg_settings = { .userDescription = { .max_symbols = 128 }, .manufacturerIdentity = { 0, 0, 0 }, }, + .delay_filter = FILTER_MOVING_MEDIAN, + .delay_filter_length = 10, }, .pod = { -- 1.8.3.1 |
From: Richard C. <ric...@gm...> - 2013-10-30 21:14:07
|
On Tue, Oct 29, 2013 at 11:58:17AM +0100, Miroslav Lichvar wrote: > Changes in v3: > - fixed declaration of filter_sample() > - renamed accepted configuration options for delay_filter > - removed forgotten MAVE_LENGTH and PORT_MAVE_LENGTH macros > > Miroslav Lichvar (3): > Add modular filter interface. > Add median filter. > Add options to configure delay filter. Series applied. Thanks, Richard |