[Ffmpeg-devel] BeOS cleanup work in progress

[Michael Niedermayer]

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>