diff mbox

[FFmpeg-devel] avcodec/videotoolboxenc: fix encoding frame crash on iOS 11

Message ID 20190813064401.59869-1-sharpbai@gmail.com
State New
Headers show

Commit Message

sharpbai Aug. 13, 2019, 6:44 a.m. UTC
On iOS 11, encoding a frame may return error with log
"Error encoding frame 0", which means vtenc_output_callback
is called with status=0 and sample_buffer=NULL. Then the
encoding session will be crashed on next callback wether or not
closing the codec context.

Let us look through the link below introducing VTCompressionOutputCallback,

https://developer.apple.com/documentation/videotoolbox/vtcompressionoutputcallback?language=objc

"status=0" (noErr) means compression was successful.
"sampleBuffer=NULL" means the frame was dropped when compression
was successful (status=0) or compression was not successful (status!=0).

So we should not set AVERROR_EXTERNAL on "status=0" and "sample_buffer=NULL"
as it is not a error.

The fix is that we only set AVERROR_EXTERNAL with status value non zero.
When sample_buffer is NULL and status value is zero, we simply return
with no other operation.

This crash often occurs on iOS 11 for example encoding 720p@25fps.

Signed-off-by: sharpbai <sharpbai@gmail.com>
---
 libavcodec/videotoolboxenc.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Aman Karmani Sept. 3, 2019, 11:48 p.m. UTC | #1
On Mon, Aug 12, 2019 at 11:50 PM sharpbai <sharpbai@gmail.com> wrote:

> On iOS 11, encoding a frame may return error with log
> "Error encoding frame 0", which means vtenc_output_callback
> is called with status=0 and sample_buffer=NULL. Then the
> encoding session will be crashed on next callback wether or not
> closing the codec context.
>
> Let us look through the link below introducing VTCompressionOutputCallback,
>
>
> https://developer.apple.com/documentation/videotoolbox/vtcompressionoutputcallback?language=objc
>
> "status=0" (noErr) means compression was successful.
> "sampleBuffer=NULL" means the frame was dropped when compression
> was successful (status=0) or compression was not successful (status!=0).
>
> So we should not set AVERROR_EXTERNAL on "status=0" and
> "sample_buffer=NULL"
> as it is not a error.
>
> The fix is that we only set AVERROR_EXTERNAL with status value non zero.
> When sample_buffer is NULL and status value is zero, we simply return
> with no other operation.
>
> This crash often occurs on iOS 11 for example encoding 720p@25fps.
>

Is it fixed in iOS 12, or untested there?


>
> Signed-off-by: sharpbai <sharpbai@gmail.com>
> ---
>  libavcodec/videotoolboxenc.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c
> index d76bb7f646..8afdd125d2 100644
> --- a/libavcodec/videotoolboxenc.c
> +++ b/libavcodec/videotoolboxenc.c
> @@ -569,12 +569,16 @@ static void vtenc_output_callback(
>          return;
>      }
>
> -    if (status || !sample_buffer) {
> +    if (status) {
>          av_log(avctx, AV_LOG_ERROR, "Error encoding frame: %d\n",
> (int)status);
>          set_async_error(vtctx, AVERROR_EXTERNAL);
>          return;
>      }
>
> +    if (!sample_buffer) {
> +        return;
> +    }
> +
>      if (!avctx->extradata && (avctx->flags &
> AV_CODEC_FLAG_GLOBAL_HEADER)) {
>          int set_status = set_extradata(avctx, sample_buffer);
>          if (set_status) {
> --
> 2.21.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".
sharpbai Sept. 4, 2019, 9:21 a.m. UTC | #2
We have tested on iOS 12 and other version. Only on iOS 11 this crash occurs,
regardless using which device such as iPhone 8, iPhone X or iPhone 7.

Aman Gupta <ffmpeg@tmm1.net> 于2019年9月4日周三 上午7:48写道:
>
>
>
> On Mon, Aug 12, 2019 at 11:50 PM sharpbai <sharpbai@gmail.com> wrote:
>>
>> On iOS 11, encoding a frame may return error with log
>> "Error encoding frame 0", which means vtenc_output_callback
>> is called with status=0 and sample_buffer=NULL. Then the
>> encoding session will be crashed on next callback wether or not
>> closing the codec context.
>>
>> Let us look through the link below introducing VTCompressionOutputCallback,
>>
>> https://developer.apple.com/documentation/videotoolbox/vtcompressionoutputcallback?language=objc
>>
>> "status=0" (noErr) means compression was successful.
>> "sampleBuffer=NULL" means the frame was dropped when compression
>> was successful (status=0) or compression was not successful (status!=0).
>>
>> So we should not set AVERROR_EXTERNAL on "status=0" and "sample_buffer=NULL"
>> as it is not a error.
>>
>> The fix is that we only set AVERROR_EXTERNAL with status value non zero.
>> When sample_buffer is NULL and status value is zero, we simply return
>> with no other operation.
>>
>> This crash often occurs on iOS 11 for example encoding 720p@25fps.
>
>
> Is it fixed in iOS 12, or untested there?
>
>>
>>
>> Signed-off-by: sharpbai <sharpbai@gmail.com>
>> ---
>>  libavcodec/videotoolboxenc.c | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c
>> index d76bb7f646..8afdd125d2 100644
>> --- a/libavcodec/videotoolboxenc.c
>> +++ b/libavcodec/videotoolboxenc.c
>> @@ -569,12 +569,16 @@ static void vtenc_output_callback(
>>          return;
>>      }
>>
>> -    if (status || !sample_buffer) {
>> +    if (status) {
>>          av_log(avctx, AV_LOG_ERROR, "Error encoding frame: %d\n", (int)status);
>>          set_async_error(vtctx, AVERROR_EXTERNAL);
>>          return;
>>      }
>>
>> +    if (!sample_buffer) {
>> +        return;
>> +    }
>> +
>>      if (!avctx->extradata && (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER)) {
>>          int set_status = set_extradata(avctx, sample_buffer);
>>          if (set_status) {
>> --
>> 2.21.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".
diff mbox

Patch

diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c
index d76bb7f646..8afdd125d2 100644
--- a/libavcodec/videotoolboxenc.c
+++ b/libavcodec/videotoolboxenc.c
@@ -569,12 +569,16 @@  static void vtenc_output_callback(
         return;
     }
 
-    if (status || !sample_buffer) {
+    if (status) {
         av_log(avctx, AV_LOG_ERROR, "Error encoding frame: %d\n", (int)status);
         set_async_error(vtctx, AVERROR_EXTERNAL);
         return;
     }
 
+    if (!sample_buffer) {
+        return;
+    }
+
     if (!avctx->extradata && (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER)) {
         int set_status = set_extradata(avctx, sample_buffer);
         if (set_status) {