| Summary: | media-video/mplayer: arch keywords | ||
|---|---|---|---|
| Product: | Gentoo Linux | Reporter: | Steve Dibb (RETIRED) <beandog> |
| Component: | New packages | Assignee: | Gentoo Media-video project <media-video> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | ||
| Priority: | High | ||
| Version: | unspecified | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Package list: | Runtime testing required: | --- | |
| Bug Depends on: | |||
| Bug Blocks: | 231836, 239130, 246972 | ||
| Attachments: | mplayer-libmpeg2-libavcodec-sparc.patch | ||
|
Description
Steve Dibb (RETIRED)
2008-10-10 13:12:02 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.
bsd done 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. (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. > 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 ppc is done as per #239130 So whats the status wrt sparc? It fails for me on gcc-4.1... 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)? (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 (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. (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. (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 :/ (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 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 Sparc stable for 1.0_rc2_p27725-r1, closing since we're last :) |