[Ffmpeg-devel] BeOS cleanup work in progress

Michael Niedermayer michaelni
Thu Jan 25 21:55:41 CET 2007


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).


> 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>

More information about the ffmpeg-devel mailing list