[FFmpeg-devel] [PATCH 1/2] avcodec/decode: don't reject flush packets when buffer_pkt is not empty

[FFmpeg-devel] [PATCH 1/2] avcodec/decode: don't reject flush packets when buffer_pkt is not empty

[James Almer]

Restores the behavior pre commit a92dbeb9ae.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/decode.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index a47abeca06..239ad70b41 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -666,10 +666,9 @@ int attribute_align_arg avcodec_send_packet(AVCodecContext *avctx, const AVPacke
     if (avpkt && !avpkt->size && avpkt->data)
         return AVERROR(EINVAL);
 
-    if (!AVPACKET_IS_EMPTY(avci->buffer_pkt))
-        return AVERROR(EAGAIN);
-
     if (avpkt && (avpkt->data || avpkt->side_data_elems)) {
+        if (!AVPACKET_IS_EMPTY(avci->buffer_pkt))
+            return AVERROR(EAGAIN);
         ret = av_packet_ref(avci->buffer_pkt, avpkt);
         if (ret < 0)
             return ret;
-- 
2.41.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".

[FFmpeg-devel] [PATCH 2/2] avcodec/decode: don't discard the buffered packet if the underlying bsf can't take it

[James Almer]

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);
                 return ret;
             }
 
-- 
2.41.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".

Re: [FFmpeg-devel] [PATCH 2/2] avcodec/decode: don't discard the buffered packet if the underlying bsf can't take it

[Anton Khirnov]

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.

-- 
Anton Khirnov
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 2/2] avcodec/decode: don't discard the buffered packet if the underlying bsf can't take it

[James Almer]

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?
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 2/2] avcodec/decode: don't discard the buffered packet if the underlying bsf can't take it

[Anton Khirnov]

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.

-- 
Anton Khirnov
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 2/2] avcodec/decode: don't discard the buffered packet if the underlying bsf can't take it

[James Almer]

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.
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 1/2] avcodec/decode: don't reject flush packets when buffer_pkt is not empty

[Anton Khirnov]

Quoting James Almer (2023-07-12 04:06:43)
> Restores the behavior pre commit a92dbeb9ae.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>

Do you see any advantage to this?

-- 
Anton Khirnov
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 1/2] avcodec/decode: don't reject flush packets when buffer_pkt is not empty

[James Almer]

On 7/12/2023 6:06 AM, Anton Khirnov wrote:
> Quoting James Almer (2023-07-12 04:06:43)
>> Restores the behavior pre commit a92dbeb9ae.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
> 
> Do you see any advantage to this?

Not returning an error when a flush packet is feed to the decoder. Why 
would the API force me to retrieve frames before i can tell it I'm not 
going to feed it any more packets?
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 1/2] avcodec/decode: don't reject flush packets when buffer_pkt is not empty

[Anton Khirnov]

Quoting James Almer (2023-07-12 13:30:30)
> On 7/12/2023 6:06 AM, Anton Khirnov wrote:
> > Quoting James Almer (2023-07-12 04:06:43)
> >> Restores the behavior pre commit a92dbeb9ae.
> >>
> >> Signed-off-by: James Almer <jamrial@gmail.com>
> > 
> > Do you see any advantage to this?
> 
> Not returning an error when a flush packet is feed to the decoder. Why 
> would the API force me to retrieve frames before i can tell it I'm not 
> going to feed it any more packets?

Right, I guess that makes sense.
We should document this behaviour explicitly though, if we're bothering
to maintain it.

-- 
Anton Khirnov
_______________________________________________
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".