ryanheise / audio_session

MIT License
117 stars 81 forks source link

Use --dart-define to optionally disable the microphone code #25

Open ryanheise opened 3 years ago

ryanheise commented 3 years ago

Theoretically, it seems like this idea might work.

As it stands, when apps get submitted to the app store, Apple's automated analyser will look at which APIs are statically compiled into your code and if you're symbolically referencing an API such as the microphone, it will ask you to include a user string explaining your use of the microphone. This analyser is not smart enough (and I guess cannot be smart enough) to know that audio_service is like a 3rd party SDK and an app isn't necessarily going to use all features of the SDK, so it will ask you to explain your use of the microphone even if you don't use it.

So with --dart-define we may be able to pass in a compile-time option and then use the preprocessor in the iOS code to selectively disable parts of the code.

This isn't strictly necessary, an app can always just include the user string for the microphone that says "This app does not use the microphone" or something similar, but maybe it'd still be a good idea to implement the dart-define option sometime down the road.

abs0 commented 3 years ago

This would be very handy for us - we've just started using audio_session and currently have a modified copy to avoid the RecordPermission related calls to avoid triggering Apple's ITMS-90683 check.

Obviously if we're not triggering any of the code users will not see a popup asking for microphone access when using the app, but we'd prefer the App Privacy section in the Store to not mention microphone access at all

ryanheise commented 3 years ago

It's just an idea on the wishlist at this stage and it may be a while before I can actually get around to it (working hard on the next audio_service release right now). But if someone else wants to take a crack at it, you would be very welcome to help out!

lukepighetti commented 3 years ago

I am also running into this issue. Would be nice if there was a clean way to handle it.

permission_handler uses code in Podfile to handle this, for what it's worth.

ryanheise commented 3 years ago

Thanks for sharing @lukepighetti . I'm happy to go with that approach since --dart-define is not as convenient as I had hoped. (Another option could be to do something with .xcconfig files.)

ryanheise commented 3 years ago

The latest commit implements @lukepighetti 's suggestion (explained in the README). I'm not sure of the comprehensive list of APIs that need to be compiled out for it to pass the app store's privacy test but let me know if there's any more code I should compile out and I can make the change.

ryanheise commented 3 years ago

FYI you can test this by adding the following to the dependency_overrides section of your pubspec.yaml:

​audio_session: 
    git:
      url: https://github.com/ryanheise/audio_session.git

I would appreciate hearing about whether it works for you or whether there are any issues. If all is well I'll publish the next release.

abs0 commented 3 years ago

FYI you can test this by adding the following to the dependency_overrides section of your pubspec.yaml:

​audio_session: 
    git:
      url: https://github.com/ryanheise/audio_session.git

I would appreciate hearing about whether it works for you or whether there are any issues. If all is well I'll publish the next release.

I think you may need to add an #ifundef MICROPHONE_ENABLED to default it to 1 for users who do not add the Podfile chunk

(Many thanks for this change)

David

lukepighetti commented 3 years ago

I haven't tested it but defaulting to all working and opt-in to remove is my preference, I believe that's also how permissions_handler works which is kind of the blueprint for all others because it's so ubiquitous

abs0 commented 3 years ago

FYI you can test this by adding the following to the dependency_overrides section of your pubspec.yaml:

​audio_session: 
    git:
      url: https://github.com/ryanheise/audio_session.git

I would appreciate hearing about whether it works for you or whether there are any issues. If all is well I'll publish the next release.

I think you may need to add an #ifundef MICROPHONE_ENABLED to default it to 1 for users who do not add the Podfile chunk

(Many thanks for this change)

David

Ah - I see one is in there already, which makes the following unexpected?

    Command CompileSwift failed with a nonzero exit code
    In file included from /Users/abs/.pub-cache/git/audio_session-9e435529b1314e84e77ef40ee6c2d58c648dd104/ios/Classes/DarwinAudioSession.m:1:
    /Users/abs/.pub-cache/git/audio_session-9e435529b1314e84e77ef40ee6c2d58c648dd104/ios/Classes/DarwinAudioSession.h:4:31: warning: ISO C99 requires whitespace after the macro name [-Wc99-extensions]
        #define MICROPHONE_ENABLED=1
                                  ^
    /Users/abs/.pub-cache/git/audio_session-9e435529b1314e84e77ef40ee6c2d58c648dd104/ios/Classes/DarwinAudioSession.m:214:5: error: invalid token at start of a preprocessor expression
    #if MICROPHONE_ENABLED
        ^
    In file included from /Users/abs/.pub-cache/git/audio_session-9e435529b1314e84e77ef40ee6c2d58c648dd104/ios/Classes/DarwinAudioSession.m:1:
    /Users/abs/.pub-cache/git/audio_session-9e435529b1314e84e77ef40ee6c2d58c648dd104/ios/Classes/DarwinAudioSession.h:4:31: note: expanded from macro 'MICROPHONE_ENABLED'
    /Users/abs/.pub-cache/git/audio_session-9e435529b1314e84e77ef40ee6c2d58c648dd104/ios/Classes/DarwinAudioSession.m:222:5: error: invalid token at start of a preprocessor expression
    #if MICROPHONE_ENABLED
        ^
    In file included from /Users/abs/.pub-cache/git/audio_session-9e435529b1314e84e77ef40ee6c2d58c648dd104/ios/Classes/DarwinAudioSession.m:1:
    /Users/abs/.pub-cache/git/audio_session-9e435529b1314e84e77ef40ee6c2d58c648dd104/ios/Classes/DarwinAudioSession.h:4:31: note: expanded from macro 'MICROPHONE_ENABLED'
        #define MICROPHONE_ENABLED=1
                                  ^
    /Users/abs/.pub-cache/git/audio_session-9e435529b1314e84e77ef40ee6c2d58c648dd104/ios/Classes/DarwinAudioSession.m:478:5: error: invalid token at start of a preprocessor expression
    #if MICROPHONE_ENABLED
        ^
    In file included from /Users/abs/.pub-cache/git/audio_session-9e435529b1314e84e77ef40ee6c2d58c648dd104/ios/Classes/DarwinAudioSession.m:1:
    /Users/abs/.pub-cache/git/audio_session-9e435529b1314e84e77ef40ee6c2d58c648dd104/ios/Classes/DarwinAudioSession.h:4:31: note: expanded from macro 'MICROPHONE_ENABLED'
        #define MICROPHONE_ENABLED=1
                                  ^
    /Users/abs/.pub-cache/git/audio_session-9e435529b1314e84e77ef40ee6c2d58c648dd104/ios/Classes/DarwinAudioSession.m:492:5: error: invalid token at start of a preprocessor expression
    #if MICROPHONE_ENABLED
        ^
    In file included from /Users/abs/.pub-cache/git/audio_session-9e435529b1314e84e77ef40ee6c2d58c648dd104/ios/Classes/DarwinAudioSession.m:1:
    /Users/abs/.pub-cache/git/audio_session-9e435529b1314e84e77ef40ee6c2d58c648dd104/ios/Classes/DarwinAudioSession.h:4:31: note: expanded from macro 'MICROPHONE_ENABLED'
        #define MICROPHONE_ENABLED=1
                                  ^
    /Users/abs/.pub-cache/git/audio_session-9e435529b1314e84e77ef40ee6c2d58c648dd104/ios/Classes/DarwinAudioSession.m:586:5: error: invalid token at start of a preprocessor expression
    #if MICROPHONE_ENABLED
        ^
    In file included from /Users/abs/.pub-cache/git/audio_session-9e435529b1314e84e77ef40ee6c2d58c648dd104/ios/Classes/DarwinAudioSession.m:1:
    /Users/abs/.pub-cache/git/audio_session-9e435529b1314e84e77ef40ee6c2d58c648dd104/ios/Classes/DarwinAudioSession.h:4:31: note: expanded from macro 'MICROPHONE_ENABLED'
        #define MICROPHONE_ENABLED=1
ryanheise commented 3 years ago

Ah, sorry. There shouldn't be an equals sign, there should be a space:

#define MICROPHONE_ENABLED 1

It's been a while since I've used the preprocessor.

I'm also thinking of renaming this macro to something more unique to audio_session in case there is ever a name clash with other plugins.

ryanheise commented 3 years ago

Should be fixed on the latest commit. I have also renamed the macro to AUDIO_SESSION_MICROPHONE.

abs0 commented 3 years ago

Should be fixed on the latest commit. I have also renamed the macro to AUDIO_SESSION_MICROPHONE.

Perfick - just had a happy upload to TestFlight with AUDIO_SESSION_MICROPHONE=0

Thanks again

ryanheise commented 3 years ago

Glad to hear it!

I have just published the latest updates in 0.1.3.

ryanheise commented 1 year ago

I haven't tested it but defaulting to all working and opt-in to remove is my preference, I believe that's also how permissions_handler works which is kind of the blueprint for all others because it's so ubiquitous

As it turns out, permissions_handler has flipped to the opt-in so that by default nothing works and you must add an option to compile that code in.

I think we should do the same for audio_session. Of course for audio_session it would not be true that the default means "nothing works" since everything except for the microphone should work by default. This will also make the platform setup easier for just_audio.

Of course it will technically be a breaking change which I have avoided until now.

Any thoughts before I go ahead with this?