| Message ID | 1482020088-1607-1-git-send-email-pkoshevoy@gmail.com |
|---|---|
| State | Accepted |
| Commit | 4240e5b047379b29c33dd3f4438bc4e610527b83 |
| Headers | show |
On Sat, 17 Dec 2016, pkoshevoy@gmail.com wrote: > From: Pavel Koshevoy <pkoshevoy@gmail.com> > > Steps to reproduce: > ./ffmpeg_g -f s16be -i /dev/null -af atempo=0.5 -y /tmp/atempo.wav > --- > libavfilter/af_atempo.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/libavfilter/af_atempo.c b/libavfilter/af_atempo.c > index 59b08ec..a487882 100644 > --- a/libavfilter/af_atempo.c > +++ b/libavfilter/af_atempo.c > @@ -914,8 +914,8 @@ static int yae_flush(ATempoContext *atempo, > > atempo->state = YAE_FLUSH_OUTPUT; > > - if (atempo->position[0] == frag->position[0] + frag->nsamples && > - atempo->position[1] == frag->position[1] + frag->nsamples) { > + if (atempo->position[0] >= frag->position[0] + frag->nsamples && > + atempo->position[1] >= frag->position[1] + frag->nsamples) { > // the current fragment is already flushed: > return 0; > } Thanks, this indeed fixes the assertion I came accross. Do you want me to apply? Regards, Marton
On Mon, 19 Dec 2016, Marton Balint wrote: > > On Sat, 17 Dec 2016, pkoshevoy@gmail.com wrote: > >> From: Pavel Koshevoy <pkoshevoy@gmail.com> >> >> Steps to reproduce: >> ./ffmpeg_g -f s16be -i /dev/null -af atempo=0.5 -y /tmp/atempo.wav >> --- >> libavfilter/af_atempo.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/libavfilter/af_atempo.c b/libavfilter/af_atempo.c >> index 59b08ec..a487882 100644 >> --- a/libavfilter/af_atempo.c >> +++ b/libavfilter/af_atempo.c >> @@ -914,8 +914,8 @@ static int yae_flush(ATempoContext *atempo, >> >> atempo->state = YAE_FLUSH_OUTPUT; >> >> - if (atempo->position[0] == frag->position[0] + frag->nsamples && >> - atempo->position[1] == frag->position[1] + frag->nsamples) { >> + if (atempo->position[0] >= frag->position[0] + frag->nsamples && >> + atempo->position[1] >= frag->position[1] + frag->nsamples) { >> // the current fragment is already flushed: >> return 0; >> } > > Thanks, this indeed fixes the assertion I came accross. Do you want me to > apply? > Thanks, applied. Regards, Marton
On Mon, 19 Dec 2016, Marton Balint wrote: > > On Sat, 17 Dec 2016, pkoshevoy@gmail.com wrote: > >> From: Pavel Koshevoy <pkoshevoy@gmail.com> >> >> Steps to reproduce: >> ./ffmpeg_g -f s16be -i /dev/null -af atempo=0.5 -y /tmp/atempo.wav >> --- >> libavfilter/af_atempo.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/libavfilter/af_atempo.c b/libavfilter/af_atempo.c >> index 59b08ec..a487882 100644 >> --- a/libavfilter/af_atempo.c >> +++ b/libavfilter/af_atempo.c >> @@ -914,8 +914,8 @@ static int yae_flush(ATempoContext *atempo, >> >> atempo->state = YAE_FLUSH_OUTPUT; >> >> - if (atempo->position[0] == frag->position[0] + frag->nsamples && >> - atempo->position[1] == frag->position[1] + frag->nsamples) { >> + if (atempo->position[0] >= frag->position[0] + frag->nsamples && >> + atempo->position[1] >= frag->position[1] + frag->nsamples) { >> // the current fragment is already flushed: >> return 0; >> } > > Thanks, this indeed fixes the assertion I came accross. Hmm, this patch seems to cause cut off data at the end, as reported in https://trac.ffmpeg.org/ticket/6540 Pavel, would you mind taking a look? Thanks, Marton
On Wed, Aug 30, 2017 at 2:00 PM, Marton Balint <cus@passwd.hu> wrote: > > On Mon, 19 Dec 2016, Marton Balint wrote: > >> >> On Sat, 17 Dec 2016, pkoshevoy@gmail.com wrote: >> >>> From: Pavel Koshevoy <pkoshevoy@gmail.com> >>> >>> Steps to reproduce: >>> ./ffmpeg_g -f s16be -i /dev/null -af atempo=0.5 -y /tmp/atempo.wav >>> --- >>> libavfilter/af_atempo.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/libavfilter/af_atempo.c b/libavfilter/af_atempo.c >>> index 59b08ec..a487882 100644 >>> --- a/libavfilter/af_atempo.c >>> +++ b/libavfilter/af_atempo.c >>> @@ -914,8 +914,8 @@ static int yae_flush(ATempoContext *atempo, >>> >>> atempo->state = YAE_FLUSH_OUTPUT; >>> >>> - if (atempo->position[0] == frag->position[0] + frag->nsamples && >>> - atempo->position[1] == frag->position[1] + frag->nsamples) { >>> + if (atempo->position[0] >= frag->position[0] + frag->nsamples && >>> + atempo->position[1] >= frag->position[1] + frag->nsamples) { >>> // the current fragment is already flushed: >>> return 0; >>> } >> >> >> Thanks, this indeed fixes the assertion I came accross. > > > Hmm, this patch seems to cause cut off data at the end, as reported in > https://trac.ffmpeg.org/ticket/6540 > > Pavel, would you mind taking a look? > > Thanks, > Marton Sure, but it will probably have to wait until the weekend. Pavel.
On 08/30/2017 03:01 PM, Pavel Koshevoy wrote: > On Wed, Aug 30, 2017 at 2:00 PM, Marton Balint <cus@passwd.hu> wrote: >> On Mon, 19 Dec 2016, Marton Balint wrote: >> >>> On Sat, 17 Dec 2016, pkoshevoy@gmail.com wrote: >>> >>>> From: Pavel Koshevoy <pkoshevoy@gmail.com> >>>> >>>> Steps to reproduce: >>>> ./ffmpeg_g -f s16be -i /dev/null -af atempo=0.5 -y /tmp/atempo.wav >>>> --- >>>> libavfilter/af_atempo.c | 4 ++-- >>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/libavfilter/af_atempo.c b/libavfilter/af_atempo.c >>>> index 59b08ec..a487882 100644 >>>> --- a/libavfilter/af_atempo.c >>>> +++ b/libavfilter/af_atempo.c >>>> @@ -914,8 +914,8 @@ static int yae_flush(ATempoContext *atempo, >>>> >>>> atempo->state = YAE_FLUSH_OUTPUT; >>>> >>>> - if (atempo->position[0] == frag->position[0] + frag->nsamples && >>>> - atempo->position[1] == frag->position[1] + frag->nsamples) { >>>> + if (atempo->position[0] >= frag->position[0] + frag->nsamples && >>>> + atempo->position[1] >= frag->position[1] + frag->nsamples) { >>>> // the current fragment is already flushed: >>>> return 0; >>>> } >>> >>> Thanks, this indeed fixes the assertion I came accross. >> >> Hmm, this patch seems to cause cut off data at the end, as reported in >> https://trac.ffmpeg.org/ticket/6540 >> >> Pavel, would you mind taking a look? >> >> Thanks, >> Marton > > Sure, but it will probably have to wait until the weekend. > > Pavel. You can probably revert 4240e5b047379b29c33dd3f4438bc4e610527b83 -- I tried it locally and I no longer see the false assertion trigger it was supposed to fix. Something must have changed elsewhere. As far as the expectation of atempo=0.5 producing output of perfectly doubled duration that's not going to be the case with this filter. It is a continuous stream filter -- it knows nothing about the input audio duration beyond what has been fed to it so far, so it can't know how long the exact output audio duration is supposed to be (until the filter is flushed, but by then it's too late). For atempo=0.5, given 1024 + n * 512 input samples it will produce (n + 1) * 1024 output samples. For large n, (n + 1) / n ratio approaches 1 so for every 512 input samples you'd get 1024 output samples. But for small n the ratio (n + 1) / n is not going to be that close to 1, so the error in duration will be more noticeable. Pavel.
On Sun, 3 Sep 2017, Pavel Koshevoy wrote: > On 08/30/2017 03:01 PM, Pavel Koshevoy wrote: >> On Wed, Aug 30, 2017 at 2:00 PM, Marton Balint <cus@passwd.hu> wrote: >>> On Mon, 19 Dec 2016, Marton Balint wrote: >>> >>>> On Sat, 17 Dec 2016, pkoshevoy@gmail.com wrote: >>>> >>>>> From: Pavel Koshevoy <pkoshevoy@gmail.com> >>>>> >>>>> Steps to reproduce: >>>>> ./ffmpeg_g -f s16be -i /dev/null -af atempo=0.5 -y /tmp/atempo.wav >>>>> --- >>>>> libavfilter/af_atempo.c | 4 ++-- >>>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/libavfilter/af_atempo.c b/libavfilter/af_atempo.c >>>>> index 59b08ec..a487882 100644 >>>>> --- a/libavfilter/af_atempo.c >>>>> +++ b/libavfilter/af_atempo.c >>>>> @@ -914,8 +914,8 @@ static int yae_flush(ATempoContext *atempo, >>>>> >>>>> atempo->state = YAE_FLUSH_OUTPUT; >>>>> >>>>> - if (atempo->position[0] == frag->position[0] + frag->nsamples && >>>>> - atempo->position[1] == frag->position[1] + frag->nsamples) { >>>>> + if (atempo->position[0] >= frag->position[0] + frag->nsamples && >>>>> + atempo->position[1] >= frag->position[1] + frag->nsamples) { >>>>> // the current fragment is already flushed: >>>>> return 0; >>>>> } >>>> >>>> Thanks, this indeed fixes the assertion I came accross. >>> >>> Hmm, this patch seems to cause cut off data at the end, as reported in >>> https://trac.ffmpeg.org/ticket/6540 >>> >>> Pavel, would you mind taking a look? >>> >>> Thanks, >>> Marton >> >> Sure, but it will probably have to wait until the weekend. >> >> Pavel. > > > > You can probably revert 4240e5b047379b29c33dd3f4438bc4e610527b83 -- I tried > it locally and I no longer see the false assertion trigger it was supposed to > fix. Something must have changed elsewhere. You can still reproduce the assertion with a sligthly different command line after reverting: ./ffmpeg -f lavfi -i sine=d=1 -af aselect=e=0,atempo=0.5 -y atempo.wav Regards, Marton
On Sun, Sep 3, 2017 at 9:26 AM, Marton Balint <cus@passwd.hu> wrote: > > On Sun, 3 Sep 2017, Pavel Koshevoy wrote: > >> On 08/30/2017 03:01 PM, Pavel Koshevoy wrote: <snip> >> You can probably revert 4240e5b047379b29c33dd3f4438bc4e610527b83 -- I >> tried it locally and I no longer see the false assertion trigger it was >> supposed to fix. Something must have changed elsewhere. > > > You can still reproduce the assertion with a sligthly different command line > after reverting: > > ./ffmpeg -f lavfi -i sine=d=1 -af aselect=e=0,atempo=0.5 -y atempo.wav > > Regards, > Marton I'll take a look at this assertion failure. Pavel.
diff --git a/libavfilter/af_atempo.c b/libavfilter/af_atempo.c index 59b08ec..a487882 100644 --- a/libavfilter/af_atempo.c +++ b/libavfilter/af_atempo.c @@ -914,8 +914,8 @@ static int yae_flush(ATempoContext *atempo, atempo->state = YAE_FLUSH_OUTPUT; - if (atempo->position[0] == frag->position[0] + frag->nsamples && - atempo->position[1] == frag->position[1] + frag->nsamples) { + if (atempo->position[0] >= frag->position[0] + frag->nsamples && + atempo->position[1] >= frag->position[1] + frag->nsamples) { // the current fragment is already flushed: return 0; }
From: Pavel Koshevoy <pkoshevoy@gmail.com> Steps to reproduce: ./ffmpeg_g -f s16be -i /dev/null -af atempo=0.5 -y /tmp/atempo.wav --- libavfilter/af_atempo.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)