Message ID | 20211213104726.24998-1-anton@khirnov.net |
---|---|
State | Accepted |
Commit | 67aceaf4ad641a4d34c3ec70b532efdc60483e3d |
Headers | show |
Series | [FFmpeg-devel,1/2] lavc/encode: improve the empty frame check | expand |
Context | Check | Description |
---|---|---|
andriy/make_x86 | success | Make finished |
andriy/make_fate_x86 | success | Make fate finished |
andriy/make_ppc | success | Make finished |
andriy/make_fate_ppc | success | Make fate finished |
On 12/13/2021 7:47 AM, Anton Khirnov wrote: > Test for buf[0] rather than data[0] (which is broken for some hwaccel > formats). > --- > libavcodec/encode.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavcodec/encode.c b/libavcodec/encode.c > index dd25cf999b..5575cf23db 100644 > --- a/libavcodec/encode.c > +++ b/libavcodec/encode.c > @@ -366,7 +366,7 @@ int attribute_align_arg avcodec_send_frame(AVCodecContext *avctx, const AVFrame > if (avci->draining) > return AVERROR_EOF; > > - if (avci->buffer_frame->data[0]) > + if (avci->buffer_frame->buf[0]) (avci->buffer_frame->buf[0] || (avci->buffer_frame->data[0]). Right now you can pass it a non reference counted frame which will be made into one by av_frame_ref() when stored in the encode context for processing, which the doxy allows, and this patch as is will break it. > return AVERROR(EAGAIN); > > if (!frame) { >
Quoting James Almer (2021-12-13 12:40:49) > On 12/13/2021 7:47 AM, Anton Khirnov wrote: > > Test for buf[0] rather than data[0] (which is broken for some hwaccel > > formats). > > --- > > libavcodec/encode.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/libavcodec/encode.c b/libavcodec/encode.c > > index dd25cf999b..5575cf23db 100644 > > --- a/libavcodec/encode.c > > +++ b/libavcodec/encode.c > > @@ -366,7 +366,7 @@ int attribute_align_arg avcodec_send_frame(AVCodecContext *avctx, const AVFrame > > if (avci->draining) > > return AVERROR_EOF; > > > > - if (avci->buffer_frame->data[0]) > > + if (avci->buffer_frame->buf[0]) > > (avci->buffer_frame->buf[0] || (avci->buffer_frame->data[0]). Right now > you can pass it a non reference counted frame which will be made into > one by av_frame_ref() when stored in the encode context for processing, > which the doxy allows, and this patch as is will break it. Will it? As you said - av_frame_ref() ensures buffer_frame is always refcounted.
On 12/13/2021 10:30 AM, Anton Khirnov wrote: > Quoting James Almer (2021-12-13 12:40:49) >> On 12/13/2021 7:47 AM, Anton Khirnov wrote: >>> Test for buf[0] rather than data[0] (which is broken for some hwaccel >>> formats). >>> --- >>> libavcodec/encode.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/libavcodec/encode.c b/libavcodec/encode.c >>> index dd25cf999b..5575cf23db 100644 >>> --- a/libavcodec/encode.c >>> +++ b/libavcodec/encode.c >>> @@ -366,7 +366,7 @@ int attribute_align_arg avcodec_send_frame(AVCodecContext *avctx, const AVFrame >>> if (avci->draining) >>> return AVERROR_EOF; >>> >>> - if (avci->buffer_frame->data[0]) >>> + if (avci->buffer_frame->buf[0]) >> >> (avci->buffer_frame->buf[0] || (avci->buffer_frame->data[0]). Right now >> you can pass it a non reference counted frame which will be made into >> one by av_frame_ref() when stored in the encode context for processing, >> which the doxy allows, and this patch as is will break it. > > Will it? As you said - av_frame_ref() ensures buffer_frame is always > refcounted. Oh, this is checking the already buffered frame, not the input one. Nevermind then.
diff --git a/libavcodec/encode.c b/libavcodec/encode.c index dd25cf999b..5575cf23db 100644 --- a/libavcodec/encode.c +++ b/libavcodec/encode.c @@ -366,7 +366,7 @@ int attribute_align_arg avcodec_send_frame(AVCodecContext *avctx, const AVFrame if (avci->draining) return AVERROR_EOF; - if (avci->buffer_frame->data[0]) + if (avci->buffer_frame->buf[0]) return AVERROR(EAGAIN); if (!frame) {