[FFmpeg-devel] [PATCH] lavf: add more beep options to sine asrc

[Michael Bradshaw]

Sorry for the long delay in my response!

On Sun, Oct 8, 2017 at 11:14 AM, Nicolas George <george at 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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-lavf-add-more-beep-options-to-sine-asrc.patch
Type: application/octet-stream
Size: 4647 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20171018/75449ea1/attachment.obj>