Message ID | 20230712020644.22606-2-jamrial@gmail.com |
---|---|
State | New |
Headers | show |
Series | [FFmpeg-devel,1/2] avcodec/decode: don't reject flush packets when buffer_pkt is not empty | expand |
Context | Check | Description |
---|---|---|
yinshiyou/make_loongarch64 | success | Make finished |
yinshiyou/make_fate_loongarch64 | success | Make fate finished |
andriy/make_x86 | success | Make finished |
andriy/make_fate_x86 | success | Make fate finished |
Quoting James Almer (2023-07-12 04:06:44) > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavcodec/decode.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/libavcodec/decode.c b/libavcodec/decode.c > index 239ad70b41..cd49cca7c2 100644 > --- a/libavcodec/decode.c > +++ b/libavcodec/decode.c > @@ -242,7 +242,8 @@ int ff_decode_get_packet(AVCodecContext *avctx, AVPacket *pkt) > (!AVPACKET_IS_EMPTY(avci->buffer_pkt) || dc->draining_started)) { > ret = av_bsf_send_packet(avci->bsf, avci->buffer_pkt); > if (ret < 0) { > - av_packet_unref(avci->buffer_pkt); > + if (ret != AVERROR(EAGAIN)) > + av_packet_unref(avci->buffer_pkt); It seems very wrong for ff_decode_get_packet() to return EAGAIN when we have a buffered packet.
On 7/12/2023 6:08 AM, Anton Khirnov wrote: > Quoting James Almer (2023-07-12 04:06:44) >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavcodec/decode.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/libavcodec/decode.c b/libavcodec/decode.c >> index 239ad70b41..cd49cca7c2 100644 >> --- a/libavcodec/decode.c >> +++ b/libavcodec/decode.c >> @@ -242,7 +242,8 @@ int ff_decode_get_packet(AVCodecContext *avctx, AVPacket *pkt) >> (!AVPACKET_IS_EMPTY(avci->buffer_pkt) || dc->draining_started)) { >> ret = av_bsf_send_packet(avci->bsf, avci->buffer_pkt); >> if (ret < 0) { >> - av_packet_unref(avci->buffer_pkt); >> + if (ret != AVERROR(EAGAIN)) >> + av_packet_unref(avci->buffer_pkt); > > It seems very wrong for ff_decode_get_packet() to return EAGAIN when > we have a buffered packet. The idea is preventing dropping a packet in the hypothetical case the bsf can't take it. Is this scenario possible, or is the call to av_bsf_receive_packet() within the loop ensuring it wont?
Quoting James Almer (2023-07-12 13:40:25) > On 7/12/2023 6:08 AM, Anton Khirnov wrote: > > Quoting James Almer (2023-07-12 04:06:44) > >> Signed-off-by: James Almer <jamrial@gmail.com> > >> --- > >> libavcodec/decode.c | 3 ++- > >> 1 file changed, 2 insertions(+), 1 deletion(-) > >> > >> diff --git a/libavcodec/decode.c b/libavcodec/decode.c > >> index 239ad70b41..cd49cca7c2 100644 > >> --- a/libavcodec/decode.c > >> +++ b/libavcodec/decode.c > >> @@ -242,7 +242,8 @@ int ff_decode_get_packet(AVCodecContext *avctx, AVPacket *pkt) > >> (!AVPACKET_IS_EMPTY(avci->buffer_pkt) || dc->draining_started)) { > >> ret = av_bsf_send_packet(avci->bsf, avci->buffer_pkt); > >> if (ret < 0) { > >> - av_packet_unref(avci->buffer_pkt); > >> + if (ret != AVERROR(EAGAIN)) > >> + av_packet_unref(avci->buffer_pkt); > > > > It seems very wrong for ff_decode_get_packet() to return EAGAIN when > > we have a buffered packet. > > The idea is preventing dropping a packet in the hypothetical case the > bsf can't take it. Is this scenario possible, or is the call to > av_bsf_receive_packet() within the loop ensuring it wont? This block can only be entered if decode_get_packet() returned EAGAIN, which should always mean that the bsf is empty.
On 7/12/2023 10:32 AM, Anton Khirnov wrote: > Quoting James Almer (2023-07-12 13:40:25) >> On 7/12/2023 6:08 AM, Anton Khirnov wrote: >>> Quoting James Almer (2023-07-12 04:06:44) >>>> Signed-off-by: James Almer <jamrial@gmail.com> >>>> --- >>>> libavcodec/decode.c | 3 ++- >>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/libavcodec/decode.c b/libavcodec/decode.c >>>> index 239ad70b41..cd49cca7c2 100644 >>>> --- a/libavcodec/decode.c >>>> +++ b/libavcodec/decode.c >>>> @@ -242,7 +242,8 @@ int ff_decode_get_packet(AVCodecContext *avctx, AVPacket *pkt) >>>> (!AVPACKET_IS_EMPTY(avci->buffer_pkt) || dc->draining_started)) { >>>> ret = av_bsf_send_packet(avci->bsf, avci->buffer_pkt); >>>> if (ret < 0) { >>>> - av_packet_unref(avci->buffer_pkt); >>>> + if (ret != AVERROR(EAGAIN)) >>>> + av_packet_unref(avci->buffer_pkt); >>> >>> It seems very wrong for ff_decode_get_packet() to return EAGAIN when >>> we have a buffered packet. >> >> The idea is preventing dropping a packet in the hypothetical case the >> bsf can't take it. Is this scenario possible, or is the call to >> av_bsf_receive_packet() within the loop ensuring it wont? > > This block can only be entered if decode_get_packet() returned EAGAIN, > which should always mean that the bsf is empty. Right, patch dropped then.
diff --git a/libavcodec/decode.c b/libavcodec/decode.c index 239ad70b41..cd49cca7c2 100644 --- a/libavcodec/decode.c +++ b/libavcodec/decode.c @@ -242,7 +242,8 @@ int ff_decode_get_packet(AVCodecContext *avctx, AVPacket *pkt) (!AVPACKET_IS_EMPTY(avci->buffer_pkt) || dc->draining_started)) { ret = av_bsf_send_packet(avci->bsf, avci->buffer_pkt); if (ret < 0) { - av_packet_unref(avci->buffer_pkt); + if (ret != AVERROR(EAGAIN)) + av_packet_unref(avci->buffer_pkt); return ret; }
Signed-off-by: James Almer <jamrial@gmail.com> --- libavcodec/decode.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)