Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#9258 closed Patch - Bug Fix (fixed)

[PATCH] Fix mythplugins to cross compile and run on Windows

Reported by: Lawrence Rust <lvr@…> Owned by: beirdo
Priority: minor Milestone: 0.25
Component: Ports - Windows Version: Master Head
Severity: medium Keywords:
Cc: Ticket locked: no

Description

MythTV plugins (0.24, 0.24-fixes and trunk) don't cross compile for Windows. See http://www.softsystem.co.uk/mythtv for howto. The attached patch fixes some problems with configure when cross compiling. In addition the patch fixes the following Windows specific issues:

  1. Mythmusic compile.
  2. Mythmusic root folder starting with a drive letter and containing backslashes.
  3. Mythmusic null pointer derefs.
  4. Mythgallery not loading pictures during startup.
  5. Mythgallery infinite loop when parsing some paths.

Attachments (8)

mythplugins-0.24.diff (12.5 KB) - added by Lawrence Rust <lvr@…> 13 years ago.
configure.diff (2.4 KB) - added by Lawrence Rust <lvr@…> 13 years ago.
gallery-load.diff (542 bytes) - added by Lawrence Rust <lvr@…> 13 years ago.
gallery-path.diff (787 bytes) - added by Lawrence Rust <lvr@…> 13 years ago.
music-cdempty.diff (1.6 KB) - added by Lawrence Rust <lvr@…> 13 years ago.
music-dechndlr.diff (2.1 KB) - added by Lawrence Rust <lvr@…> 13 years ago.
music-main-mingw.diff (1.4 KB) - added by Lawrence Rust <lvr@…> 13 years ago.
music-pbox-null.diff (3.9 KB) - added by Lawrence Rust <lvr@…> 13 years ago.

Download all attachments as: .zip

Change History (27)

Changed 13 years ago by Lawrence Rust <lvr@…>

Attachment: mythplugins-0.24.diff added

comment:1 Changed 13 years ago by robertm

Owner: changed from Nigel to robertm
Status: newassigned

Lawrence,

I appreciate your past and present patches very much. The above is very difficult for me to apply all at once. Can you please break element out into a single patch so that they can be more effectively reviewed? They can all be tracked within this ticket, but applying the patch as a whole is probably a no-go.

Thanks.

comment:2 Changed 13 years ago by beirdo

I'd be happy to look over and work the mythgallery portion once this is split apart.

comment:3 Changed 13 years ago by robertm

Also, Lawrence, removing the disable the compilation of MythMusic when it has missing dependencies is probably not acceptable. Meeting them in the Windows Build script would be, however.

comment:4 Changed 13 years ago by Lawrence Rust <lvr@…>

OK, good points. I've split the patch as follows:

  • configure.diff. This also addresses comment3 above
  • gallery-load.diff. Fix mythgallery to load pictures for WIN32
  • gallery-path.diff. Fix an infinite loop in mythgallery with certain pathnames
  • music-cdempty.diff. Fix mythmusic to empty the current playlist before playing a CD
  • music-dechndlr.diff. Fix mythmusic to play files with a patname starting c:/...[[BR]]
  • music-main-mingw.diff. Fix mythmusic to compile with mingw
  • music-pbox-null.diff - Fix a number of null derefs in mythmusic playbackbox.cpp. These seem to be tickled more often on Win32.

Changed 13 years ago by Lawrence Rust <lvr@…>

Attachment: configure.diff added

Changed 13 years ago by Lawrence Rust <lvr@…>

Attachment: gallery-load.diff added

Changed 13 years ago by Lawrence Rust <lvr@…>

Attachment: gallery-path.diff added

Changed 13 years ago by Lawrence Rust <lvr@…>

Attachment: music-cdempty.diff added

Changed 13 years ago by Lawrence Rust <lvr@…>

Attachment: music-dechndlr.diff added

Changed 13 years ago by Lawrence Rust <lvr@…>

Attachment: music-main-mingw.diff added

Changed 13 years ago by Lawrence Rust <lvr@…>

Attachment: music-pbox-null.diff added

comment:5 Changed 13 years ago by robertm

Lawrence, in music-pbox-null.diff, how about just

if (!mdata)

return;

instead of all the reindentation?

comment:6 Changed 13 years ago by robertm

Also, don't think we need the comments everywhere to tell us how Windows absolute paths work, can probably remove those. We know that paths start with the driver letter on Windows. ;)

comment:7 Changed 13 years ago by Lawrence Rust <lvr@…>

inline return has the same effect but introduces multi-point returns which are IMHO so-so - can lead to resource leaks etc.

OK if you want to remove the Win32 comments, but leave one somewhere. It was because of the assumption that 'all paths lead to root' (sic) that it needed fixing.

comment:8 Changed 13 years ago by Lawrence Rust <lvr@…>

P.S. re. inline return. Without profiling info they can cause problems for the optimiser and the CPU's predictive branching. It's best for preventing pipeline stalls if the normal path just flows through. Otherwise gcc can generate a load of 'branch-over-and-back' style assembler.

comment:9 Changed 13 years ago by paulh

(In [27398]) MythMusic: Empty the current playlist before playing a CD and lock the cd reader thread while creating the UI trees. Patch by Lawrence Rust. Refs #9258.

comment:10 Changed 13 years ago by paulh

(In [27399]) MythMusic: Use the more Windows friendly QFileInfo::isAbsolute() to determine if a url points to a local file. Patch by Lawrence Rust. Refs #9258.

comment:11 Changed 13 years ago by paulh

(In [27400]) MythMusic: NULL pointer checks. Patch by Lawrence Rust. Refs #9258.

comment:12 Changed 13 years ago by paulh

(In [27402]) MythMusic: Hopefully temporary compile fix with mingw. Patch by Lawrence Rust. Refs #9258.

comment:13 Changed 13 years ago by robertm

Owner: changed from robertm to beirdo

The rest of this ticket is probably best fielded by Beirdo.

comment:14 Changed 13 years ago by beirdo

gallery-load.diff was incorporated as is in b0eb1ccf29

gallery-path.diff, I incorporated, then further reworked the section of code to get 9c2d32c32

Thanks.

comment:15 Changed 13 years ago by beirdo

Owner: changed from beirdo to Janne Grunau

I think that just leaves the configure script part, which I will leave for Janne, since it's his code, and I don't want to break it.

comment:16 Changed 13 years ago by Craig Treleaven <ctreleaven@…>

Re MythPlugins? configure: I believe it is currently broken for OS X, as well. 1) It disables MythNetvision? insisting that the Perl bindings aren't installed. Earlier in the osx-packager.pl run, the Perl bindings are reported available. 2) It disables MythMusic saying "MythMusic requires taglib 1.5 or later." Ver 1.5 is, in fact, built and installed. By commenting out these two checks, I can get both plugins to compile. From minimal testing, they both seem to be functional. Hope this isn't a me-too comment! Craig

comment:17 Changed 13 years ago by beirdo

Owner: changed from Janne Grunau to beirdo

I'll take this back as Janne is taking a break for the time being.

comment:18 Changed 13 years ago by Lawrence Rust

Resolution: fixed
Status: assignedclosed

Add targetos and sysroot support to mythplugins makefile

Closes #9258. This was the last thing left for that set of patches presented to allow better Windows compiling.

Changeset: 1582957bf693a26c2f7daddaf4b5af509363bb3d

comment:19 Changed 13 years ago by beirdo

Milestone: unknown0.25
Note: See TracTickets for help on using tickets.