Message ID | 20171006162757.16574-1-mjbshaw@gmail.com |
---|---|
State | New |
Headers | show |
Thanks for the patch. Le quintidi 15 vendémiaire, an CCXXVI, Michael Bradshaw a écrit : > From: Michael Bradshaw <mjbshaw@google.com> > > Signed-off-by: Michael Bradshaw <mjbshaw@google.com> > --- > doc/filters.texi | 13 ++++++++++++- > libavfilter/asrc_sine.c | 17 +++++++++++++---- > 2 files changed, 25 insertions(+), 5 deletions(-) > > diff --git a/doc/filters.texi b/doc/filters.texi > index 57189c77b0..ec1c335950 100644 > --- a/doc/filters.texi > +++ b/doc/filters.texi > @@ -4624,7 +4624,18 @@ Set the carrier frequency. Default is 440 Hz. > > @item beep_factor, b > Enable a periodic beep every second with frequency @var{beep_factor} times > -the carrier frequency. Default is 0, meaning the beep is disabled. > +the carrier frequency. Default is 0, meaning the beep is disabled. If > +@var{frequency} is 0, this value is interpreted as the beep frequency (in Hertz) > +(rather than a multiplier of the @var{frequency}). > + > +@item beep_delay > +The delay for the first beep, in seconds. Default is 0. > + > +@item beep_period > +The time beriod between two beeps, in seconds. Default is 1. > + > +@item beep_duration > +The duration of a beep, in seconds. Default is 0.04. > > @item sample_rate, r > Specify the sample rate, default is 44100. > diff --git a/libavfilter/asrc_sine.c b/libavfilter/asrc_sine.c > index 3a87210b4b..643952792f 100644 > --- a/libavfilter/asrc_sine.c > +++ b/libavfilter/asrc_sine.c > @@ -32,6 +32,9 @@ typedef struct SineContext { > const AVClass *class; > double frequency; > double beep_factor; > + double beep_delay; > + double beep_period_opt; > + double beep_duration; > char *samples_per_frame; > AVExpr *samples_per_frame_expr; > int sample_rate; > @@ -71,6 +74,9 @@ static const AVOption sine_options[] = { > OPT_DBL("f", frequency, 440, 0, DBL_MAX, "set the sine frequency",), > OPT_DBL("beep_factor", beep_factor, 0, 0, DBL_MAX, "set the beep frequency factor",), > OPT_DBL("b", beep_factor, 0, 0, DBL_MAX, "set the beep frequency factor",), > + OPT_DBL("beep_delay", beep_delay, 0, 0, DBL_MAX, "set the delay for the first beep",), > + OPT_DBL("beep_period", beep_period_opt, 1, DBL_MIN, DBL_MAX, "set the gap between beeps",), > + OPT_DBL("beep_duration", beep_duration, 0.04, DBL_MIN, DBL_MAX, "set the duration of a beep",), I think these should use OPT_DUR / AV_OPT_TYPE_DURATION rather than doubles. Also, DBL_MIN seems strange: I do not think a negative value makes sense. > OPT_INT("sample_rate", sample_rate, 44100, 1, INT_MAX, "set the sample rate",), > OPT_INT("r", sample_rate, 44100, 1, INT_MAX, "set the sample rate",), > OPT_DUR("duration", duration, 0, 0, INT64_MAX, "set the audio duration",), > @@ -152,10 +158,13 @@ static av_cold int init(AVFilterContext *ctx) > make_sin_table(sine->sin); > > if (sine->beep_factor) { > - sine->beep_period = sine->sample_rate; > - sine->beep_length = sine->beep_period / 25; > - sine->dphi_beep = ldexp(sine->beep_factor * sine->frequency, 32) / > - sine->sample_rate + 0.5; > + unsigned beep_start = sine->beep_delay * sine->sample_rate; > + double beep_frequency = (sine->frequency ? sine->frequency : 1.0) * > + sine->beep_factor; > + sine->beep_period = sine->beep_period_opt * sine->sample_rate; With integer durations, av_rescale() would be better. > + sine->beep_index = (sine->beep_period - beep_start) % sine->beep_period; I think this will produce strange results if beep_start is greater than beep_period. Maybe document the limitation, or adjust the arithmetic. > + sine->beep_length = sine->beep_duration * sine->sample_rate; > + sine->dphi_beep = ldexp(beep_frequency, 32) / sine->sample_rate + 0.5; > } > > ret = av_expr_parse(&sine->samples_per_frame_expr, Regards,
Sorry for the long delay in my response! On Sun, Oct 8, 2017 at 11:14 AM, Nicolas George <george@nsup.org> wrote: > > Le quintidi 15 vendémiaire, an CCXXVI, Michael Bradshaw a écrit : > > + OPT_DBL("beep_delay", beep_delay, 0, 0, > DBL_MAX, "set the delay for the first beep",), > > + OPT_DBL("beep_period", beep_period_opt, 1, DBL_MIN, > DBL_MAX, "set the gap between beeps",), > > + OPT_DBL("beep_duration", beep_duration, 0.04, DBL_MIN, > DBL_MAX, "set the duration of a beep",), > > I think these should use OPT_DUR / AV_OPT_TYPE_DURATION rather than > doubles. Also, DBL_MIN seems strange: I do not think a negative value > makes sense. > Fixed. DBL_MIN is actually positive. I use it to prevent the user from setting the beep period or duration to 0. But I've changed it to now use 1. > OPT_INT("sample_rate", sample_rate, 44100, 1, INT_MAX, > "set the sample rate",), > > OPT_INT("r", sample_rate, 44100, 1, > INT_MAX, "set the sample rate",), > > OPT_DUR("duration", duration, 0, 0, > INT64_MAX, "set the audio duration",), > > @@ -152,10 +158,13 @@ static av_cold int init(AVFilterContext *ctx) > > make_sin_table(sine->sin); > > > > if (sine->beep_factor) { > > - sine->beep_period = sine->sample_rate; > > - sine->beep_length = sine->beep_period / 25; > > - sine->dphi_beep = ldexp(sine->beep_factor * sine->frequency, > 32) / > > - sine->sample_rate + 0.5; > > + unsigned beep_start = sine->beep_delay * sine->sample_rate; > > + double beep_frequency = (sine->frequency ? sine->frequency : > 1.0) * > > + sine->beep_factor; > > > + sine->beep_period = sine->beep_period_opt * sine->sample_rate; > > With integer durations, av_rescale() would be better. > Fixed. > + sine->beep_index = (sine->beep_period - beep_start) % > sine->beep_period; > > I think this will produce strange results if beep_start is greater than > beep_period. Maybe document the limitation, or adjust the arithmetic. Good point. I fixed the arithmetic so now the beep_start can be greater than the beep_period. Attached is an updated patch.
diff --git a/doc/filters.texi b/doc/filters.texi index 57189c77b0..ec1c335950 100644 --- a/doc/filters.texi +++ b/doc/filters.texi @@ -4624,7 +4624,18 @@ Set the carrier frequency. Default is 440 Hz. @item beep_factor, b Enable a periodic beep every second with frequency @var{beep_factor} times -the carrier frequency. Default is 0, meaning the beep is disabled. +the carrier frequency. Default is 0, meaning the beep is disabled. If +@var{frequency} is 0, this value is interpreted as the beep frequency (in Hertz) +(rather than a multiplier of the @var{frequency}). + +@item beep_delay +The delay for the first beep, in seconds. Default is 0. + +@item beep_period +The time beriod between two beeps, in seconds. Default is 1. + +@item beep_duration +The duration of a beep, in seconds. Default is 0.04. @item sample_rate, r Specify the sample rate, default is 44100. diff --git a/libavfilter/asrc_sine.c b/libavfilter/asrc_sine.c index 3a87210b4b..643952792f 100644 --- a/libavfilter/asrc_sine.c +++ b/libavfilter/asrc_sine.c @@ -32,6 +32,9 @@ typedef struct SineContext { const AVClass *class; double frequency; double beep_factor; + double beep_delay; + double beep_period_opt; + double beep_duration; char *samples_per_frame; AVExpr *samples_per_frame_expr; int sample_rate; @@ -71,6 +74,9 @@ static const AVOption sine_options[] = { OPT_DBL("f", frequency, 440, 0, DBL_MAX, "set the sine frequency",), OPT_DBL("beep_factor", beep_factor, 0, 0, DBL_MAX, "set the beep frequency factor",), OPT_DBL("b", beep_factor, 0, 0, DBL_MAX, "set the beep frequency factor",), + OPT_DBL("beep_delay", beep_delay, 0, 0, DBL_MAX, "set the delay for the first beep",), + OPT_DBL("beep_period", beep_period_opt, 1, DBL_MIN, DBL_MAX, "set the gap between beeps",), + OPT_DBL("beep_duration", beep_duration, 0.04, DBL_MIN, DBL_MAX, "set the duration of a beep",), OPT_INT("sample_rate", sample_rate, 44100, 1, INT_MAX, "set the sample rate",), OPT_INT("r", sample_rate, 44100, 1, INT_MAX, "set the sample rate",), OPT_DUR("duration", duration, 0, 0, INT64_MAX, "set the audio duration",), @@ -152,10 +158,13 @@ static av_cold int init(AVFilterContext *ctx) make_sin_table(sine->sin); if (sine->beep_factor) { - sine->beep_period = sine->sample_rate; - sine->beep_length = sine->beep_period / 25; - sine->dphi_beep = ldexp(sine->beep_factor * sine->frequency, 32) / - sine->sample_rate + 0.5; + unsigned beep_start = sine->beep_delay * sine->sample_rate; + double beep_frequency = (sine->frequency ? sine->frequency : 1.0) * + sine->beep_factor; + sine->beep_period = sine->beep_period_opt * sine->sample_rate; + sine->beep_index = (sine->beep_period - beep_start) % sine->beep_period; + sine->beep_length = sine->beep_duration * sine->sample_rate; + sine->dphi_beep = ldexp(beep_frequency, 32) / sine->sample_rate + 0.5; } ret = av_expr_parse(&sine->samples_per_frame_expr,