* [FFmpeg-devel] [PATCH] avcodec/audiotoolboxenc: return EAGAIN if frame_queue.available is 0 and not at->eof @ 2022-06-28 14:42 Steven Liu 2022-06-28 15:06 ` "zhilizhao(赵志立)" 2022-06-28 15:22 ` Jan Ekström 0 siblings, 2 replies; 5+ messages in thread From: Steven Liu @ 2022-06-28 14:42 UTC (permalink / raw) To: ffmpeg-devel; +Cc: Steven Liu There will return success and failure after commit 7c05b7951cb47716230c95744240bc60ec5f9433. But the AudioConverterFillComplexBuffer will return 1 and *got_packet_ptr is 0 when frame_queue.available == 0 and at->eof == 0. So should return EAGAIN here, this because the encode function should return either 0 or negative error code. Signed-off-by: Steven Liu <lq@chinaffmpeg.org> --- libavcodec/audiotoolboxenc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libavcodec/audiotoolboxenc.c b/libavcodec/audiotoolboxenc.c index 00293154bf..c23deb06a9 100644 --- a/libavcodec/audiotoolboxenc.c +++ b/libavcodec/audiotoolboxenc.c @@ -558,7 +558,8 @@ static int ffat_encode(AVCodecContext *avctx, AVPacket *avpkt, } else if (ret && ret != 1) { av_log(avctx, AV_LOG_ERROR, "Encode error: %i\n", ret); ret = AVERROR_EXTERNAL; - } + } else if (ret == 1) + ret = AVERROR(EAGAIN); return ret; } -- 2.34.1 _______________________________________________ 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". ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avcodec/audiotoolboxenc: return EAGAIN if frame_queue.available is 0 and not at->eof 2022-06-28 14:42 [FFmpeg-devel] [PATCH] avcodec/audiotoolboxenc: return EAGAIN if frame_queue.available is 0 and not at->eof Steven Liu @ 2022-06-28 15:06 ` "zhilizhao(赵志立)" 2022-06-28 15:22 ` Jan Ekström 1 sibling, 0 replies; 5+ messages in thread From: "zhilizhao(赵志立)" @ 2022-06-28 15:06 UTC (permalink / raw) To: FFmpeg development discussions and patches; +Cc: Steven Liu > On Jun 28, 2022, at 10:42 PM, Steven Liu <lq@chinaffmpeg.org> wrote: > > There will return success and failure after commit 7c05b7951cb47716230c95744240bc60ec5f9433. > But the AudioConverterFillComplexBuffer will return 1 and *got_packet_ptr is 0 > when frame_queue.available == 0 and at->eof == 0. So should return EAGAIN here, > this because the encode function should return either 0 or negative error code. > > Signed-off-by: Steven Liu <lq@chinaffmpeg.org> > --- > libavcodec/audiotoolboxenc.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/libavcodec/audiotoolboxenc.c b/libavcodec/audiotoolboxenc.c > index 00293154bf..c23deb06a9 100644 > --- a/libavcodec/audiotoolboxenc.c > +++ b/libavcodec/audiotoolboxenc.c > @@ -558,7 +558,8 @@ static int ffat_encode(AVCodecContext *avctx, AVPacket *avpkt, > } else if (ret && ret != 1) { > av_log(avctx, AV_LOG_ERROR, "Encode error: %i\n", ret); > ret = AVERROR_EXTERNAL; > - } > + } else if (ret == 1) > + ret = AVERROR(EAGAIN); How about keep the old behavior before 7c05b7951, and return early in the error path? diff --git a/libavcodec/audiotoolboxenc.c b/libavcodec/audiotoolboxenc.c index 00293154bf..8bbaabd960 100644 --- a/libavcodec/audiotoolboxenc.c +++ b/libavcodec/audiotoolboxenc.c @@ -554,13 +554,12 @@ static int ffat_encode(AVCodecContext *avctx, AVPacket *avpkt, avctx->frame_size, &avpkt->pts, &avpkt->duration); - ret = 0; } else if (ret && ret != 1) { av_log(avctx, AV_LOG_ERROR, "Encode error: %i\n", ret); - ret = AVERROR_EXTERNAL; + return AVERROR_EXTERNAL; } - return ret; + return 0; } static av_cold void ffat_encode_flush(AVCodecContext *avctx) > > return ret; > } > -- > 2.34.1 > > _______________________________________________ > 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". _______________________________________________ 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". ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avcodec/audiotoolboxenc: return EAGAIN if frame_queue.available is 0 and not at->eof 2022-06-28 14:42 [FFmpeg-devel] [PATCH] avcodec/audiotoolboxenc: return EAGAIN if frame_queue.available is 0 and not at->eof Steven Liu 2022-06-28 15:06 ` "zhilizhao(赵志立)" @ 2022-06-28 15:22 ` Jan Ekström 2022-06-28 15:55 ` Steven Liu 2022-06-28 16:14 ` [FFmpeg-devel] [PATCH] avcodec/audiotoolboxenc: return AVERROR_EXTERNAL immediately when encode error Steven Liu 1 sibling, 2 replies; 5+ messages in thread From: Jan Ekström @ 2022-06-28 15:22 UTC (permalink / raw) To: FFmpeg development discussions and patches On Tue, Jun 28, 2022 at 5:43 PM Steven Liu <lq@chinaffmpeg.org> wrote: > > There will return success and failure after commit 7c05b7951cb47716230c95744240bc60ec5f9433. > But the AudioConverterFillComplexBuffer will return 1 and *got_packet_ptr is 0 > when frame_queue.available == 0 and at->eof == 0. So should return EAGAIN here, > this because the encode function should return either 0 or negative error code. > This mirrors what I noted to you on IRC. Can you document what ret == 1 actually is? AudioConverterFillComplexBuffer documentation says it returns OSStatus, but with a quick look I can't see any AudioToolbox related errors in f.ex. https://www.osstatus.com/search/results?platform=all&framework=all&search=1 . If it is just equivalent of EAGAIN, then since there is no need to check for got_packet_ptr ? In any case, with errors or special cases early exit should always be preferred. See an example @ https://github.com/jeeb/ffmpeg/commits/audiotoolboxenc_return_codes . (this was originally quickly whipped up before I saw your patch, and thus does not take care of the EAGAIN case). Jan _______________________________________________ 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". ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avcodec/audiotoolboxenc: return EAGAIN if frame_queue.available is 0 and not at->eof 2022-06-28 15:22 ` Jan Ekström @ 2022-06-28 15:55 ` Steven Liu 2022-06-28 16:14 ` [FFmpeg-devel] [PATCH] avcodec/audiotoolboxenc: return AVERROR_EXTERNAL immediately when encode error Steven Liu 1 sibling, 0 replies; 5+ messages in thread From: Steven Liu @ 2022-06-28 15:55 UTC (permalink / raw) To: FFmpeg development discussions and patches; +Cc: Steven Liu > 在 2022年6月28日，23:22，Jan Ekström <jeebjp@gmail.com> 写道： > > On Tue, Jun 28, 2022 at 5:43 PM Steven Liu <lq@chinaffmpeg.org> wrote: >> >> There will return success and failure after commit 7c05b7951cb47716230c95744240bc60ec5f9433. >> But the AudioConverterFillComplexBuffer will return 1 and *got_packet_ptr is 0 >> when frame_queue.available == 0 and at->eof == 0. So should return EAGAIN here, >> this because the encode function should return either 0 or negative error code. >> > Hi jeeb, > This mirrors what I noted to you on IRC. > > Can you document what ret == 1 actually is? > AudioConverterFillComplexBuffer documentation says it returns > OSStatus, but with a quick look I can't see any AudioToolbox related > errors in f.ex. > https://www.osstatus.com/search/results?platform=all&framework=all&search=1 > . The return value comes from the callback ffat_encode_callback, > > If it is just equivalent of EAGAIN, then since there is no need to > check for got_packet_ptr ? > > In any case, with errors or special cases early exit should always be > preferred. See an example @ > https://github.com/jeeb/ffmpeg/commits/audiotoolboxenc_return_codes . > (this was originally quickly whipped up before I saw your patch, and > thus does not take care of the EAGAIN case). I think the solution from quink should be ok "just return error code immediately when the failure of old way." What about your opinion to the quinks suggestion? > > Jan > _______________________________________________ > 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". > Thanks Steven _______________________________________________ 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". ^ permalink raw reply [flat|nested] 5+ messages in thread
* [FFmpeg-devel] [PATCH] avcodec/audiotoolboxenc: return AVERROR_EXTERNAL immediately when encode error 2022-06-28 15:22 ` Jan Ekström 2022-06-28 15:55 ` Steven Liu @ 2022-06-28 16:14 ` Steven Liu 1 sibling, 0 replies; 5+ messages in thread From: Steven Liu @ 2022-06-28 16:14 UTC (permalink / raw) To: ffmpeg-devel; +Cc: Steven Liu, Zhao Zhili Just return AVERROR_EXTERNAL immediately when encode error. The other logic should keep the old behavior before commit 7c05b7951. Suggested-By: Zhao Zhili <zhilizhao@tencent.com> Signed-off-by: Steven Liu <lq@chinaffmpeg.org> --- libavcodec/audiotoolboxenc.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/libavcodec/audiotoolboxenc.c b/libavcodec/audiotoolboxenc.c index 00293154bf..8bbaabd960 100644 --- a/libavcodec/audiotoolboxenc.c +++ b/libavcodec/audiotoolboxenc.c @@ -554,13 +554,12 @@ static int ffat_encode(AVCodecContext *avctx, AVPacket *avpkt, avctx->frame_size, &avpkt->pts, &avpkt->duration); - ret = 0; } else if (ret && ret != 1) { av_log(avctx, AV_LOG_ERROR, "Encode error: %i\n", ret); - ret = AVERROR_EXTERNAL; + return AVERROR_EXTERNAL; } - return ret; + return 0; } static av_cold void ffat_encode_flush(AVCodecContext *avctx) -- 2.34.1 _______________________________________________ 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". ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-06-28 16:15 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-06-28 14:42 [FFmpeg-devel] [PATCH] avcodec/audiotoolboxenc: return EAGAIN if frame_queue.available is 0 and not at->eof Steven Liu 2022-06-28 15:06 ` "zhilizhao(赵志立)" 2022-06-28 15:22 ` Jan Ekström 2022-06-28 15:55 ` Steven Liu 2022-06-28 16:14 ` [FFmpeg-devel] [PATCH] avcodec/audiotoolboxenc: return AVERROR_EXTERNAL immediately when encode error Steven Liu
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel This inbox may be cloned and mirrored by anyone: git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \ ffmpegdev@gitmailbox.com public-inbox-index ffmpegdev Example config snippet for mirrors. AGPL code for this site: git clone https://public-inbox.org/public-inbox.git