Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MACOSX: Use modern CoreAudio API on modern systems by default, cleanup #157

Merged
merged 2 commits into from
Jan 14, 2012
Merged

MACOSX: Use modern CoreAudio API on modern systems by default, cleanup #157

merged 2 commits into from
Jan 14, 2012

Conversation

fingolfin
Copy link
Contributor

The commit messages & changed content should hopefully be self-explanatory.

@sev-
Copy link
Member

sev- commented Jan 2, 2012

I agree that leaving just one combination with 6.5 years old OS is a fair enough tradeoff. Thus I am for not making the code too complex.

@bluegr
Copy link
Member

bluegr commented Jan 11, 2012

Judging from the opinions voiced, this looks ok to be merged?

@vinterstum
Copy link
Member

Yep, no objections here.

@vinterstum vinterstum closed this Jan 14, 2012
@vinterstum vinterstum reopened this Jan 14, 2012
@vinterstum
Copy link
Member

(oops)

The new API has been present since Mac OS X 10.5 (released four years ago,
in October 2007), and also since iOS 2.0 (thus, the iOS port may be able to
use it, too). Moreover, 10.5 was the last system to support PowerPC.
@fingolfin
Copy link
Contributor Author

Actually, here is a rebased version of the patch. I replaced the second commit by a different one: After looking at browser_mm.cpp, I realized that the check for PowerPC in it is not needed at all -- it apparently was an attempt to workaround the fact that the NSString method cStringUsingEncoding: does not exist before OS X 10.4. But since it was used with NSUTF8StringEncoding, one can simply use UTF8String instead, which has been around since 10.0.

@vinterstum
Copy link
Member

Nice. I'll merge this, but change the check slightly to the below, if that's ok, which should avoid the deprecation warnings for most people but allow the buildbot/official release builds to still work on Intel 10.4 (will have to be changed again when an OS X version is released which removes the deprecated functions, of course):

#if TARGET_CPU_PPC || TARGET_CPU_PPC64 || !defined(MAC_OS_X_VERSION_10_6)

@vinterstum
Copy link
Member

(Without that I'd have to upgrade the buildbot and my own release toolchain to the 10.5 SDK, which I'd rather avoid :) )

vinterstum added a commit that referenced this pull request Jan 14, 2012
MACOSX: Use modern CoreAudio API on modern systems by default, cleanup
@vinterstum vinterstum merged commit 85c8bd6 into scummvm:master Jan 14, 2012
@fingolfin
Copy link
Contributor Author

Unfortunately it now always uses the deprecated API (and in my original patch, it always used the new API). To fix this, you need to add

#include <AvailabilityMacros.h>

at the top of the file (say, after the #ifdef MACOSX).

@vinterstum
Copy link
Member

Good catch; fixed now. Thanks :).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants