[MPlayer-dev-eng] [PATCH] Switch libavutil macros instead of defining our own

Reimar Döffinger Reimar.Doeffinger at stud.uni-karlsruhe.de
Sat Feb 24 20:51:10 CET 2007


Hello,
On Sat, Feb 24, 2007 at 07:46:25PM +0100, Roberto Togni wrote:
> On Sat, 24 Feb 2007 18:35:10 +0100
> Reimar Döffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de> wrote:
> > attached patch replaces MIN, MAX, BE_32 and BE_16 in some places with
> > the libavutil equivalent.
> > Do you think this is okay despite it's size?
> > I assume it would also fix
> > http://bugzilla.mplayerhq.hu/show_bug.cgi?id=767 along the way, though
> > that is not really the main intention (and could be done by copying the
> > definitions from rmff.c instead).
> 
> About the header includes, shouldn't it be
> 
> #ifdef USE_LIBAVUTIL_SO
> #include "ffmpeg/intreadwrite.h
> #else
> #include "libavutil/intreadwrite.h
> #endif
> 
> like it's done in real.c for md5.h ?

IMO no, see the other thread.

> Also real.c #include "mpbswap.h", that line should be removed too since
> it's useless after this patch.

Done locally

> For rmff.c you can make the patch much shorter by 
> #define BE_16(x) AV_RB16(x)
> #define BE_32(x) AV_RB32(x)

Yes, that was mostly what I was asking about. It would make the patch
shorter, but actually replacing has the benefit of having more "uniform"
code.
I could also first do this macro variant and then a cosmetic patch that
does the actual renaming and some alignment like Michael suggested.
Just tell your opinion (even if you don't care about this code
specifically, there are probably more place and in the long term I'd
like to have them all migrated.

> but I accept also your version of the patch, in my opinion this code is
> getting so different from xine that we can stop considering xine
> version as upstream and take our code as a fork that is developed
> independently.

Reminds me of xine's copy of our loader code. I quickly gave up an
attempt to merge them last time, unfortunate as it may be.

Greetings,
Reimar Döffinger



More information about the MPlayer-dev-eng mailing list