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

"Add call" (three-way call) results in five files #284

Closed
blackwind opened this issue Apr 11, 2023 · 10 comments
Closed

"Add call" (three-way call) results in five files #284

blackwind opened this issue Apr 11, 2023 · 10 comments
Assignees
Labels
bug Bug in BCR itself enhancement New feature or request

Comments

@blackwind
Copy link

I'm throwing the kitchen sink at BCR to see how well it holds up under various scenarios, and the only problem I've found so far is with three-way calls.

In Google Dialer, place a call, press the "Add call" button, choose a contact, and Dialer places the original call on hold and dials the new contact. "Add call" is then replaced with "Merge", which can be pressed to merge the two active calls. Although BCR does function in this scenario, recording the complete call (albeit over several files), it outputs one file for the first call before you press "Add call" that gets correctly named with contact name and number, a second file for the second call before you press "Merge" that also gets correctly named with contact name and number, a third file with the merged portion of the call with no contact name or number, a fourth file with the same merged portion named with contact name and number for the first call, and a fifth file with the same merged portion named with contact number for the second call but NOT contact name.

Sample output filenames:

  1. 20230410_193856.312-0600_out_18884722222_Shaw Cable.m4a
  2. 20230410_193914.011-0600_out_18885547827_Shaw Direct.m4a
  3. 20230410_193926.969-0600_out.m4a
  4. 20230410_193927.091-0600_out_+18884722222_Shaw Cable.m4a
  5. 20230410_193927.122-0600_out_+18885547827.m4a

In a perfect world, a merged call would output as a single file. If you take on this task, be sure not to break call waiting recordings, which I suspect have a similar flow behind the scenes and should write two separate files unless merged.

chenxiaolong added a commit that referenced this issue Apr 11, 2023
* RecorderInCallService and RecorderThread have been updated to be aware
  of parent/child relationships of calls. Previously, child calls were
  treated as independent calls, so a conference call would result in
  separate (but identical in audio) recordings for the parent conference
  call and every call it was merged from.
* RecorderInCallService and RecorderThread now mute the recording when
  the corresponding call is in the HOLDING state. This prevents audio
  from a different call from being recorded while the call is on hold
  (due to call waiting).

Fixes: #284

Signed-off-by: Andrew Gunnerson <accounts+github@chiller3.com>
@chenxiaolong
Copy link
Owner

Thanks for the detailed write-up! I gave this a try on my device following your exact steps and was able to reproduce this.

Can you give the test build at #285 a try? I fixed a few things:

  • The merged recording (file 3) should now have the correct filename (eg. 20230411_032353.554-0400_out_<phone1>,<phone2>_<contact1>,<contact2>).
  • The separate recordings after merge (files 4 and 5) should no longer be created.
  • The recordings before merging (files 1 and 2) should no longer contain audio for the wrong call. The recording for the call that is on hold is now muted.

There are some limitations I found while testing:

  • Android always reports conference calls as outgoing (out in filename), even if an incoming call was converted into a conference call by the other party. Maybe I should use conference in the filename instead.
  • Android does not report phone numbers, caller IDs, or contact names if the call was converted into a conference call by the other party.
  • Occasionally, Android does not report the contact name for one of the two calls after merging. This is likely an Android bug.

In a perfect world, a merged call would output as a single file. If you take on this task, be sure not to break call waiting recordings, which I suspect have a similar flow behind the scenes and should write two separate files unless merged.

With the way Android's telephony framework works right now, I believe the only way to do this is to decode all the separate recordings, join them, and reencode to a new file. Behind the scenes, Android disconnects the two original calls and creates a new conference call when the merge button is hit. As far as I can tell, there's no metadata in the conference call that links to the calls prior to merging. It would unfortunately be a guessing game to find the previous recordings that should be joined and I'd prefer not to implement that in BCR.

@chenxiaolong chenxiaolong self-assigned this Apr 11, 2023
@chenxiaolong chenxiaolong added bug Bug in BCR itself enhancement New feature or request labels Apr 11, 2023
@blackwind
Copy link
Author

And thank you, equally, for the deep-dive and swift code changes! I can confirm all fixes are working as advertised on this end, though I'd suggest pausing recording rather than muting it in the HOLDING state if that's sufficiently simple. In the call waiting scenario in particular, if I get another call and talk to the other person for, say, five minutes, there's no good (non-technical) reason to have five minutes of dead air on the original recording.

Android always reports conference calls as outgoing (out in filename), even if an incoming call was converted into a conference call by the other party. Maybe I should use conference in the filename instead.

I vote for preserving the original call direction. If I get a call from someone who then calls someone else, that still qualifies as the same incoming call from my perspective even if the recordings are split up for technical reasons.

Android does not report phone numbers, caller IDs, or contact names if the call was converted into a conference call by the other party.

Unfortunate, but not surprising.

Occasionally, Android does not report the contact name for one of the two calls after merging. This is likely an Android bug.

Is it not possible to manually query based on the phone number?

As far as I can tell, there's no metadata in the conference call that links to the calls prior to merging. It would unfortunately be a guessing game to find the previous recordings that should be joined and I'd prefer not to implement that in BCR.

But now that you're tracking parents and children, iterating through them should be sufficient, no? You know (or can easily document) what you're recording, where you're recording it, and the order in which they should be joined. The stumbling point would be the decoding and re-encoding portion, which could be worked around by recording to PCM initially and encoding only after all portions of the call are disconnected. A bit of added complexity, no doubt, but ultimately the outcome users would expect.

@chenxiaolong
Copy link
Owner

chenxiaolong commented Apr 11, 2023

I'd suggest pausing recording rather than muting it in the HOLDING state if that's sufficiently simple. In the call waiting scenario in particular, if I get another call and talk to the other person for, say, five minutes, there's no good (non-technical) reason to have five minutes of dead air on the original recording.

Good point! I'll make that change.

But now that you're tracking parents and children, iterating through them should be sufficient, no? You know (or can easily document) what you're recording, where you're recording it, and the order in which they should be joined. The stumbling point would be the decoding and re-encoding portion, which could be worked around by recording to PCM initially and encoding only after all portions of the call are disconnected. A bit of added complexity, no doubt, but ultimately the outcome users would expect.

Unfortunately not. Android doesn't convert the two original calls into children of the new parent conference call. It first disconnects both original calls and then creates the parent with two new children. The new children have completely different metadata (eg. different ID and call creation/connection timestamps).

At the point the user presses merge, I can't think of a good way determine that the call hasn't really ended yet from the user's perspective. The disconnection of the original two calls when merging looks exactly the same as hanging up when using call waiting. (I don't think this is an Android quirk. My understanding is that with VoNR/VoLTE/wifi calling, these disconnects + reconnects are actually what happens on the network.)

I vote for preserving the original call direction. If I get a call from someone who then calls someone else, that still qualifies as the same incoming call from my perspective even if the recordings are split up for technical reasons.

I did some digging into the Android telephony framework and it seems that the call direction for a conference call is (intentionally) nonsensical until enough participants drop out that it goes back to being a one-on-one call. I get a mix of in and out when doing the same test call scenario repeatedly. For now, I've updated the filename to use conference so that it's at least not displaying a random call direction.

Is it not possible to manually query based on the phone number?

Another good idea. I've added this now. It turned out to be less complicated than I originally thought. And now that we have this, Android <11 users can see the contact name too.

I looked at the AOSP dialer implementation and it looks like it also does manual queries itself. I guess lack of testing via the dialer might be why the "official" method of getting the contact name from the call is a bit buggy.


EDIT: I've attached a new test build to #285.

@blackwind
Copy link
Author

At the point the user presses merge, I can't think of a good way determine that the call hasn't really ended yet from the user's perspective. The disconnection of the original two calls when merging looks exactly the same as hanging up when using call waiting. (I don't think this is an Android quirk. My understanding is that with VoNR/VoLTE/wifi calling, these disconnects + reconnects are actually what happens on the network.)

My limited understanding is the same. You remind me of another important scenario I hadn't yet considered, though -- what about network to roaming handoff and network to VoWiFi handoff? Both should produce the same issue -- a technically new call that should be part of the original recording. I feel like this is something that, while tough to pin down, deserves a proper resolution. I'll take a look at the logs later and see if I can come up with anything remotely feasible. First thought: Is there a way to differentiate between calls that ring and don't ring?

I looked at the AOSP dialer implementation and it looks like it also does manual queries itself. I guess lack of testing via the dialer might be why the "official" method of getting the contact name from the call is a bit buggy.

The good citizen in me wants to suggest you file issues with Google for all these quirks, but the pragmatist knows they'll all still be around in Android 37.

Heading out now for the next couple days, but I've installed the new build and will make time to test as soon as I can.

@chenxiaolong
Copy link
Owner

what about network to roaming handoff and network to VoWiFi handoff?

I gave this a quick try and it looks like Android keeps the same call going for the VoNR/VoLTE <-> VoWiFi handoff. The only visible change in the call I see is the addition and removal of the PROPERTY_WIFI flag.

Is there a way to differentiate between calls that ring and don't ring?

Sometimes... From prior bug reports, the call states are quite unreliable on many devices. There is a RINGING state that a ringing call is supposed to go through, but I've seen it skipped for no discernible reason. Same thing with the DIALING and CONNECTING states. (BCR uses ACTIVE to start recording since it's the only state that seems to work on every device. Even DISCONNECTING/DISCONNECTED is broken on some devices and BCR tries to work around that.)

@blackwind
Copy link
Author

So, I can confirm the pause fix works, and the manual query seems to work as well based on a couple tests (though you did note this can be intermittent, so I don't have 100% certainty). One weird issue: The conference portion is now being saved with a .mp3 file extension. A quick look at the file header confirms it's the same M4A format as the others, it's just getting a weird extension now for some reason.

I gave this a quick try and it looks like Android keeps the same call going for the VoNR/VoLTE <-> VoWiFi handoff.

Wow, definitely did not see that one coming. Do you have the means to test network to roaming?

There is a RINGING state that a ringing call is supposed to go through, but I've seen it skipped for no discernible reason.

Puzzlingly, there doesn't seem to be RINGING states on any of my calls since I started using the debug build, and this is on a Pixel 7 Pro where you'd most expect the rules to be followed. Possibly a network thing, not an OS thing? Sounds like a real clusterfuck in any case. I already don't envy the developers of call recording apps.

@chenxiaolong
Copy link
Owner

Awesome, thanks for testing!

the manual query seems to work as well based on a couple tests (though you did note this can be intermittent, so I don't have 100% certainty).

If you still happen to have the log file, it should have some messages like this near the very bottom if a manual query was required:

04-11 15:43:43.128 27086 27305 W RecorderThread/241: Contact display name missing in conference child call
04-11 15:43:43.128 27086 27305 D RecorderThread/241: Performing manual contact lookup
04-11 15:43:43.133 27086 27305 D RecorderThread/241: Found contact display name via manual lookup
04-11 15:43:43.133 27086 27305 I RecorderThread/241: Updated filename due to call details change: 20230411_154336.379-0400_conference_<conference phone numbers>_<conference contact names>

One weird issue: The conference portion is now being saved with a .mp3 file extension. A quick look at the file header confirms it's the same M4A format as the others, it's just getting a weird extension now for some reason.

Wow, I'm baffled what might cause that. In the log file, if you see:

Creating <filename> with MIME type audio/mp4 in <output directory>

then I think you might've found a bug in the Android storage access framework. It's responsible for adding the file extension based on the file type.

Do you have the means to test network to roaming?

Fortunately unfortunately not. My carrier has really good coverage in my area. Briefly looking through what AOSP's dialer does, I think it's unlikely that the call gets recreated when switching to/from roaming.

Possibly a network thing, not an OS thing?

It's very possible it's a network thing. Or possibly the modem firmware--I think at the lowest level, that's where all the call states come from, even for internet-based calls (VoNR/VoLTE/VoWiFi).

Sounds like a real clusterfuck in any case. I already don't envy the developers of call recording apps.

100% agreed there, the whole telephony stack is a giant clusterfuck. It's very slowly getting better though. A few years ago with my old Samsung Galaxy S8+, it was possible for the modem to get stuck in a bad state that could not be resolved by rebooting or reflashing the firmware. The modem didn't actually reset itself until the battery was completely drained.

@blackwind
Copy link
Author

If you still happen to have the log file, it should have some messages like this near the very bottom if a manual query was required:

Yep, found it.

Wow, I'm baffled what might cause that. In the log file, if you see:

04-11 18:34:20.462  5628  8743 D OutputDirUtils: Creating 20230411_183403.682-0600_conference_<conference phone numbers>_<conference contact names>.mp3 with MIME type audio/mpeg in content://com.android.externalstorage.documents/tree/primary%3AAudioRecordings%2FBasicCallRecorder/document/primary%3AAudioRecordings%2FBasicCallRecorder

It occurs to me now that I ran into the same problem the other night when I briefly messed with filename formats. To reproduce, simply create a bcr.properties that contains only "filename.1.suffix = Test". Gives me a .mp3 every time.

My carrier has really good coverage in my area.

Good/bad news: Mine doesn't. I'll see if I can make it happen on my way home.

100% agreed there, the whole telephony stack is a giant clusterfuck. It's very slowly getting better though. A few years ago with my old Samsung Galaxy S8+, it was possible for the modem to get stuck in a bad state that could not be resolved by rebooting or reflashing the firmware. The modem didn't actually reset itself until the battery was completely drained.

Wild, and you still started this project? Brave man. 🤣

@blackwind
Copy link
Author

Just spent about an hour testing. I was able to get the call to drop many times, but it didn't once hand off to the roaming network. At any rate, given your hypothesis, I'm ready to concede that the current implementation is good enough.

@chenxiaolong
Copy link
Owner

04-11 18:34:20.462  5628  8743 D OutputDirUtils: Creating 20230411_183403.682-0600_conference_<conference phone numbers>_<conference contact names>.mp3 with MIME type audio/mpeg in content://com.android.externalstorage.documents/tree/primary%3AAudioRecordings%2FBasicCallRecorder/document/primary%3AAudioRecordings%2FBasicCallRecorder

It occurs to me now that I ran into the same problem the other night when I briefly messed with filename formats. To reproduce, simply create a bcr.properties that contains only "filename.1.suffix = Test". Gives me a .mp3 every time.

Thanks for the reliable reproducer. I've opened #292 to track this bug.

The fact that filename.1.suffix = Test alone works is a bug too... It's not supposed to merge with the default config. Oh well, that'll get fixed by #288.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug in BCR itself enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants