[FFmpeg-devel] [PATCH] add timeout to udp_read

[Michael Niedermayer]

On Wed, Sep 23, 2009 at 10:30:26PM +0200, Hagen Schmidt wrote:
> On 15.09.2009 at 03:03, Michael Niedermayer wrote:
> > On Mon, Sep 14, 2009 at 10:29:53PM +0200, Hagen Schmidt wrote:
> > > On 11.09.2009 at 02:43, Michael Niedermayer wrote:
> > >  doc/ffplay-doc.texi |    3 +++
> > >  ffplay.c            |   10 +++++++++-
> > >  2 files changed, 12 insertions(+), 1 deletion(-)
> > > bfb89aafa9238a91f01626291e34d9d042e1db89  timeout.diff
> > > Index: ffplay.c
> > > =================================================================
> > >== --- ffplay.c	(Revision 19841)
> > > +++ ffplay.c	(Arbeitskopie)
> > > @@ -1863,10 +1863,16 @@
> > >  /* since we have only one decoding thread, we can use a global
> > >     variable instead of a thread local variable */
> > >  static VideoState *global_video_state;
> > > +static int64_t reference_time = 0;
> > > +static int timeout_network = 0;
> > >
> > >  static int decode_interrupt_cb(void)
> > >  {
> > > -    return (global_video_state &&
> > > global_video_state->abort_request); +    static int
> > > timeout_expired = 0;
> > > +    if (timeout_network && !timeout_expired && reference_time &&
> > > +        (timeout_expired = av_gettime() - reference_time >
> > > timeout_network * 1000)) +        fprintf(stderr, "Timeout for
> > > receiving network stream expired after %i msec\n",
> > > timeout_network); +    return (global_video_state &&
> > > global_video_state->abort_request) || timeout_expired; }
> > >
> > >  /* this thread gets the stream from the disk or the network */
> > 
> > i would suggest to write this slightly less obfuscated
> > maybe doing the assignment outside a multiline conditional ...
> ok, I split the multiline conditional into 2 conditionals.
> 
> 
> > also i wonder how timeout_expired could be reset or what good that
> >  function is once its non zero
> ok, set timeout_expired to file scope.
> Since timeout_expired was only used within that function, I limited 
> variable's scope to that function in the last patch. But as you wrote 
> it decreases function's usabiliy.
> Nevertheless I omit setting timeout_expired = 0 before calling 
> decode_interrupt_cb, because it's already initialised to 0.
> 
> 
> > > @@ -1896,6 +1902,7 @@
> > >      ap->time_base= (AVRational){1, 25};
> > >      ap->pix_fmt = frame_pix_fmt;
> > >
> > > +    reference_time = av_gettime();
> > >      err = av_open_input_file(&ic, is->filename, is->iformat, 0,
> > > ap); if (err < 0) {
> > >          print_error(is->filename, err);
> > 
> > i dont see reference_time time being reset or otherwise the timeout
> >  being disabled, maybe iam missing something but doesnt that kill
> >  ffplay at that time even long after av_open_input_file() ?
> You are right, I removed resetting of reference_time from last patch 
> by  mistake... I add it again.
> 
> Hagen

>  doc/ffplay-doc.texi |    3 +++
>  ffplay.c            |   13 ++++++++++++-
>  2 files changed, 15 insertions(+), 1 deletion(-)
> b1ebb5e59a4b4b608a521bdb0cd7c920eea6a7f1  timeout.diff
> Index: doc/ffplay-doc.texi
> ===================================================================
> --- doc/ffplay-doc.texi	(Revision 19933)
> +++ doc/ffplay-doc.texi	(Arbeitskopie)
> @@ -102,6 +102,9 @@
>  refers to the list of all the input subtitle streams. If it is greater
>  than the number of subtitle streams minus one, then the last one is
>  selected, if it is negative the subtitle rendering is disabled.
> + at item -nto @var{seconds}
> +Set timeout in milliseconds when opening a network stream shall be aborted.
> +Default is 0 which waits endlessly.
>  @end table
>  
>  @section While playing
> Index: ffplay.c
> ===================================================================
> --- ffplay.c	(Revision 19933)
> +++ ffplay.c	(Arbeitskopie)
> @@ -1863,10 +1863,18 @@
>  /* since we have only one decoding thread, we can use a global
>     variable instead of a thread local variable */
>  static VideoState *global_video_state;
> +static int64_t reference_time = 0;
> +static int timeout_network = 0;
> +static int timeout_expired = 0;
>  
>  static int decode_interrupt_cb(void)
>  {
> -    return (global_video_state && global_video_state->abort_request);
> +    if (timeout_network && !timeout_expired && reference_time) {
> +        timeout_expired = av_gettime() - reference_time > timeout_network * 1000;
> +        if (timeout_expired)
> +            fprintf(stderr, "Timeout for receiving network stream expired after %i msec\n", timeout_network);
> +    }
> +    return (global_video_state && global_video_state->abort_request) || timeout_expired;
>  }

is it neccesary to design this so complex?
dosnt
if (reference_time && av_gettime() - reference_time > timeout_network){
    fprintf(stderr, "Timeout for receiving network stream expired after %i msec\n", timeout_network);
    return 1;
}
return global_video_state && global_video_state->abort_request;

work as well?
here timeout_network could be set to UINT64_MAX for the disabled case

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Good people do not need laws to tell them to act responsibly, while bad
people will find a way around the laws. -- Plato
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20091007/38499776/attachment.pgp>