Modify
Warning Please read the Ticket HowTo before creating or commenting on a ticket. Failure to do so may cause your ticket to be rejected or result in a slower response.

Opened 3 years ago

Closed 3 years ago

Last modified 3 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@…> 3 years ago.
configure.diff (2.4 KB) - added by Lawrence Rust <lvr@…> 3 years ago.
gallery-load.diff (542 bytes) - added by Lawrence Rust <lvr@…> 3 years ago.
gallery-path.diff (787 bytes) - added by Lawrence Rust <lvr@…> 3 years ago.
music-cdempty.diff (1.6 KB) - added by Lawrence Rust <lvr@…> 3 years ago.
music-dechndlr.diff (2.1 KB) - added by Lawrence Rust <lvr@…> 3 years ago.
music-main-mingw.diff (1.4 KB) - added by Lawrence Rust <lvr@…> 3 years ago.
music-pbox-null.diff (3.9 KB) - added by Lawrence Rust <lvr@…> 3 years ago.

Download all attachments as: .zip

Change History (27)

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

comment:1 Changed 3 years ago by robertm

  • Owner changed from nigel to robertm
  • Status changed from new to assigned

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 3 years ago by beirdo

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

comment:3 Changed 3 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 3 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 3 years ago by Lawrence Rust <lvr@…>

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

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

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

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

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

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

comment:5 Changed 3 years ago by robertm

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

if (!mdata)

return;

instead of all the reindentation?

comment:6 Changed 3 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 3 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 3 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 3 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 3 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 3 years ago by paulh

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

comment:12 Changed 3 years ago by paulh

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

comment:13 Changed 3 years ago by robertm

  • Owner changed from robertm to beirdo

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

comment:14 Changed 3 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 3 years ago by beirdo

  • Owner changed from beirdo to janne

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 3 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 3 years ago by beirdo

  • Owner changed from janne to beirdo

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

comment:18 Changed 3 years ago by Lawrence Rust

  • Resolution set to fixed
  • Status changed from assigned to closed

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 3 years ago by beirdo

  • Milestone changed from unknown to 0.25

Add Comment

Modify Ticket

Action
as closed .
The resolution will be deleted. Next status will be 'new'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.