| Message ID | 20180930204513.26772-1-cus@passwd.hu |
|---|---|
| State | Accepted |
| Commit | 0e9a09793a98a416ed86f7be0ed89dc10e9734a5 |
| Headers | show |
On Mon, Oct 1, 2018 at 2:15 AM Marton Balint <cus@passwd.hu> wrote: > > + { "duration", "set cross fade duration", > OFFSET(duration), AV_OPT_TYPE_DURATION, {.i64 = 0. }, 0, 60000000, > FLAGS }, > + { "d", "set cross fade duration", > OFFSET(duration), AV_OPT_TYPE_DURATION, {.i64 = 0. }, 0, 60000000, > FLAGS }, > What's the rationale for this non-standard value? Gyan
On 9/30/18, Marton Balint <cus@passwd.hu> wrote: > Signed-off-by: Marton Balint <cus@passwd.hu> > --- > libavfilter/af_afade.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > Why? Value is too big.
On Mon, 1 Oct 2018, Paul B Mahol wrote: > On 9/30/18, Marton Balint <cus@passwd.hu> wrote: >> Signed-off-by: Marton Balint <cus@passwd.hu> >> --- >> libavfilter/af_afade.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> > > Why? > > Value is too big. AV_OPT_TYPE_DURATION is microsecond precision, therefore 60 is 60 usecs. Probably you meant 60 seconds as limit. That is what the patch fixes. Regards, Marton
On Mon, 1 Oct 2018, Gyan wrote: > On Mon, Oct 1, 2018 at 2:15 AM Marton Balint <cus@passwd.hu> wrote: > >> >> + { "duration", "set cross fade duration", >> OFFSET(duration), AV_OPT_TYPE_DURATION, {.i64 = 0. }, 0, 60000000, >> FLAGS }, >> + { "d", "set cross fade duration", >> OFFSET(duration), AV_OPT_TYPE_DURATION, {.i64 = 0. }, 0, 60000000, >> FLAGS }, >> > > What's the rationale for this non-standard value? Maybe Paul can answer this. Regards, Marton
On 10/1/18, Marton Balint <cus@passwd.hu> wrote: > > > On Mon, 1 Oct 2018, Paul B Mahol wrote: > >> On 9/30/18, Marton Balint <cus@passwd.hu> wrote: >>> Signed-off-by: Marton Balint <cus@passwd.hu> >>> --- >>> libavfilter/af_afade.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >> >> Why? >> >> Value is too big. > > AV_OPT_TYPE_DURATION is microsecond precision, therefore 60 is 60 usecs. > Probably you meant 60 seconds as limit. That is what the patch fixes. I do not think so. Duration is interpreted as seconds, look how variable is used in code.
On Mon, 1 Oct 2018, Paul B Mahol wrote: > On 10/1/18, Marton Balint <cus@passwd.hu> wrote: >> >> >> On Mon, 1 Oct 2018, Paul B Mahol wrote: >> >>> On 9/30/18, Marton Balint <cus@passwd.hu> wrote: >>>> Signed-off-by: Marton Balint <cus@passwd.hu> >>>> --- >>>> libavfilter/af_afade.c | 4 ++-- >>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>> >>> >>> Why? >>> >>> Value is too big. >> >> AV_OPT_TYPE_DURATION is microsecond precision, therefore 60 is 60 usecs. >> Probably you meant 60 seconds as limit. That is what the patch fixes. > > I do not think so. Duration is interpreted as seconds, look how > variable is used in code. As far as I see duration is rescaled from AV_TIME_BASE (microseconds) to sample_rate time base: if (s->duration) s->nb_samples = av_rescale(s->duration, outlink->sample_rate, AV_TIME_BASE); Duration is parsed as seconds (can be fractional as well), but internally it is stored as microseconds (int64). Another example is the f_realtime filter where the sleep time limit default according to the documentation is 2 seconds and it is expressed as 2000000 in the code. Regards, Marton
On 10/1/18, Marton Balint <cus@passwd.hu> wrote: > > > On Mon, 1 Oct 2018, Paul B Mahol wrote: > >> On 10/1/18, Marton Balint <cus@passwd.hu> wrote: >>> >>> >>> On Mon, 1 Oct 2018, Paul B Mahol wrote: >>> >>>> On 9/30/18, Marton Balint <cus@passwd.hu> wrote: >>>>> Signed-off-by: Marton Balint <cus@passwd.hu> >>>>> --- >>>>> libavfilter/af_afade.c | 4 ++-- >>>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>>> >>>> >>>> Why? >>>> >>>> Value is too big. >>> >>> AV_OPT_TYPE_DURATION is microsecond precision, therefore 60 is 60 usecs. >>> Probably you meant 60 seconds as limit. That is what the patch fixes. >> >> I do not think so. Duration is interpreted as seconds, look how >> variable is used in code. > > As far as I see duration is rescaled from AV_TIME_BASE (microseconds) to > sample_rate time base: > > if (s->duration) > s->nb_samples = av_rescale(s->duration, outlink->sample_rate, > AV_TIME_BASE); > > Duration is parsed as seconds (can be fractional as well), but internally > it is stored as microseconds (int64). Another example is the f_realtime > filter where the sleep time limit default according to the documentation > is 2 seconds and it is expressed as 2000000 in the code. Yes, and current patch is wrong. Simply exceding max value is not proper fix.
On Mon, 1 Oct 2018, Paul B Mahol wrote: > On 10/1/18, Marton Balint <cus@passwd.hu> wrote: >> >> >> On Mon, 1 Oct 2018, Paul B Mahol wrote: >> >>> On 10/1/18, Marton Balint <cus@passwd.hu> wrote: >>>> >>>> >>>> On Mon, 1 Oct 2018, Paul B Mahol wrote: >>>> >>>>> On 9/30/18, Marton Balint <cus@passwd.hu> wrote: >>>>>> Signed-off-by: Marton Balint <cus@passwd.hu> >>>>>> --- >>>>>> libavfilter/af_afade.c | 4 ++-- >>>>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>>>> >>>>> >>>>> Why? >>>>> >>>>> Value is too big. >>>> >>>> AV_OPT_TYPE_DURATION is microsecond precision, therefore 60 is 60 usecs. >>>> Probably you meant 60 seconds as limit. That is what the patch fixes. >>> >>> I do not think so. Duration is interpreted as seconds, look how >>> variable is used in code. >> >> As far as I see duration is rescaled from AV_TIME_BASE (microseconds) to >> sample_rate time base: >> >> if (s->duration) >> s->nb_samples = av_rescale(s->duration, outlink->sample_rate, >> AV_TIME_BASE); >> >> Duration is parsed as seconds (can be fractional as well), but internally >> it is stored as microseconds (int64). Another example is the f_realtime >> filter where the sleep time limit default according to the documentation >> is 2 seconds and it is expressed as 2000000 in the code. > > Yes, and current patch is wrong. Simply exceding max value is not proper fix. What is the proper fix then? Regards, Marton
On 10/1/18, Marton Balint <cus@passwd.hu> wrote: > > > On Mon, 1 Oct 2018, Paul B Mahol wrote: > >> On 10/1/18, Marton Balint <cus@passwd.hu> wrote: >>> >>> >>> On Mon, 1 Oct 2018, Paul B Mahol wrote: >>> >>>> On 10/1/18, Marton Balint <cus@passwd.hu> wrote: >>>>> >>>>> >>>>> On Mon, 1 Oct 2018, Paul B Mahol wrote: >>>>> >>>>>> On 9/30/18, Marton Balint <cus@passwd.hu> wrote: >>>>>>> Signed-off-by: Marton Balint <cus@passwd.hu> >>>>>>> --- >>>>>>> libavfilter/af_afade.c | 4 ++-- >>>>>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>>>>> >>>>>> >>>>>> Why? >>>>>> >>>>>> Value is too big. >>>>> >>>>> AV_OPT_TYPE_DURATION is microsecond precision, therefore 60 is 60 >>>>> usecs. >>>>> Probably you meant 60 seconds as limit. That is what the patch fixes. >>>> >>>> I do not think so. Duration is interpreted as seconds, look how >>>> variable is used in code. >>> >>> As far as I see duration is rescaled from AV_TIME_BASE (microseconds) to >>> sample_rate time base: >>> >>> if (s->duration) >>> s->nb_samples = av_rescale(s->duration, outlink->sample_rate, >>> AV_TIME_BASE); >>> >>> Duration is parsed as seconds (can be fractional as well), but internally >>> it is stored as microseconds (int64). Another example is the f_realtime >>> filter where the sleep time limit default according to the documentation >>> is 2 seconds and it is expressed as 2000000 in the code. >> >> Yes, and current patch is wrong. Simply exceding max value is not proper >> fix. > > What is the proper fix then? Interpreting duration correctly, like trim filters do.
On Mon, 1 Oct 2018, Paul B Mahol wrote: > On 10/1/18, Marton Balint <cus@passwd.hu> wrote: >> >> >> On Mon, 1 Oct 2018, Paul B Mahol wrote: >> >>> On 10/1/18, Marton Balint <cus@passwd.hu> wrote: >>>> >>>> >>>> On Mon, 1 Oct 2018, Paul B Mahol wrote: >>>> >>>>> On 10/1/18, Marton Balint <cus@passwd.hu> wrote: >>>>>> >>>>>> >>>>>> On Mon, 1 Oct 2018, Paul B Mahol wrote: >>>>>> >>>>>>> On 9/30/18, Marton Balint <cus@passwd.hu> wrote: >>>>>>>> Signed-off-by: Marton Balint <cus@passwd.hu> >>>>>>>> --- >>>>>>>> libavfilter/af_afade.c | 4 ++-- >>>>>>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>>>>>> >>>>>>> >>>>>>> Why? >>>>>>> >>>>>>> Value is too big. >>>>>> >>>>>> AV_OPT_TYPE_DURATION is microsecond precision, therefore 60 is 60 >>>>>> usecs. >>>>>> Probably you meant 60 seconds as limit. That is what the patch fixes. >>>>> >>>>> I do not think so. Duration is interpreted as seconds, look how >>>>> variable is used in code. >>>> >>>> As far as I see duration is rescaled from AV_TIME_BASE (microseconds) to >>>> sample_rate time base: >>>> >>>> if (s->duration) >>>> s->nb_samples = av_rescale(s->duration, outlink->sample_rate, >>>> AV_TIME_BASE); >>>> >>>> Duration is parsed as seconds (can be fractional as well), but internally >>>> it is stored as microseconds (int64). Another example is the f_realtime >>>> filter where the sleep time limit default according to the documentation >>>> is 2 seconds and it is expressed as 2000000 in the code. >>> >>> Yes, and current patch is wrong. Simply exceding max value is not proper >>> fix. >> >> What is the proper fix then? > > Interpreting duration correctly, like trim filters do. Somebody help me, because I truly don't get it. 1) OPT_TYPE_DURATION default value is given in the .i64 field in microseconds. 2) OPT_TYPE_DURATION min/max value is given in the min/max fields in microseconds. Do you disagree with any of these? If yes, then what do you suggest? Thanks, Marton
On 10/1/18, Marton Balint <cus@passwd.hu> wrote: > > > On Mon, 1 Oct 2018, Paul B Mahol wrote: > >> On 10/1/18, Marton Balint <cus@passwd.hu> wrote: >>> >>> >>> On Mon, 1 Oct 2018, Paul B Mahol wrote: >>> >>>> On 10/1/18, Marton Balint <cus@passwd.hu> wrote: >>>>> >>>>> >>>>> On Mon, 1 Oct 2018, Paul B Mahol wrote: >>>>> >>>>>> On 10/1/18, Marton Balint <cus@passwd.hu> wrote: >>>>>>> >>>>>>> >>>>>>> On Mon, 1 Oct 2018, Paul B Mahol wrote: >>>>>>> >>>>>>>> On 9/30/18, Marton Balint <cus@passwd.hu> wrote: >>>>>>>>> Signed-off-by: Marton Balint <cus@passwd.hu> >>>>>>>>> --- >>>>>>>>> libavfilter/af_afade.c | 4 ++-- >>>>>>>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>>>>>>> >>>>>>>> >>>>>>>> Why? >>>>>>>> >>>>>>>> Value is too big. >>>>>>> >>>>>>> AV_OPT_TYPE_DURATION is microsecond precision, therefore 60 is 60 >>>>>>> usecs. >>>>>>> Probably you meant 60 seconds as limit. That is what the patch fixes. >>>>>> >>>>>> I do not think so. Duration is interpreted as seconds, look how >>>>>> variable is used in code. >>>>> >>>>> As far as I see duration is rescaled from AV_TIME_BASE (microseconds) >>>>> to >>>>> sample_rate time base: >>>>> >>>>> if (s->duration) >>>>> s->nb_samples = av_rescale(s->duration, outlink->sample_rate, >>>>> AV_TIME_BASE); >>>>> >>>>> Duration is parsed as seconds (can be fractional as well), but >>>>> internally >>>>> it is stored as microseconds (int64). Another example is the f_realtime >>>>> filter where the sleep time limit default according to the >>>>> documentation >>>>> is 2 seconds and it is expressed as 2000000 in the code. >>>> >>>> Yes, and current patch is wrong. Simply exceding max value is not proper >>>> fix. >>> >>> What is the proper fix then? >> >> Interpreting duration correctly, like trim filters do. > > Somebody help me, because I truly don't get it. > > 1) OPT_TYPE_DURATION default value is given in the .i64 field in > microseconds. > > 2) OPT_TYPE_DURATION min/max value is given in the min/max fields in > microseconds. > > Do you disagree with any of these? If yes, then what do you suggest? No, I do not disagree with any of those above. But patch is incomplete.
On Mon, 1 Oct 2018, Paul B Mahol wrote: > On 10/1/18, Marton Balint <cus@passwd.hu> wrote: >> >> >> On Mon, 1 Oct 2018, Paul B Mahol wrote: >> >>> On 10/1/18, Marton Balint <cus@passwd.hu> wrote: >>>> >>>> >>>> On Mon, 1 Oct 2018, Paul B Mahol wrote: >>>> >>>>> On 10/1/18, Marton Balint <cus@passwd.hu> wrote: >>>>>> >>>>>> >>>>>> On Mon, 1 Oct 2018, Paul B Mahol wrote: >>>>>> >>>>>>> On 10/1/18, Marton Balint <cus@passwd.hu> wrote: >>>>>>>> >>>>>>>> >>>>>>>> On Mon, 1 Oct 2018, Paul B Mahol wrote: >>>>>>>> >>>>>>>>> On 9/30/18, Marton Balint <cus@passwd.hu> wrote: >>>>>>>>>> Signed-off-by: Marton Balint <cus@passwd.hu> >>>>>>>>>> --- >>>>>>>>>> libavfilter/af_afade.c | 4 ++-- >>>>>>>>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>>>>>>>> >>>>>>>>> >>>>>>>>> Why? >>>>>>>>> >>>>>>>>> Value is too big. >>>>>>>> >>>>>>>> AV_OPT_TYPE_DURATION is microsecond precision, therefore 60 is 60 >>>>>>>> usecs. >>>>>>>> Probably you meant 60 seconds as limit. That is what the patch fixes. >>>>>>> >>>>>>> I do not think so. Duration is interpreted as seconds, look how >>>>>>> variable is used in code. >>>>>> >>>>>> As far as I see duration is rescaled from AV_TIME_BASE (microseconds) >>>>>> to >>>>>> sample_rate time base: >>>>>> >>>>>> if (s->duration) >>>>>> s->nb_samples = av_rescale(s->duration, outlink->sample_rate, >>>>>> AV_TIME_BASE); >>>>>> >>>>>> Duration is parsed as seconds (can be fractional as well), but >>>>>> internally >>>>>> it is stored as microseconds (int64). Another example is the f_realtime >>>>>> filter where the sleep time limit default according to the >>>>>> documentation >>>>>> is 2 seconds and it is expressed as 2000000 in the code. >>>>> >>>>> Yes, and current patch is wrong. Simply exceding max value is not proper >>>>> fix. >>>> >>>> What is the proper fix then? >>> >>> Interpreting duration correctly, like trim filters do. >> >> Somebody help me, because I truly don't get it. >> >> 1) OPT_TYPE_DURATION default value is given in the .i64 field in >> microseconds. >> >> 2) OPT_TYPE_DURATION min/max value is given in the min/max fields in >> microseconds. >> >> Do you disagree with any of these? If yes, then what do you suggest? > > No, I do not disagree with any of those above. But patch is incomplete. Post/push a better patch then please. Thanks, Marton
On 10/1/18, Marton Balint <cus@passwd.hu> wrote: > > > On Mon, 1 Oct 2018, Paul B Mahol wrote: > >> On 10/1/18, Marton Balint <cus@passwd.hu> wrote: >>> >>> >>> On Mon, 1 Oct 2018, Paul B Mahol wrote: >>> >>>> On 10/1/18, Marton Balint <cus@passwd.hu> wrote: >>>>> >>>>> >>>>> On Mon, 1 Oct 2018, Paul B Mahol wrote: >>>>> >>>>>> On 10/1/18, Marton Balint <cus@passwd.hu> wrote: >>>>>>> >>>>>>> >>>>>>> On Mon, 1 Oct 2018, Paul B Mahol wrote: >>>>>>> >>>>>>>> On 10/1/18, Marton Balint <cus@passwd.hu> wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> On Mon, 1 Oct 2018, Paul B Mahol wrote: >>>>>>>>> >>>>>>>>>> On 9/30/18, Marton Balint <cus@passwd.hu> wrote: >>>>>>>>>>> Signed-off-by: Marton Balint <cus@passwd.hu> >>>>>>>>>>> --- >>>>>>>>>>> libavfilter/af_afade.c | 4 ++-- >>>>>>>>>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Why? >>>>>>>>>> >>>>>>>>>> Value is too big. >>>>>>>>> >>>>>>>>> AV_OPT_TYPE_DURATION is microsecond precision, therefore 60 is 60 >>>>>>>>> usecs. >>>>>>>>> Probably you meant 60 seconds as limit. That is what the patch >>>>>>>>> fixes. >>>>>>>> >>>>>>>> I do not think so. Duration is interpreted as seconds, look how >>>>>>>> variable is used in code. >>>>>>> >>>>>>> As far as I see duration is rescaled from AV_TIME_BASE (microseconds) >>>>>>> to >>>>>>> sample_rate time base: >>>>>>> >>>>>>> if (s->duration) >>>>>>> s->nb_samples = av_rescale(s->duration, outlink->sample_rate, >>>>>>> AV_TIME_BASE); >>>>>>> >>>>>>> Duration is parsed as seconds (can be fractional as well), but >>>>>>> internally >>>>>>> it is stored as microseconds (int64). Another example is the >>>>>>> f_realtime >>>>>>> filter where the sleep time limit default according to the >>>>>>> documentation >>>>>>> is 2 seconds and it is expressed as 2000000 in the code. >>>>>> >>>>>> Yes, and current patch is wrong. Simply exceding max value is not >>>>>> proper >>>>>> fix. >>>>> >>>>> What is the proper fix then? >>>> >>>> Interpreting duration correctly, like trim filters do. >>> >>> Somebody help me, because I truly don't get it. >>> >>> 1) OPT_TYPE_DURATION default value is given in the .i64 field in >>> microseconds. >>> >>> 2) OPT_TYPE_DURATION min/max value is given in the min/max fields in >>> microseconds. >>> >>> Do you disagree with any of these? If yes, then what do you suggest? >> >> No, I do not disagree with any of those above. But patch is incomplete. > > Post/push a better patch then please. Actually patch is fine, sorry.
diff --git a/libavfilter/af_afade.c b/libavfilter/af_afade.c index d823e82d39..c517b4117c 100644 --- a/libavfilter/af_afade.c +++ b/libavfilter/af_afade.c @@ -354,8 +354,8 @@ AVFilter ff_af_afade = { static const AVOption acrossfade_options[] = { { "nb_samples", "set number of samples for cross fade duration", OFFSET(nb_samples), AV_OPT_TYPE_INT, {.i64 = 44100}, 1, INT32_MAX/10, FLAGS }, { "ns", "set number of samples for cross fade duration", OFFSET(nb_samples), AV_OPT_TYPE_INT, {.i64 = 44100}, 1, INT32_MAX/10, FLAGS }, - { "duration", "set cross fade duration", OFFSET(duration), AV_OPT_TYPE_DURATION, {.i64 = 0. }, 0, 60, FLAGS }, - { "d", "set cross fade duration", OFFSET(duration), AV_OPT_TYPE_DURATION, {.i64 = 0. }, 0, 60, FLAGS }, + { "duration", "set cross fade duration", OFFSET(duration), AV_OPT_TYPE_DURATION, {.i64 = 0. }, 0, 60000000, FLAGS }, + { "d", "set cross fade duration", OFFSET(duration), AV_OPT_TYPE_DURATION, {.i64 = 0. }, 0, 60000000, FLAGS }, { "overlap", "overlap 1st stream end with 2nd stream start", OFFSET(overlap), AV_OPT_TYPE_BOOL, {.i64 = 1 }, 0, 1, FLAGS }, { "o", "overlap 1st stream end with 2nd stream start", OFFSET(overlap), AV_OPT_TYPE_BOOL, {.i64 = 1 }, 0, 1, FLAGS }, { "curve1", "set fade curve type for 1st stream", OFFSET(curve), AV_OPT_TYPE_INT, {.i64 = TRI }, 0, NB_CURVES - 1, FLAGS, "curve" },
Signed-off-by: Marton Balint <cus@passwd.hu> --- libavfilter/af_afade.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)