[FFmpeg-devel] [PATCH] libavdevice: JACK demuxer

[Michael Niedermayer]

On Mon, Mar 02, 2009 at 09:22:31PM +0100, Olivier Guilyardi wrote:
> Attached: timefilter libavformat patch version 0.2

[...]
> +#include <stdlib.h>
> +#include <math.h>
> +#include <sys/time.h>
> +#include <time.h>
> +
> +#include "timefilter.h"
> +
> +#define PI 3.14159265358979323846

other parts of lav* simply use M_PI without defining it ...


> +
> +struct TimeFilter {
> +
> +    /// Delay Locked Loop data. These variables refer to mathematical
> +    /// concepts described in: http://www.kokkinizita.net/papers/usingdll.pdf
> +    double tper;
> +    double b;
> +    double c;
> +    double e2;
> +    double t0;
> +    double t1;

please use some english words for these variables


[...]
> +TimeFilter * ff_timefilter_new(double period, double bandwidth)
> +{
> +    double o;
> +    TimeFilter *self = calloc(1, sizeof(TimeFilter));

code in lav* uses av_malloz()


> +    self->tper = period;
> +    o = 2 * PI * bandwidth * self->tper;
> +    self->b = sqrt(2 * o);
> +    self->c = o * o;

> +    self->t0 = 0;

useless


[...]
> +void ff_timefilter_update(TimeFilter *self, double system_time)
> +{
> +    double e;
> +
> +    if (!self->t0) {
> +        // init loop

> +        self->e2 = self->tper;

this is incorrect in case of reset


> +        self->t0 = system_time;
> +        self->t1 = self->t0 + self->e2;
> +
> +        // init stats
> +        self->device_time = system_time;
> +        self->system_period_error = self->filter_period_error = 0;
> +        self->ncycles = 0;

id put all of above in reset and call reset from new


[...]
> +/**
> + * Various statistics as reported by ff_timefilter_stats()
> + *
> + * All values are in micro-seconds
> + */
> +typedef struct TimeFilterStats {
> +    double filter_time;     /** last filtered time (same as ff_timefilter_read()) */
> +    double filter_jitter;   /** last jitter between filtered and device clocks */
> +    double filter_drift;    /** total drift between filtered and system clocks */
> +    double next_filter_time;/** estimate of the next filtered time */
> +
> +    double system_time;     /** last system time */
> +    double system_jitter;   /** last jitter between system and device clocks */
> +
> +    double device_time;     /** last device time */
> +    double device_drift;    /** total drift between device and system clocks */
> +} TimeFilterStats;

what use are these statistics except for debuging?
And i surely agree they are very valuable for debug but they dont
seem so usefull for normal use.


> +
> +/**
> + * Create a new DLL time filter
> + *
> + * Period must be the device cycle duration in micro-seconds. For example, at

considering that we are working with floats, the natural base unit is
seconds not micro-
besides the unit shouldnt matter for the filter


> + * 44.1Hz and a buffer size of 512 frames, period = 512 / 44100. The filter

> + * only works if the cycle duration is fixed.

this should be trivial to fix


> + *
> + * The bandwidth is up to you to choose. Smaller values will filter out more
> + * of the jitter, but also take a longer time for the loop to settle. A good
> + * starting point is something between 0.3 and 3 Hz.

the filter factors should be adjusted at the begin 


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In a rich man's house there is no place to spit but his face.
-- Diogenes of Sinope
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090303/e1668308/attachment.pgp>