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

[Michael Niedermayer]

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

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No human being will ever know the Truth, for even if they happen to say it
by chance, they would not even known they had done so. -- Xenophanes
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20110111/68de075f/attachment.pgp>