| Message ID | 20181129091340.1559-1-linjie.fu@intel.com |
|---|---|
| State | New |
| Headers | show |
On 11/29/18, Linjie Fu <linjie.fu@intel.com> wrote: > Add frame_alloc and frame_count check in opus_encode_frame to avoid > the infinite loop issue. > > Fix #7578. > > Signed-off-by: Linjie Fu <linjie.fu@intel.com> > --- > libavcodec/opusenc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavcodec/opusenc.c b/libavcodec/opusenc.c > index 578785f4b4..7146968fc8 100644 > --- a/libavcodec/opusenc.c > +++ b/libavcodec/opusenc.c > @@ -543,7 +543,7 @@ static int opus_encode_frame(AVCodecContext *avctx, > AVPacket *avpkt, > ff_bufqueue_add(avctx, &s->bufqueue, av_frame_clone(frame)); > } else { > ff_opus_psy_signal_eof(&s->psyctx); > - if (!s->afq.remaining_samples) > + if (!s->afq.remaining_samples || (!s->afq.frame_alloc && > !s->afq.frame_count)) > return 0; /* We've been flushed and there's nothing left to > encode */ lgtm
On Thu, Nov 29, 2018 at 10:14 AM Linjie Fu <linjie.fu@intel.com> wrote: > > Add frame_alloc and frame_count check in opus_encode_frame to avoid > the infinite loop issue. > > Fix #7578. > > Signed-off-by: Linjie Fu <linjie.fu@intel.com> > --- > libavcodec/opusenc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavcodec/opusenc.c b/libavcodec/opusenc.c > index 578785f4b4..7146968fc8 100644 > --- a/libavcodec/opusenc.c > +++ b/libavcodec/opusenc.c > @@ -543,7 +543,7 @@ static int opus_encode_frame(AVCodecContext *avctx, AVPacket *avpkt, > ff_bufqueue_add(avctx, &s->bufqueue, av_frame_clone(frame)); > } else { > ff_opus_psy_signal_eof(&s->psyctx); > - if (!s->afq.remaining_samples) > + if (!s->afq.remaining_samples || (!s->afq.frame_alloc && !s->afq.frame_count)) > return 0; /* We've been flushed and there's nothing left to encode */ > } What does remaining_samples contain if it wasn't even allocated? - Hendrik
> -----Original Message----- > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf > Of Hendrik Leppkes > Sent: Thursday, November 29, 2018 19:40 > To: FFmpeg development discussions and patches <ffmpeg- > devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH] lavc/opusenc: add frame_alloc and > frame_count check to quit encode > > On Thu, Nov 29, 2018 at 10:14 AM Linjie Fu <linjie.fu@intel.com> wrote: > > > > Add frame_alloc and frame_count check in opus_encode_frame to avoid > > the infinite loop issue. > > > > Fix #7578. > > > > Signed-off-by: Linjie Fu <linjie.fu@intel.com> > > --- > > libavcodec/opusenc.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/libavcodec/opusenc.c b/libavcodec/opusenc.c > > index 578785f4b4..7146968fc8 100644 > > --- a/libavcodec/opusenc.c > > +++ b/libavcodec/opusenc.c > > @@ -543,7 +543,7 @@ static int opus_encode_frame(AVCodecContext > *avctx, AVPacket *avpkt, > > ff_bufqueue_add(avctx, &s->bufqueue, av_frame_clone(frame)); > > } else { > > ff_opus_psy_signal_eof(&s->psyctx); > > - if (!s->afq.remaining_samples) > > + if (!s->afq.remaining_samples || (!s->afq.frame_alloc && !s- > >afq.frame_count)) > > return 0; /* We've been flushed and there's nothing left to encode > */ > > } > > What does remaining_samples contain if it wasn't even allocated? remaining_samples equals 120 in this case. Investigate this: remaining_samples was initialized in ff_af_queue_init(): afq->remaining_samples = avctx->initial_padding; And avctx->initial_padding is a hardcode which equals 120 in opus_encode_init(): avctx->initial_padding = 120; Thanks, - Linjie
On Thu, 29 Nov 2018 at 09:14, Linjie Fu <linjie.fu@intel.com> wrote: > Add frame_alloc and frame_count check in opus_encode_frame to avoid > the infinite loop issue. > > Fix #7578. > > Signed-off-by: Linjie Fu <linjie.fu@intel.com> > --- > libavcodec/opusenc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavcodec/opusenc.c b/libavcodec/opusenc.c > index 578785f4b4..7146968fc8 100644 > --- a/libavcodec/opusenc.c > +++ b/libavcodec/opusenc.c > @@ -543,7 +543,7 @@ static int opus_encode_frame(AVCodecContext *avctx, > AVPacket *avpkt, > ff_bufqueue_add(avctx, &s->bufqueue, av_frame_clone(frame)); > } else { > ff_opus_psy_signal_eof(&s->psyctx); > - if (!s->afq.remaining_samples) > + if (!s->afq.remaining_samples || (!s->afq.frame_alloc && > !s->afq.frame_count)) > return 0; /* We've been flushed and there's nothing left to > encode */ > } > > -- > 2.17.1 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel I don't think that's the correct way to do this, maybe it would be better to check if the remaning samples are above the initial_padding amount or decreasing the first frame's duration by the initial_padding amount to make the count correct.
diff --git a/libavcodec/opusenc.c b/libavcodec/opusenc.c index 578785f4b4..7146968fc8 100644 --- a/libavcodec/opusenc.c +++ b/libavcodec/opusenc.c @@ -543,7 +543,7 @@ static int opus_encode_frame(AVCodecContext *avctx, AVPacket *avpkt, ff_bufqueue_add(avctx, &s->bufqueue, av_frame_clone(frame)); } else { ff_opus_psy_signal_eof(&s->psyctx); - if (!s->afq.remaining_samples) + if (!s->afq.remaining_samples || (!s->afq.frame_alloc && !s->afq.frame_count)) return 0; /* We've been flushed and there's nothing left to encode */ }
Add frame_alloc and frame_count check in opus_encode_frame to avoid the infinite loop issue. Fix #7578. Signed-off-by: Linjie Fu <linjie.fu@intel.com> --- libavcodec/opusenc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)