Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 241110 - media-video/mplayer: arch keywords
Summary: media-video/mplayer: arch keywords
Status: RESOLVED FIXED
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: New packages (show other bugs)
Hardware: All Linux
: High normal (vote)
Assignee: media-video herd
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 231836 CVE-2008-3827 246972
  Show dependency tree
 
Reported: 2008-10-10 13:12 UTC by Steve Dibb (RETIRED)
Modified: 2008-11-24 23:06 UTC (History)
0 users

See Also:
Package list:
Runtime testing required: ---


Attachments
mplayer-libmpeg2-libavcodec-sparc.patch (mplayer-libmpeg2-libavcodec.patch,695 bytes, patch)
2008-10-12 03:03 UTC, Friedrich Oslage (RETIRED)
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Steve Dibb (RETIRED) gentoo-dev 2008-10-10 13:12:02 UTC
After cleaning up use flags for arches, per bug 240347, mplayer should be fine to rekeyword for arches.  I just didn't want to do it myself, so if you guys can verify that the use flags are correct, please rekeyword ~ for:

=media-video/mplayer-1.0_rc2_p27458
=media-video/mplayer-1.0_rc2_p27725-r1

Target keywords: ~alpha ~x86-bsd ~ia64 ~ppc ~sparc

Thanks
Comment 1 Friedrich Oslage (RETIRED) gentoo-dev 2008-10-12 03:03:26 UTC
Created attachment 168112 [details, diff]
mplayer-libmpeg2-libavcodec-sparc.patch

The use flags/conditions seem to be fine, but the libmpeg2 and libavcodec they ship are broken for sparc:

1.0_rc2_p27458
  libmpeg2 is fine
  libavcodec has an undefined reference to ff_simple_idct_vis
1.0_rc2_p27725-r1
   libmpeg2 has undefined references to mpeg2_idct_copy_mvi, mpeg2_idct_add_mvi and mpeg2_idct_alpha_init
  libavcodec has an undefined reference to ff_simple_idct_vis

I attached a unified patch but it should probably be split into two patches to make it more flexible. Do you want to do that or do you want me to do it?
It also affects alpha but that shouldn't be a problem since alpha also needs to re-keyword 1.0_rc2_p27725-r1.
Comment 2 Alexis Ballier gentoo-dev 2008-10-15 12:50:26 UTC
bsd done
Comment 3 Christian Hoffmann (RETIRED) gentoo-dev 2008-10-19 09:52:04 UTC
We need =media-video/mplayer-1.0_rc2_p27725-r1 keyworded and stabled for security reasons. Please consider this bug a security issue.
Stabling for this version is handled in bug 239130.
Comment 4 Alexis Ballier gentoo-dev 2008-10-19 14:51:18 UTC
(In reply to comment #1)
> 1.0_rc2_p27725-r1
>    libmpeg2 has undefined references to mpeg2_idct_copy_mvi, mpeg2_idct_add_mvi
> and mpeg2_idct_alpha_init

-#ifdef HAVE_VIS
+#ifdef ARCH_ALPHA
     if (accel & MPEG2_ACCEL_ALPHA_MVI) {
 	mpeg2_idct_copy = mpeg2_idct_copy_mvi;
 	mpeg2_idct_add = mpeg2_idct_add_mvi;
 	mpeg2_idct_alpha_init ();
     } else


this should probably be HAVE_MVI and be sent upstream for discussion

>   libavcodec has an undefined reference to ff_simple_idct_vis

+extern void ff_simple_idct_vis(DCTELEM *data);
 inline void ff_simple_idct_vis(DCTELEM *data) {

weird; does ffmpeg-0.4.9_p20081014 exhibits the same problem ? I don't understand why you need to define twice the function but to me this doesn't look correct. Code hasn't changed here for a while.
Comment 5 Friedrich Oslage (RETIRED) gentoo-dev 2008-10-20 22:15:40 UTC
> this should probably be HAVE_MVI and be sent upstream for discussion

It was ARCH_ALPHA before they changed it to HAVE_VIS, so I just changed it back. But yes, HAVE_MVI would probably be even better. Do you want me to ping upstream or do it yourself?

http://svn.mplayerhq.hu/mplayer/trunk/libmpeg2/idct.c?r1=26657&r2=27554

> weird; does ffmpeg-0.4.9_p20081014 exhibits the same problem ? I don't
> understand why you need to define twice the function but to me this doesn't
> look correct. Code hasn't changed here for a while.

Yes it does.
There are basicly three ways to define a function as inline:
  - static inline void function()
  - extern inline void function()
and
  - extern void function(); inline void function()
They do already have the "extern void ff_simple_idct_vis(DCTELEM *data);" but it's in dsputil_vis.c and it also needs to be in simple_idct_vis.c(or in a header file)
GCC 4.1 is able to compile this correctly - that's why we didn't detect it until now - but 4.3 is not. Since our current stable is 4.1 we could ignore it for now, but we have to fix it eventually ;)

http://www.greenend.org.uk/rjk/2003/03/inline.html
Comment 6 Tobias Scherbaum (RETIRED) gentoo-dev 2008-10-30 20:09:10 UTC
ppc is done as per #239130
Comment 7 Raúl Porcel (RETIRED) gentoo-dev 2008-11-08 16:35:50 UTC
So whats the status wrt sparc?
It fails for me on gcc-4.1...
Comment 8 Tobias Klausmann (RETIRED) gentoo-dev 2008-11-09 14:58:43 UTC
I've stabilised 1.0_rc2_p27725-r1 already (sec bug 239130 )

Is there a point in keywording the older version (1.0_rc2_p27458)?

Comment 9 Raúl Porcel (RETIRED) gentoo-dev 2008-11-10 11:24:34 UTC
(In reply to comment #8)
> I've stabilised 1.0_rc2_p27725-r1 already (sec bug 239130 )
> 
> Is there a point in keywording the older version (1.0_rc2_p27458)?
> 

That version doesn't exist anymore. ~ia64 done
Comment 10 Alexis Ballier gentoo-dev 2008-11-10 12:49:25 UTC
(In reply to comment #7)
> So whats the status wrt sparc?
> It fails for me on gcc-4.1...

beh, seems I hadn't seen Friedrich's reply :(

(In reply to comment #5)
> > this should probably be HAVE_MVI and be sent upstream for discussion
> 
> It was ARCH_ALPHA before they changed it to HAVE_VIS, so I just changed it
> back. But yes, HAVE_MVI would probably be even better. Do you want me to ping
> upstream or do it yourself?

They include a bundled version of libmpeg2 and wrongly patch it... nice :)
Please ping upstream, as this is sparc specific they may want more information which i wouldn't be able to provide (though as it seems to be a small typo, I really doubt it).
 
> > weird; does ffmpeg-0.4.9_p20081014 exhibits the same problem ? I don't
> > understand why you need to define twice the function but to me this doesn't
> > look correct. Code hasn't changed here for a while.
> 
> Yes it does.
> There are basicly three ways to define a function as inline:
>   - static inline void function()
>   - extern inline void function()
> and
>   - extern void function(); inline void function()
> They do already have the "extern void ff_simple_idct_vis(DCTELEM *data);" but
> it's in dsputil_vis.c and it also needs to be in simple_idct_vis.c(or in a
> header file)
> GCC 4.1 is able to compile this correctly - that's why we didn't detect it
> until now - but 4.3 is not. Since our current stable is 4.1 we could ignore it
> for now, but we have to fix it eventually ;)
> 
> http://www.greenend.org.uk/rjk/2003/03/inline.html

Ok, or maybe just drop the inline... I'm not sure there: ff_simple_idct_mmx isn't inline as far as i can see.
Anyway, please briefly read http://ffmpeg.mplayerhq.hu/general.html#SEC28 and send that patch to ffmpeg-devel mailing list (https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel/). It'll quickly get reviewed and applied/rejected; I'd really prefer to have reviewed patches as this is a stable candidate version. Once a patch reaches ffmpeg svn, feel free to apply it to ffmpeg & mplayer ebuilds I'd say.
Comment 11 Steve Dibb (RETIRED) gentoo-dev 2008-11-11 00:53:07 UTC
(In reply to comment #10)
> (In reply to comment #7)
> > So whats the status wrt sparc?
> > It fails for me on gcc-4.1...
> 
> beh, seems I hadn't seen Friedrich's reply :(
> 
> (In reply to comment #5)
> > > this should probably be HAVE_MVI and be sent upstream for discussion
> > 
> > It was ARCH_ALPHA before they changed it to HAVE_VIS, so I just changed it
> > back. But yes, HAVE_MVI would probably be even better. Do you want me to ping
> > upstream or do it yourself?
> 
> They include a bundled version of libmpeg2 and wrongly patch it... nice :)
> Please ping upstream, as this is sparc specific they may want more information
> which i wouldn't be able to provide (though as it seems to be a small typo, I
> really doubt it).
> 
> > > weird; does ffmpeg-0.4.9_p20081014 exhibits the same problem ? I don't
> > > understand why you need to define twice the function but to me this doesn't
> > > look correct. Code hasn't changed here for a while.
> > 
> > Yes it does.
> > There are basicly three ways to define a function as inline:
> >   - static inline void function()
> >   - extern inline void function()
> > and
> >   - extern void function(); inline void function()
> > They do already have the "extern void ff_simple_idct_vis(DCTELEM *data);" but
> > it's in dsputil_vis.c and it also needs to be in simple_idct_vis.c(or in a
> > header file)
> > GCC 4.1 is able to compile this correctly - that's why we didn't detect it
> > until now - but 4.3 is not. Since our current stable is 4.1 we could ignore it
> > for now, but we have to fix it eventually ;)
> > 
> > http://www.greenend.org.uk/rjk/2003/03/inline.html
> 
> Ok, or maybe just drop the inline... I'm not sure there: ff_simple_idct_mmx
> isn't inline as far as i can see.
> Anyway, please briefly read http://ffmpeg.mplayerhq.hu/general.html#SEC28 and
> send that patch to ffmpeg-devel mailing list
> (https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel/). It'll quickly get
> reviewed and applied/rejected; I'd really prefer to have reviewed patches as
> this is a stable candidate version. Once a patch reaches ffmpeg svn, feel free
> to apply it to ffmpeg & mplayer ebuilds I'd say.
> 

aballier, would you mind pinging them yourself since you know what you're talking about? :)  I'd have a hard time explaining it if any objections came up.
Comment 12 Alexis Ballier gentoo-dev 2008-11-12 18:26:49 UTC
(In reply to comment #11)
> aballier, would you mind pinging them yourself since you know what you're
> talking about? :)  I'd have a hard time explaining it if any objections came
> up.

yes i could too, but that's the same for me: i'd have hard time explaining/doing further tests if some objections come up; i'd need a machine to test it which i don't have :/
Comment 13 Raúl Porcel (RETIRED) gentoo-dev 2008-11-23 12:09:35 UTC
(In reply to comment #12)
> (In reply to comment #11)
> > aballier, would you mind pinging them yourself since you know what you're
> > talking about? :)  I'd have a hard time explaining it if any objections came
> > up.
> 
> yes i could too, but that's the same for me: i'd have hard time
> explaining/doing further tests if some objections come up; i'd need a machine
> to test it which i don't have :/
> 

We can provide access to it. Ping me on irc if you want to
Comment 14 Steve Dibb (RETIRED) gentoo-dev 2008-11-24 21:53:43 UTC
Thanks aballier,

patch accepted upstream:

http://lists.mplayerhq.hu/pipermail/mplayer-cvslog/2008-November/035850.html

I'm going to apply the patch to the same ebuild (1.0_rc2_p27725-r1).  Every arch is stable but sparc, can you please test /stable it 
Comment 15 Friedrich Oslage (RETIRED) gentoo-dev 2008-11-24 23:06:49 UTC
Sparc stable for 1.0_rc2_p27725-r1, closing since we're last :)