diff mbox

[FFmpeg-devel] avformat/utils.c: allows av_read_frame to return after a timeout period.

Message ID 20191121212710.2055-1-ggarra13@gmail.com
State New
Headers show

Commit Message

Gonzalo Garramuño Nov. 21, 2019, 9:27 p.m. UTC
From: Gonzalo Garramuño <ggarra13@gmail.com>

This patch is based on a patch by bsenftner at earthlink.net.
---
 libavformat/utils.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Gonzalo Garramuño Nov. 24, 2019, 11:27 a.m. UTC | #1
Friendly ping.  Does the patch need anything else for approval?

On 21/11/19 18:27, ggarra13@gmail.com wrote:
> From: Gonzalo Garramuño <ggarra13@gmail.com>
>
> This patch is based on a patch by bsenftner at earthlink.net.
> ---
>   libavformat/utils.c | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index 8196442dd1..c3c2c77c0c 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -1838,6 +1838,11 @@ int av_read_frame(AVFormatContext *s, AVPacket *pkt)
>               }
>           }
>   
> +        if (ff_check_interrupt(&s->interrupt_callback)) {
> +            av_log(s, AV_LOG_DEBUG, "interrupted\n");
> +            return AVERROR_EXIT;
> +        }
> +
>           ret = read_frame_internal(s, pkt);
>           if (ret < 0) {
>               if (pktl && ret != AVERROR(EAGAIN)) {
Michael Niedermayer Nov. 26, 2019, 5:31 p.m. UTC | #2
On Thu, Nov 21, 2019 at 06:27:10PM -0300, ggarra13@gmail.com wrote:
> From: Gonzalo Garramuño <ggarra13@gmail.com>
> 
> This patch is based on a patch by bsenftner at earthlink.net.
> ---
>  libavformat/utils.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index 8196442dd1..c3c2c77c0c 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -1838,6 +1838,11 @@ int av_read_frame(AVFormatContext *s, AVPacket *pkt)
>              }
>          }
>  
> +        if (ff_check_interrupt(&s->interrupt_callback)) {
> +            av_log(s, AV_LOG_DEBUG, "interrupted\n");
> +            return AVERROR_EXIT;
> +        }
> +

I think this can be moved into the if() above, which might
reduce the number of calls.

thx
[...]
Gonzalo Garramuño Nov. 26, 2019, 5:45 p.m. UTC | #3
On 26/11/19 14:31, Michael Niedermayer wrote:
> On Thu, Nov 21, 2019 at 06:27:10PM -0300, ggarra13@gmail.com wrote:
>> From: Gonzalo Garramuño <ggarra13@gmail.com>
>>
>> This patch is based on a patch by bsenftner at earthlink.net.
>> ---
>>   libavformat/utils.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/libavformat/utils.c b/libavformat/utils.c
>> index 8196442dd1..c3c2c77c0c 100644
>> --- a/libavformat/utils.c
>> +++ b/libavformat/utils.c
>> @@ -1838,6 +1838,11 @@ int av_read_frame(AVFormatContext *s, AVPacket *pkt)
>>               }
>>           }
>>   
>> +        if (ff_check_interrupt(&s->interrupt_callback)) {
>> +            av_log(s, AV_LOG_DEBUG, "interrupted\n");
>> +            return AVERROR_EXIT;
>> +        }
>> +
> I think this can be moved into the if() above, which might
> reduce the number of calls.
>
> thx
> [...]
>
It would probably reduce only one call, as pktl (the if above) is a list 
that will get filled probably as soon as there is a packet.  Or maybe I 
am reading the code wrong?  Also, if it does not get filled, we probably 
want to exit anyway, too.
Michael Niedermayer Nov. 27, 2019, 2:06 p.m. UTC | #4
On Tue, Nov 26, 2019 at 02:45:05PM -0300, gga wrote:
> 
> 
> On 26/11/19 14:31, Michael Niedermayer wrote:
> >On Thu, Nov 21, 2019 at 06:27:10PM -0300, ggarra13@gmail.com wrote:
> >>From: Gonzalo Garramuño <ggarra13@gmail.com>
> >>
> >>This patch is based on a patch by bsenftner at earthlink.net.
> >>---
> >>  libavformat/utils.c | 5 +++++
> >>  1 file changed, 5 insertions(+)
> >>
> >>diff --git a/libavformat/utils.c b/libavformat/utils.c
> >>index 8196442dd1..c3c2c77c0c 100644
> >>--- a/libavformat/utils.c
> >>+++ b/libavformat/utils.c
> >>@@ -1838,6 +1838,11 @@ int av_read_frame(AVFormatContext *s, AVPacket *pkt)
> >>              }
> >>          }
> >>+        if (ff_check_interrupt(&s->interrupt_callback)) {
> >>+            av_log(s, AV_LOG_DEBUG, "interrupted\n");
> >>+            return AVERROR_EXIT;
> >>+        }
> >>+
> >I think this can be moved into the if() above, which might
> >reduce the number of calls.
> >
> >thx
> >[...]
> >
> It would probably reduce only one call, as pktl (the if above) is a list
> that will get filled probably as soon as there is a packet.  Or maybe I am
> reading the code wrong?  Also, if it does not get filled, we probably want
> to exit anyway, too.

we want to call ff_check_interrupt() between time consuming operations
but calling it before everything seems a bit odd to me.

Why did one call av_read_frame() if one wants to interrupt before doing
anything

Now if the list is empty the loop has just been entered so why would we
interrupt here ?
maybe iam missing something but this doesnt seem to be usefull

Thanks


[...]
diff mbox

Patch

diff --git a/libavformat/utils.c b/libavformat/utils.c
index 8196442dd1..c3c2c77c0c 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -1838,6 +1838,11 @@  int av_read_frame(AVFormatContext *s, AVPacket *pkt)
             }
         }
 
+        if (ff_check_interrupt(&s->interrupt_callback)) {
+            av_log(s, AV_LOG_DEBUG, "interrupted\n");
+            return AVERROR_EXIT;
+        }
+
         ret = read_frame_internal(s, pkt);
         if (ret < 0) {
             if (pktl && ret != AVERROR(EAGAIN)) {