[FFmpeg-devel] [PATCH] aviobuf: Retry if the read/write function returns AVERROR(EAGAIN)

[Martin Storsjö]

On Tue, 11 Jan 2011, Michael Niedermayer wrote:

> On Tue, Jan 11, 2011 at 07:22:56PM +0200, Martin Storsj? wrote:
> > On Tue, 11 Jan 2011, Michael Niedermayer wrote:
> > 
> > > On Fri, Jan 07, 2011 at 03:00:54PM +0200, Martin Storsj? wrote:
> > > > On Fri, 7 Jan 2011, Michael Niedermayer wrote:
> > > > 
> > > > > > As for whether they are allowed to return EAGAIN - url_read_complete
> > > > > > already handles EAGAIN, so url_read seems to be allowed to return it, 
> > > > > > which ByteIOContext uses directly at the moment.
> > > > > 
> > > > > > Not sure if write_packet
> > > > > > is allowed to return it though - url_write hides it at least.
> > > > > 
> > > > > If none of the current protocols can generate EAGAIN on write_packet() then
> > > > > we could just disallow it for future protocols to do it
> > > > > 
> > > > > If some of the current protocols can generate EAGAIN on write_packet() then we
> > > > > need to handle this somewhere, either in the protocols or the callers. Where
> > > > > its easier depends probably on how many protocols are affected and if the
> > > > > code could benefit from being protocol specific
> > > > 
> > > > I assume this (and/or the paragraph above) refers to read_packet(), since 
> > > > it's more of an issue there than in write_packet() I think.
> > > > 
> > > > I'm not sure if any of the current protocols do return it - I was about to 
> > > > add a codepath where it would be returned, triggering me to write this 
> > > > patch, but I managed to avoid that particular case.
> > > > 
> > > > > We also could change the API to use 2 size parameters.
> > > > > One that specifies the minimum that we need and one the maximum we could accept
> > > > > then all the retry code could be put inside protocols and still allow flexible
> > > > > blocking and non blocking reads. It of course doesnt really avoid the retry
> > > > > need but it might simplify API by reducing the duplication of complete and non
> > > > > complete IO functions
> > > > 
> > > > Perhaps, although the current API is quite acceptable I think.
> > > > 
> > > > > In the end i dont know what is best, i know though what is worst and that is
> > > > > leaving it buggy ...
> > > > 
> > > > Agreed.
> > > > 
> > > > Another way of solving it is as in attached. This has much less code 
> > > > duplication, but adds a new public function instead.
> > > 
> > > the patches should be ok if tested
> > > though i must admit the api is becoming a little bloated
> > 
> > Hmm, so you prefer this extra function to the small retry wrapper in 
> > abiobuf.c as I posted initially?
> > 
> > If we want to avoid the API bloat, it could of course also be added as an 
> > lavf-internal function with a ff_ prefix.
> 
> i feel that having 2+ read functions at a single API layer (and we have
> more API layers with read functions) is a bit ugly
> 
> Between the patches i have no strong preference if they all work, apply
> what you prefer

Applied the one I preferred, adding a small retry wrapper in aviobuf, 
avoiding adding yet another public API function.

// Martin