diff mbox

[FFmpeg-devel] av_read_frame timeout

Message ID ba124bc7-9436-703b-e8b8-c5a4459243c3@gmail.com
State New
Headers show

Commit Message

Gonzalo Garramuño Nov. 17, 2019, 9:37 p.m. UTC
The following patch adds a timeout interrupt to av_read_frame to prevent 
it from hanging up the application.  This patch was proposed some years 
ago but was not applied back then.  I believe it is useful and should be 
considered for approval.

Comments

Michael Niedermayer Nov. 18, 2019, 5:12 p.m. UTC | #1
On Sun, Nov 17, 2019 at 06:37:58PM -0300, gga wrote:
> The following patch adds a timeout interrupt to av_read_frame to prevent it
> from hanging up the application.  This patch was proposed some years ago but
> was not applied back then.  I believe it is useful and should be considered
> for approval.

If this was proposed previously please provide a link to the previous patch/
discussion

Thanks

[...]
Gonzalo Garramuño Nov. 18, 2019, 9:55 p.m. UTC | #2
El 18/11/19 a las 14:12, Michael Niedermayer escribió:
> On Sun, Nov 17, 2019 at 06:37:58PM -0300, gga wrote:
>> The following patch adds a timeout interrupt to av_read_frame to prevent it
>> from hanging up the application.  This patch was proposed some years ago but
>> was not applied back then.  I believe it is useful and should be considered
>> for approval.
> If this was proposed previously please provide a link to the previous patch/
> discussion
>
> Thanks
>
> [...]

I only have the link to the proposed patch, not the discussion, as I did 
not write the patch.

https://ffmpeg.org/pipermail/libav-user/2017-February/010086.html

Paul B Mahol also asked in the libav-user list about the discussion to the person who wrote the patch and who would be more qualified than me to state why it was rejected back then.  Sorry I cannot be more helpful.  I tried searching the archives with google with no success, as not even the above email shows up (and I tried several terms).

Thanks.
Michael Niedermayer Nov. 19, 2019, 5:30 p.m. UTC | #3
On Sun, Nov 17, 2019 at 06:37:58PM -0300, gga wrote:
> The following patch adds a timeout interrupt to av_read_frame to prevent it
> from hanging up the application.  This patch was proposed some years ago but
> was not applied back then.  I believe it is useful and should be considered
> for approval.
> 

> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index 8196442dd1..d98ebe46a4 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -1792,6 +1792,12 @@ int av_read_frame(AVFormatContext *s, AVPacket *pkt)
>      }
>  
>      for (;;) {
> +        if (ff_check_interrupt(&s->interrupt_callback)) {
> +           ret = AVERROR_EXIT;
> +           av_log(s, AV_LOG_DEBUG, "interrupted\n");
> +           return ret;
> +        }
> +
>          AVPacketList *pktl = s->internal->packet_buffer;

In principle i dont see why this change would be bad (if it fixes some
use case)
but a few things needs to be corrected
the author of the commit has to be corrected
the indention is inconsistent
the ret variable is unneeded
also this mixes declarations and statements, it should be moved down

and i would move this so ff_check_interrupt() is only called if the loop
iterates and not on the first iteration

thx

[...]
diff mbox

Patch

diff --git a/libavformat/utils.c b/libavformat/utils.c
index 8196442dd1..d98ebe46a4 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -1792,6 +1792,12 @@  int av_read_frame(AVFormatContext *s, AVPacket *pkt)
     }
 
     for (;;) {
+        if (ff_check_interrupt(&s->interrupt_callback)) {
+           ret = AVERROR_EXIT;
+           av_log(s, AV_LOG_DEBUG, "interrupted\n");
+           return ret;
+        }
+
         AVPacketList *pktl = s->internal->packet_buffer;
 
         if (pktl) {