opus icon indicating copy to clipboard operation
opus copied to clipboard

Peculiar issue calling lib from iPhone

Open AndrueCope opened this issue 4 years ago • 7 comments

We are using opus successfully from Windows and from Android (encode and decode - the Windows piece has been in use for over a year). However when called on iPhones we're getting strange results.

The calling code is C# and the code actually making the calls is shared by all platforms so we expected it to 'just work'. However what we're getting has us baffled.

  • When presented with 320 int16s on both Windows and Android platforms opus_encode() writes 28 bytes to the output buffer and they look the same. We know that correct data is being generated.
  • When called on the iPhone with the same input array opus_encode() writes 651 bytes to the output buffer. This consists of roughly 100 non-zero bytes, about 200 zero bytes then another 300 non-zero bytes. We have confirmed that by reducing the size of the target buffer and input buffer we can modify the behaviour (eg; if we say the output buffer is only 200 bytes then opus_encode() only writes 200 bytes). The bytes it writes are consistent each time. We've modified the opus_encode() source to display the input parameters and that shows that they are making the transition from C# to C correctly (we sprintf() to the output buffer and examine the output buffer back on the C# side).
  • For what it's worth both Windows and iPhone are calling the 'non FIXED_POINT' version of the method.
  • We have confirmed that other calls to the library (eg; getting the version) return valid results on iPhone.

Hence our bafflement. I've been programming for several decades and I have experience with calling external code. Usually if you get something wrong the code blows up. Or at least behaves very randomly. Yet our testing indicates that opus is being called correctly because it's stable. It's consistent. It's just the returned data is wrong. Very wrong.

Any thoughts?

Thank you.

AndrueCope avatar Dec 21 '20 18:12 AndrueCope

In case you are using cmake to build the lib it's not really well tested on iOS so there might be bugs there.

In case you have your custom build I would probably take a step back and test opus by itself without any wrapper layers and ensure your build is working correctly (build flags etc).

xnorpx avatar Dec 22 '20 03:12 xnorpx

Yeah we're going to try and create a console app for iOS and see what that shows. Thanks.

AndrueCope avatar Dec 22 '20 08:12 AndrueCope

I think I know the problem (will confirm tomorrow after a good night's sleep). What we're seeing is that the bitrate_bps value is wrong when run on the iPhone. It should be 16,000 but is something mad like 270,000 on the iPhone. I think the reason is our use of opus_encoder_ctl() to specify the bandwidth during initialisation.

I think that the problem lies in the export declaration of this function:

OPUS_EXPORT int opus_encoder_ctl(OpusEncoder *st, int request, ...) OPUS_ARG_NONNULL(1);

It's a variadac function and last I heard exporting these from a library was asking for trouble because different compilers/platforms encode them differently. For example there's this bodgy code someone else had to implement for another library:

https://github.com/xamarin/xamarin-macios/blob/fc55e4306f79491fd269ca2495c6a859799cb1c6/src/UIKit/UIAppearance.cs#L112

I will confirm this (or refute it, lol) tomorrow by exporting a new function just for setting the bandwidth. If this does prove to be the solution I think we need an 'official' alternative to opus_encoder_ctl(). I'm currently thinking a union to replace the '...' but it's been many years since I used C/C++ so there might be a more conventional way of doing this. Any thoughts?

AndrueCope avatar Dec 22 '20 23:12 AndrueCope

It's confirmed. When we stopped trying to specify the output bitrate we got the results we wanted. We're mulling over what we're going to do next. I think we can just stop trying to override the bitrate. But I think someone ought to consider providing a non-variadac version of the opus_set_ctrl() function for this library.

I've suggested it to my boss but we may just go with not specifying the output bit rate.

AndrueCope avatar Dec 23 '20 10:12 AndrueCope

Glad you figured out the issue. That is tricky!

I always considered vararg functions a necessary part of the C abi because of printf. Are you using a different compiler for the opus library and the rest of your app? Is this a toolchain bug you could report to Apple?

rillian avatar Dec 23 '20 19:12 rillian

I think they mention it here, https://developer.apple.com/documentation/xcode/writing_arm64_code_for_apple_platforms

xnorpx avatar Dec 23 '20 21:12 xnorpx

Looking at the callouts on a few different pages, the one that stands out most to me is that args are never promoted for varargs. What if you call opus_encoder_ctl with (opus_int32)16000 ? There's an off chance it's sending int16 while Opus expects int32, since 16000 can fit in int16. Kind of a guess here since I don't have an Xcode environment set up.

If nothing else, at least Opus is pretty stable now and a bunch of constant shims shouldn't be a huge maintenance burden.

-Em

silverbacknet avatar Dec 24 '20 01:12 silverbacknet