[MPlayer-dev-eng] [PATCH] Fix for ARM compilation failure when?IWMMXT support is disabled

[Siarhei Siamashka]

On Monday 28 April 2008, Diego Biurrun wrote:
> On Mon, Apr 28, 2008 at 02:03:35AM +0300, Siarhei Siamashka wrote:
> > On Monday 28 April 2008, Diego Biurrun wrote:
> > > On Mon, Apr 28, 2008 at 12:44:42AM +0300, Siarhei Siamashka wrote:
> > > > --- libmpeg2/motion_comp.c	(revision 26548)
> > > > +++ libmpeg2/motion_comp.c	(working copy)
> > > > @@ -62,9 +62,12 @@
> > > >  #ifdef ARCH_ARM
> > > > +#ifdef HAVE_IWMMXT
> > > >      if (accel & MPEG2_ACCEL_ARM_IWMMXT)
> > > >  	mpeg2_mc = mpeg2_mc_iwmmxt;
> > > > -    else if (accel & MPEG2_ACCEL_ARM)
> > > > +    else
> > > > +#endif
> > > > +    if (accel & MPEG2_ACCEL_ARM)
> > > >  	mpeg2_mc = mpeg2_mc_arm;
> > > >      else
> > > >  #endif
> > >
> > > Hmm, I think this is wrong.  Why is MPEG2_ACCEL_ARM_IWMMXT defined if
> > > IWMMXT is not available?
> >
> > It is not defined, but we just get linking error:
> > [...]
> > libmpeg2/libmpeg2.a(motion_comp.o): In function
> > `mpeg2_mc_init':/home/serge/mplayer-svn/libmpeg2/motion_comp.c:68:
> > undefined reference to `mpeg2_mc_iwmmxt'
> > collect2: ld returned 1 exit status
>
> The bug must be elsewhere.  Where is mpeg2_mc_iwmmxt declared?  And why
> is it declared on your machine?

This code refers to 'mpeg2_mc_iwmmxt' symbol, which is defined
in 'motion_comp_iwmmxt.c' when HAVE_IWMMXT macro is enabled.

So either the code which refers to 'mpeg2_mc_iwmmxt' in 'motion_comp.c' should
be hidden under conditional compilation too, or something more cluttered
should be introduced.

Well, having one more look, turns out that now neither MPEG2_ACCEL_ARM nor
MPEG2_ACCEL_ARM_IWMMXT flags get ever set in 'accel' variable, so these
optimizations can not be activated. Looks like a lot has changed since I 
looked at this code last time.

> > And with the latest makefiles merging update, there is one more ARM
> > related breakage:
> > libmpeg2/motion_comp_arm_s.S:1: *** missing separator.  Stop.
> >
> > Looks like the rule for building *.S files is missing, but it is too late
> > today and I'm going to sleep.
>
> That's not the reason, but it is fixed now.

Thank you, fix confirmed.

> > PS. You can try installing ARM crosscompiler for testing when doing major
> > build system changes. No ARM hardware is required for this and the
> > problems would be spotted right away :)
>
> Nah, I prefer to wait for bug reports to trickle in :)

Well, you will have to deal with a number of reports from me then :)

-- 
Best regards,
Siarhei Siamashka