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

[Martin Storsjö]

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.

// Martin