[FFmpeg-devel] Behaviour of url_read_complete

[Art Clarke]

On Fri, Jan 22, 2010 at 2:25 PM, Reimar D?ffinger
<Reimar.Doeffinger at gmx.de>wrote:

> On Fri, Jan 22, 2010 at 07:33:23PM +0100, Reimar D?ffinger wrote:
> > Hello,
> > I am a bit unsure about the purpose of url_read_complete.
> > However I would find it more convenient if its behaviour was as
> > with patch below.
> > What are your opinions?
> > The users of it in FFmpeg I saw would still work with that change.
>
> Here is a proper patch.
>

This negatively impacts our Octopus media server (
http://www.xuggle.com/octopus) when reading streaming media.  Our demuxers
share threads with other objects doing work, and when reading network
sources and getting EAGAIN we were using that opportunity to share
(cooperatively) the thread to do other work.  Now, effectively the thread is
blocked doing a hot spin loop on the socket waiting for data which means we
have to move those other jobs on to other threads (which changes our scaling
model as the thread-context switch is not desired).  Under conditions where
a RTMP, RTP or HTTP server we're reading from is temporarily slow, it now
slows down our entire media server very noticeably.  Boo hoo :(

My request is that the original patch be reverted, but I understand why it
was made.

Assuming it won't get reverted I've modified our upstream version of FFmpeg
to achieve the same goals as your patch, but let other users be able to
configure it back to the old behaviour.   The attached patch is running
through our test system as we speak (takes about a day to test all OS
combinations), but it lets people modify their URLContext to configure
whether they want the EAGAIN spin loop or not.  I submit it for your
consideration.

Thanks,

- Art

-- 
http://www.xuggle.com/
xu?ggle (z?' gl) v. To freely encode, decode, and experience audio and
video.

Use Xuggle to get the power of FFmpeg in Java.
-------------- next part --------------
Index: libavformat/avio.c
===================================================================
--- libavformat/avio.c	(revision 21400)
+++ libavformat/avio.c	(working copy)
@@ -87,6 +87,7 @@
     uc->flags = flags;
     uc->is_streamed = 0; /* default = not streamed */
     uc->max_packet_size = 0; /* default: stream file */
+    uc->max_retries = -1; /* default: retry until all data gotten */
     err = up->url_open(uc, filename, flags);
     if (err < 0) {
         av_free(uc);
@@ -151,16 +152,21 @@
 
 int url_read_complete(URLContext *h, unsigned char *buf, int size)
 {
-    int ret, len;
+    int ret, len, retries;
 
     len = 0;
+    retries = 0;
     while (len < size) {
         ret = url_read(h, buf+len, size-len);
         if (ret == AVERROR(EAGAIN)) {
-            ret = 0;
+            if (h->max_retries < 0 || retries < h->max_retries)
+              ret = 0;
+            else
+              return ret;
         } else if (ret < 1)
             return ret < 0 ? ret : len;
         len += ret;
+        ++retries;
     }
     return len;
 }
Index: libavformat/avio.h
===================================================================
--- libavformat/avio.h	(revision 21400)
+++ libavformat/avio.h	(working copy)
@@ -51,6 +51,7 @@
     int max_packet_size;  /**< if non zero, the stream is packetized with this max packet size */
     void *priv_data;
     char *filename; /**< specified filename */
+    int max_retries; /**< max number of times {@link url_read_complete} will retry on EAGAIN; < 0 is infinite; default = -1 */
 } URLContext;
 
 typedef struct URLPollEntry {
@@ -76,6 +77,9 @@
  * This makes special short-read handling in applications
  * unnecessary, if the return value is < size then it is
  * certain there was either an error or the end of file was reached.
+ *
+ * To override how many times this function retries on AVERROR(EAGAIN)
+ * see {@link URLContext#max_retries}.
  */
 int url_read_complete(URLContext *h, unsigned char *buf, int size);
 int url_write(URLContext *h, unsigned char *buf, int size);