twilio / video-quickstart-ios

Twilio Video Quickstart for iOS
https://www.twilio.com/docs/api/video
MIT License
465 stars 178 forks source link

Error when adding app audio track in ReplayKit SampleHandler #339

Closed MilanNosal closed 4 years ago

MilanNosal commented 5 years ago

Description

I am trying to add app audio using ExampleCoreAudioDeviceRecordCallback(sampleBuffer) to the broadcast extension. However, when I use the following code to add it, I got a crash from the broadcast extension:

Code

    override func processSampleBuffer(_ sampleBuffer: CMSampleBuffer, with sampleBufferType: RPSampleBufferType) {
        switch sampleBufferType {
        case RPSampleBufferType.video:
            videoSource?.processVideoSampleBuffer(sampleBuffer)
            break
        case RPSampleBufferType.audioApp:
            // trying to add app audio
            ExampleCoreAudioDeviceRecordCallback(sampleBuffer)
            break
        case RPSampleBufferType.audioMic:
            // I don't want mic audio
            break
        }
    }

Otherwise I use the code from your latest ReplayKit example. It is reproducible with ReplayKit example on 2.6.0-preview branch of video-quickstart-swift, you just need to move the ExampleCoreAudioDeviceRecordCallback(sampleBuffer) from RPSampleBufferType.audioMic to RPSampleBufferType.audioApp.

Expected Behavior

Broadcasting the screen capture along with the system audio (not mic)

Actual Behavior

The broadcast extension crashes

Logs

didConnectToRoom:  <<TVIRoom: 0x283e9ed80> name: 4D149C63-8BD7-4CCB-A37F-04DDDE6175AA, state: Connected, sid: RM8043cedde0c5210929792b120f3edf55, delegate: <squad_broadcast.SampleHandler: 0x283b950a0>>
2018-12-17 11:05:51.289171+0100 squad-broadcast[635:8535] *** Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: 'The supplied sizeInBytes is invalid. The sizeInBytes must match with the size returned by TVIAudioFormat:bufferSize utility method.'
*** First throw call stack:
(0x225d0fea0 0x224ee1a40 0x225c2997c 0x1043d0ec4 0x1040333e0 0x104037150 0x1040378d4 0x242580fe4 0x2257496c8 0x22574a484 0x2256f1be0 0x2256f2728 0x2256faec8 0x22592c0dc 0x22592ecec)
libc++abi.dylib: terminating with uncaught exception of type NSException

Versions

Video iOS SDK

2.5.6

Xcode

10.1

iOS Version

12.1

iOS Device

iPhone 6s

ceaglest commented 5 years ago

Hi @MilanNosal,

I can give this a shot to see what is going on. So far I haven't tested capturing app (as opposed to mic) audio using ReplayKit, but it seems like there might an issue with how the sample size and format is being specified.

Best, Chris

ceaglest commented 5 years ago

Hi,

Trying this out and debugging locally, it looks like ReplayKit is delivering a very large number of microphone audio samples at a time, more so than TVIAudioDeviceContext is prepared to deal with. By spending some additional memory we could solve this problem.

    numSamples = 22596

This is pretty much a half second of audio that we are being asked to consume. This is absolutely not typical behavior of any Apple audio API which work in much smaller slices...

CMSampleBuffer 0x12a0090b0 retainCount: 8 allocator: 0x1b24655f0
    invalid = NO
    dataReady = YES
    makeDataReadyCallback = 0x0
    makeDataReadyRefcon = 0x0
    formatDescription = <CMAudioFormatDescription 0x2816fc630 [0x1b24655f0]> {
    mediaType:'soun' 
    mediaSubType:'lpcm' 
    mediaSpecific: {
        ASBD: {
            mSampleRate: 44100.000000 
            mFormatID: 'lpcm' 
            mFormatFlags: 0xe 
            mBytesPerPacket: 2 
            mFramesPerPacket: 1 
            mBytesPerFrame: 2 
            mChannelsPerFrame: 1 
            mBitsPerChannel: 16     } 
        cookie: {(null)} 
        ACL: {(null)}
        FormatList Array: {(null)} 
    } 
    extensions: {(null)}
}
    sbufToTrackReadiness = 0x0
    numSamples = 22596
    sampleTimingArray[1] = {
        {PTS = {25222430444916/1000000000 = 25222.430}, DTS = {INVALID}, duration = {1/44100 = 0.000}},
    }
    dataBuffer = 0x2816fc5a0

Other than that the TVIAudioFormat appears to be created correctly. That being said, we expected a much lower number of frames than ReplayKit is giving us. What I would try next is increasing this line: https://github.com/twilio/video-quickstart-swift/blob/2.6.0-preview/ReplayKitExample/BroadcastExtension/ExampleReplayKitAudioCapturer.m#L11

If ReplayKit insists on such large samples you would need to consume at least 22,596 frames at a time.

// Making this larger is a start.
static size_t kMaximumFramesPerBuffer = 2048;

I'm writing this from the middle of a meeting. More detailed debugging will have to wait.

Best, Chris

MilanNosal commented 5 years ago

Hi Chris! Thanks for prompt reply. I tried updating kMaximumFramesPerBuffer to 22596. Now there is no error, which is nice. However, the audio is not played - there is silence.

I am wondering if that is not the result of our setup. We have an app with video call implemented, for which there is a TVI participant. Now to be able to broadcast screen share, I am conntecting from the broadcast extension using a different identity, therefore as a different TVI participant. The first, lets call it camera participant, broadcasts camera video track + microphone audio track. This works nicely. Now the broadcast participant (the one from the broadcast extension) broadcasts the output generated by ReplayKit as video track, and I was hoping to make it work with in app audio track. However, I can imagine that the current problem might potentially be result of the given setup - there are two participants connected to the same room from the same device - one is trying to broadcast microphone audio, the other one app audio. The mic audio works, the in app does not. Could it maybe be the problem with feedback from in app (our app generates sounds from the room, the extension consumes it and broadcasts it back to the room, etc.)?

Best Milan

ceaglest commented 5 years ago

Hi @MilanNosal,

... there are two participants connected to the same room from the same device - one is trying to broadcast microphone audio, the other one app audio. The mic audio works, the in app does not ...

Unfortunately, I have not tried the setup that you describe. If you need both the app and microphone audio you probably want to tap that out at the same place (an app, or an extension). If you need full duplex audio playback and recording this might only be possible in an app.

We have a conferencing mode in the example which uses ReplayKit for video, records from the mic, and plays back remote audio. It does not however, capture app audio and is of course limited to your own app screen.

As for extensions, we have a long standing request to mix app and microphone audio. ( https://github.com/twilio/twilio-video-ios/issues/7).

I am handing this issue over to @piyushtank who will investigate further. We should be able to at least support capturing from the microphone using ReplayKit in an extension.

Regards, Chris

etown commented 5 years ago

@ceaglest Thanks for the info, regardless of the setup, I still have not seen a way to capture app audio, it would be really great to see in example working to make sure it even is compatible with twilio in the simplest case (Just from the broadcast extension and no mixing mic)

ceaglest commented 5 years ago

Hi,

Thanks for the info, regardless of the setup, I still have not seen a way to capture app audio, it would be really great to see in example working to make sure it even is compatible with twilio in the simplest case (Just from the broadcast extension and no mixing mic)

I agree, and we have cut ISDK-2334 for internal tracking on the issue. We need to get this working before moving onto the next step, and @piyushtank has got you covered on this one.

Thanks for your patience, Chris

MilanNosal commented 5 years ago

Hi Chris! Just an update from my experiments with Twilio video quickstart project and broadcast extension. In order to reduce complexity, I switched to your sample project and played around with it. I guess you guys will be able to easily find the same results as I did, but maybe it will help. First, I have updated the kMaximumFramesPerBuffer as per your advice. After that the broadcast extension worked without crash. However, the sound went through only as some static noise (testing it through youtube). Second, when I changed position of the phone from portrait to landscape, there was a crash:

2018-12-18 14:41:24.656145+0100 BroadcastExtension[4295:269013] 45192
broadcastStartedWithSetupInfo:  Optional([:])
didConnectToRoom:  <<TVIRoom: 0x282d460d0> name: milan, state: Connected, sid: RMabf7e095b98c450eb79503166be68c9b, delegate: <BroadcastExtension.SampleHandler: 0x282d42300>>
2018-12-18 14:41:41.423890+0100 BroadcastExtension[4295:269001] *** Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: 'The supplied sizeInBytes is invalid. The sizeInBytes must match with the size returned by TVIAudioFormat:bufferSize utility method.'
*** First throw call stack:
(0x225d0fea0 0x224ee1a40 0x225c2997c 0x10299c638 0x102612b90 0x10261c050 0x10261c450 0x242580fe4 0x2257496c8 0x22574a484 0x2256f1be0 0x2256f2728 0x2256faec8 0x22592c0dc 0x22592ecec)
libc++abi.dylib: terminating with uncaught exception of type NSException

It looks like the same thing from before.

screen shot 2018-12-18 at 14 41 54

Hope this helps. Best regards Milan

piyushtank commented 5 years ago

@MilanNosal Thanks for filing the issue. Wanted to let you know that I have started working on the problem and I am able to reproduce it. I will keep you posted.

Piyush

piyushtank commented 5 years ago

Hi @MilanNosal

I observed, for microphone audio, ReplayKit delivers audio samples in little endian format. And, for app audio, the audio samples are delivered in big endian. The Video SDK expects audio samples in little endian format. In future, we will add this to our SDK documentation or update our SDK to handle big endian internally in future.

For now, I posted a PR to fix this problem in our ReplayKit sample app. See [PR] for for the fix. Try it out and let me know.

Best, Piyush

MilanNosal commented 5 years ago

Hi @piyushtank Thank you for your fast reply and working out the issue. I've updated the code based on your PR, and it fixed the issue. I am glad to also mention that it works nicely even in our complicated setup. We are going to test it more, and if we won't find any problems, I believe we can close the issue. Thanks again! Best, Milan

MilanNosal commented 5 years ago

Hi @piyushtank ! I've just noticed one weird thing when using the PR example - when I use for example Youtube app, the SampleHandler works as expected - the sound is being broadcasted. However, when I use the iOS Music app (the default music player), there is no sound from it. It is as if the ExampleReplayKitAudioCapturer implementation has somehow overriden the sound. The music is not played both on the local device and it is also not broadcasted. Since this is one of the use cases we need in the app, I am reopening the issue. Best regards Milan

piyushtank commented 5 years ago

@MilanNosal I am able to reproduce the problem.

Then, Instead of using Broadcast extension, when I used "Screen Recording" provided by iOS, I ran into the same problem. The Music app audio got suppressed (music stopped playing) and the recorded file did not have any audio in it either. Since broadcast extension is using the same underlying module as Screen Recording, I thinking if this is an issue of running MusicApp + ReplayKit together in iOS. I haven't done enough research yet on Apple developer forum or open radars yet. We will do some more research around this when our iOS team is back from vacation mid of next week and get back with more details.

MilanNosal commented 5 years ago

@piyushtank thanks for the prompt reply!

That sounds bad. I will be waiting for the news.

piyushtank commented 4 years ago

@MilanNosal Is this problem still reproducible with latest iOS? Can we close this old ticket?

MilanNosal commented 4 years ago

@piyushtank Thanks for following up. TBH I forgot about the issue, and since we are not using Twilio anymore, I cannot really confirm reproducibility. Anyway, you can close it for me.