GNOME Bugzilla – Bug 118473
mplex plugin does not work on Solaris
Last modified: 2004-12-22 21:47:04 UTC
The mplex plugin fails on Solaris due to a few problems. + Forte does not support __inline__. It does support "inline". + multplex.cc alocates sector_buf with "sector_size". The Forte compiler does not allow you to allocate arrays to be the size of a variable. It would work with Forte if you allocate this array with g_new and g_free. If you want, you could use #ifdef __SUNPRO_C to tell when Forte is being used, so that the current code could be retained for other compilers. + vector.cc fails to compile with this error: "vector.cc", line 12: Error: Cannot assign Aunit* to Aunit**. On this line: buf = new (Aunit *)[AUStream::BUF_SIZE]; Note that in vector.hh, that "buf" is defined to be "Aunit **buf", not an Aunit *, so something is clearly wrong here. Perhaps this error slips by gcc, but Forte noticed it. Two are addressed in the attached patch. I am not sure how to best address the last issue mentioned, since my first shot was killed. See bug 118304.
Created attachment 18672 [details] [review] patch to fix Forte compile problems
A few other Forte issues that I didn't mention before. + In gst-libs/ext/mplex/yuv4mpeg_ratio.cc The function strchr on Solaris returns "const char *". Therefore in the function y4m_parse_ratio, it would be better if "char *t" were changed to "const char *t", or a cast used as follows: char *t = (char *)strchr (s, ':'); In ext/mplex: + -lstdc++ is a GCC specific library, and not availabe with Forte. This library should only be included in libgstmplex_la_LIBADD if the compiler is GCC.
-lstdc++ should never be used. libtool-1.5 needs to be used, and libtool will take care of c++ dependencies automatically. Just like C tools don't use -lc, c++ tools should never use -lstdc++ directly in their Makefile.am. re: strchr(): you're right. Could you commit a fix that fixes both these? I'll look at your patch later today, I'm at work right now.
Created attachment 18677 [details] [review] updated patch
The updated patch addresses everything except the problem with vector.cc. Once again, it fails to compile with this error: "vector.cc", line 12: Error: Cannot assign Aunit* to Aunit**. On this line: buf = new (Aunit *)[AUStream::BUF_SIZE]; Note that in vector.hh, that "buf" is defined to be "Aunit **buf", not an Aunit *, so something is clearly wrong here. Perhaps this error slips by gcc, but Forte noticed it. Once again, I am not sure how to best address the last issue mentioned, since my first shot was killed. See bug 118304.
re: your patch, for the inline part, can't you just change all __inline__'s to inline? Not that I mind, both solutions are fine with me. However, for the other part (the part where you use g_new()), I'd appreciate if you didn't use glib inside here, since it's supposed to be backportable. Please use new and delete instead. For the last part, I don't know yet.
Created attachment 18683 [details] [review] updated patch to use new/delete
Created attachment 18684 [details] [review] fix to patch - removed glib.h include
Created attachment 18685 [details] [review] argh! this patch removes the broken fix to yuv4mpeg_ratio.cc
> "vector.cc", line 12: Error: Cannot assign Aunit* to Aunit**. This appears to be a bug in your compiler. The new returns an (Auint **), as I understand it.
Could we create the buf statically, as suggested in my patch. This technique works with the Forte compiler and, by avoiding calling new, avoids the problem.
Created attachment 19184 [details] [review] patch suggteting different way to allocate array the works with Forte
I've applied the pre-latest patch. I don't know whether making the buffer static is a good idea, please someone test (I can try it this weekend or so). That part is still open, rest is fixed.
seems reasonable, as long as compilers like it.