[FFmpeg-devel] lavc/opusenc: add frame_alloc and frame_count check to quit encode

Submitted by Linjie Fu on Nov. 29, 2018, 9:13 a.m.

Details

Message ID 20181129091340.1559-1-linjie.fu@intel.com
State New
Headers show

Commit Message

Linjie Fu Nov. 29, 2018, 9:13 a.m.
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(-)

Comments

Paul B Mahol Nov. 29, 2018, 10:36 a.m.
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
Hendrik Leppkes Nov. 29, 2018, 11:40 a.m.
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
Linjie Fu Nov. 29, 2018, 12:53 p.m.
> -----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
Rostislav Pehlivanov Dec. 5, 2018, 12:27 a.m.
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.

Patch hide | download patch | download mbox

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 */
     }