[FFmpeg-devel] avcodec/videotoolboxenc: don't wait when flushing data

From: Tian Qi <tianqi@kuaishou.com>

because there is run in thread mode, few times will block
the workflow at the wait, so check the status is flushing data,
don't wait when flushing data.

Signed-off-by: Tian Qi <tianqi@kuaishou.com>
Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
---
 libavcodec/videotoolboxenc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

> On Aug 27, 2020, at 10:53 AM, Steven Liu <lq@chinaffmpeg.org> wrote:
> 
> From: Tian Qi <tianqi@kuaishou.com>
> 
> because there is run in thread mode, few times will block
> the workflow at the wait, so check the status is flushing data,
> don't wait when flushing data.
> 
> Signed-off-by: Tian Qi <tianqi@kuaishou.com>
> Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
> ---
> libavcodec/videotoolboxenc.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c
> index e89cfaeed8..f9626be18f 100644
> --- a/libavcodec/videotoolboxenc.c
> +++ b/libavcodec/videotoolboxenc.c
> @@ -292,7 +292,7 @@ static int vtenc_q_pop(VTEncContext *vtctx, bool wait, CMSampleBufferRef *buf, E
>         return 0;
>     }
> 
> -    while (!vtctx->q_head && !vtctx->async_error && wait) {
> +    while (!vtctx->q_head && !vtctx->async_error && wait && !vtctx->flushing) {
>         pthread_cond_wait(&vtctx->cv_sample_sent, &vtctx->lock);
>     }

I’m not familar with the code. I have some questions:
1. The commit message doesn’t give a detailed description of the real issue.

2. vtctx->flushing is modified without holding the lock, which is questionable.
And there is no cond_signal, how does it work?

> 
> @@ -308,6 +308,7 @@ static int vtenc_q_pop(VTEncContext *vtctx, bool wait, CMSampleBufferRef *buf, E
>         vtctx->q_tail = NULL;
>     }
> 
> +    vtctx->frame_ct_out++;

Looks like this modification doesn’t belong to the patch.

>     pthread_mutex_unlock(&vtctx->lock);
> 
>     *buf = info->cm_buffer;
> @@ -319,7 +320,6 @@ static int vtenc_q_pop(VTEncContext *vtctx, bool wait, CMSampleBufferRef *buf, E
>     }
>     av_free(info);
> 
> -    vtctx->frame_ct_out++;
> 
>     return 0;
> }
> -- 
> 2.25.0
> 
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

On Wed, Aug 26, 2020 at 10:53 PM Steven Liu <lq@chinaffmpeg.org> wrote:

> From: Tian Qi <tianqi@kuaishou.com>
>
> because there is run in thread mode, few times will block
> the workflow at the wait, so check the status is flushing data,
> don't wait when flushing data.
>
I'd like to reproduce the issue and test the fix before the patch is
applied, so if you can provide a media file that causes the issue or code
snippet, it would be helpful.

It looks like this could happen after an encoding error. Do you see any
"Error encoding frame ..." logs?

>
> Signed-off-by: Tian Qi <tianqi@kuaishou.com>
> Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
> ---
>  libavcodec/videotoolboxenc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c
> index e89cfaeed8..f9626be18f 100644
> --- a/libavcodec/videotoolboxenc.c
> +++ b/libavcodec/videotoolboxenc.c
> @@ -292,7 +292,7 @@ static int vtenc_q_pop(VTEncContext *vtctx, bool wait,
> CMSampleBufferRef *buf, E
>          return 0;
>      }
>
> -    while (!vtctx->q_head && !vtctx->async_error && wait) {
> +    while (!vtctx->q_head && !vtctx->async_error && wait &&
> !vtctx->flushing) {
>          pthread_cond_wait(&vtctx->cv_sample_sent, &vtctx->lock);
>      }
>
> @@ -308,6 +308,7 @@ static int vtenc_q_pop(VTEncContext *vtctx, bool wait,
> CMSampleBufferRef *buf, E
>          vtctx->q_tail = NULL;
>      }
>
> +    vtctx->frame_ct_out++;
>      pthread_mutex_unlock(&vtctx->lock);
>
>      *buf = info->cm_buffer;
> @@ -319,7 +320,6 @@ static int vtenc_q_pop(VTEncContext *vtctx, bool wait,
> CMSampleBufferRef *buf, E
>      }
>      av_free(info);
>
> -    vtctx->frame_ct_out++;
>
>      return 0;
>  }
> --
> 2.25.0
>
>
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".