Hi On Thu, Jan 25, 2007 at 09:53:42AM +0100, Fran?ois Revol wrote: > > > > and there is some code duplication in the patch which has to be > > > > removed > > > > (yes the E* -> AVERROR_* mapping swiches) > > > > > > I originally wrote a func for that, but there were only 2... > > > Did a static func in avcodec.h > > > > i do not agree with these functions in avcodec.h, i also do not agree > > with > > moving them into libavutil as its IMO bloat from libavutils point of > > view > > Someone argued there was code dup in libavformat/tcp.c (2 occurences, > see above). yes > So I factored it out there in the first one. yes but you droped the code in a public header it 1. shouldnt be in a header and 2. shouldnt be public there are 2 uses in tcp.c and none anywhere else? factoring the code out here obviously means putting the new function as static (NOT inline) function in tcp.c > So please make your mind :D see above > > As for which lib they go into, well there were several return -EFOO in > libavcodec as well. And someone suggested moving errors there. > I can move that back to avformat, and change others to return -1. > I don't care as long as it's not those ugly -EFOO. after thinking a little about this i feel less and less comfortable with the whole change both my copy of the c standard and "info libc" says that E* MUST be positive, so -EFOO seems the simplest and cleanest way to do this ... also abs(AVERROR_*) if we do agree on them should have the same value as abs(E*) that way mapping from one to the other becomes much simpler [...] > > > +#define AVERROR_TRYAGAIN (-11) /* busy at the moment... */ > > > > ok, but please change the comments to doxygen compatible ones /**< i > > think */ > > note, this can be done in a seperate patch/commit if you like > > I didn't change a thing in the comments, but I can add that I suppose. yes, thats why i said seperate commit/patch > > > > - return errno; > > > + return av_error_from_errno(errno); > > > > hmm this should be a AVERROR_NOMEM i think? if so it should > > be changed in a seperate commit before the other AVERROR stuff > > No idea, I didn't write that code. sure but the correctness of this affects the need and placement of some function you add so your change depends on first having the above resolved [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I have often repented speaking, but never of holding my tongue. -- Xenocrates -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 189 bytes Desc: not available URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070125/85541ed5/attachment.pgp>
RetroSearch is an open source project built by @garambo | Open a GitHub Issue
Search and Browse the WWW like it's 1997 | Search results from DuckDuckGo
HTML:
3.2
| Encoding:
UTF-8
| Version:
0.7.4