fixed IOS gui issues, now runs on device (but without sound support) #1450

jeroenvv · 2021-04-05T13:04:32Z

This is my first attempt to create a working IOS version. Made some minor changes to have to GUI run on IOS iPhone and iPad.
I have not yet worked on sound support.

- no multiple window support - storyboard is for splash screen, not main window - set client flag in main.cpp

make sure client settings and chat dialog can be closed in IOS

hoffie · 2021-04-05T15:57:08Z

Thanks for working on this!
As I understand it, there is already code for IOS, which is currently non-functional. Your PR makes the UI functional, although sound is still missing, right?

If there's already an improvement over the status quo by getting this merged, we could merge it as-is, I guess. I have no way to test IOS-related stuff though. @ann0see Are you able to test this?

jeroenvv · 2021-04-05T18:08:57Z

Thanks for working on this!
As I understand it, there is already code for IOS, which is currently non-functional. Your PR makes the UI functional, although sound is still missing, right?

Correct. The .pro file was functional for iOS, all I did was run qmake for iOS and made the project work from within XCode.

Some review would be nice - this is the first open source project I participate in, and also the first iOS app / C++ / Qt project (my background is in Java service programming). So all feedback is welcome.

ann0see · 2021-04-05T18:26:52Z

Great! Thank you for working on this. I just compiled and installed it for my iPhone SE and it crashed on the first run. After I granted microphone privileges and started it again, it seemed to load with GUI. Unfortunately, it's a bit slow, but also shows what's possible!!

I think Android could also learn from this: You implemented a close button which lacks on Android!

ann0see

Thank you very much for working on this! I really appreciate it. GUI seems to work (especially on an iPad?). See my other comments and especially look at the code style.

ios/Info.plist

src/chatdlg.cpp

src/clientsettingsdlg.cpp

src/main.cpp

ann0see · 2021-04-05T18:39:02Z

Attaching crash log:

dyld: Library not loaded: /usr/lib/libexpat.1.dylib
  Referenced from: /System/Library/Frameworks/ImageIO.framework/ImageIO
  Reason: no suitable image found.  Did find:
	/usr/lib/libexpat.1.6.7.dylib: code signature invalid for '/usr/lib/libexpat.1.6.7.dylib'

	/usr/lib/libexpat.1.6.7.dylib: code signature invalid for '/usr/lib/libexpat.1.6.7.dylib'

dyld: launch, loading dependent libraries
DYLD_LIBRARY_PATH=/usr/lib/system/introspection
DYLD_IMAGE_SUFFIX=_debug
DYLD_INSERT_LIBRARIES=/Developer/usr/lib/libBacktraceRecording.dylib:/Developer/Library/PrivateFrameworks/DTDDISupport.framework/libViewDebuggerSupport.dylib
(lldb)

->  0x102f71428 <+8>:  b.lo   0x102f71444               ; <+36>

Probably something with code signing. It only occurs on a start through Xcode. I don't think you'll need to worry about that.

ngocdh · 2021-04-05T19:28:12Z

Such an exciting news! Thank you @ann0see and @jeroenvv for working on this.
The "close" button is indeed a very good idea. On my android, I have to use the "New UI" version (discussion #1343) to avoid having to deal with windows.

jeroenvv

I think Android could also learn from this: You implemented a close button which lacks on Android!

When things started working, I immediately got into trouble when I opened the settings window. I could not get back other than quitting the app. For the chat window there was an escape because I could somehow click 'next' to the window and get back to the main window.
However, I dislike the menus in iOS. That is not really the way a GUI should work on a mobile device... but that's quite another issue.

ios/Info.plist

ann0see · 2021-04-06T07:47:49Z

I dislike the menus in iOS. That is not really the way a GUI should work on a mobile device... but that's quite another issue.

Yep. The GUI works but it clearly shows that it's not an iOS App

ann0see

Not approving it yet since we might be able to remove code.

src/main.cpp

ngocdh · 2021-04-06T19:36:32Z

@ann0see @jeroenvv although coding is not my thing, I'd love to try and do a few things here. If it's not complicated, could you show me how to build the iOS version?
What I've done so far:

install qt5
download @jeroenvv branch
Generate the .xcodeproj file: qmake -spec macx-ios-clang Jamulus.pro

Then I couldn't build the project, either using xcodebuild build or in Xcode. Could you please help?

jeroenvv · 2021-04-06T19:42:25Z

@ann0see @jeroenvv although coding is not my thing, I'd love to try and do a few things here. If it's not complicated, could you show me how to build the iOS version?
What I've done so far:

install qt5

download @jeroenvv branch

Generate the .xcodeproj file: qmake -spec macx-ios-clang Jamulus.pro

Then I couldn't build the project, either using xcodebuild build or in Xcode. Could you please help?

Which qmake did you run? What I found out was that I needed to install qt5 with the qt installer (not homebrew), and explicitly select iOS when choosing the Qt version.
After installation, I ran /5.12.10/ios/bin/qmake -spec macx-xcode Jamulus.pro
Only with the right qmake, I got all the necessary libraries correctly configured. Then, in XCode, check the Signing & Capabilities and configure a team if you want to run on a device.
After that, I could run from XCode.

ngocdh · 2021-04-06T19:45:48Z

Which qmake did you run? What I found out was that I needed to install qt5 with the qt installer (not homebrew), and explicitly select iOS when choosing the Qt version.
After installation, I ran /5.12.10/ios/bin/qmake -spec macx-xcode Jamulus.pro
Only with the right qmake, I got all the necessary libraries correctly configured. Then, in XCode, check the Signing & Capabilities and configure a team if you want to run on a device.
After that, I could run from XCode.

Great! Thanks a lot for the quick response. Indeed I run with homebrew's qmake. I'm reinstalling :)

src/clientsettingsdlg.cpp

ann0see · 2021-04-07T15:52:02Z

src/main.cpp

@@ -632,7 +632,6 @@ int main ( int argc, char** argv )
    QCoreApplication* pApp = new QCoreApplication ( argc, argv );
 #else
 # if defined ( Q_OS_IOS )
-    bIsClient = false;
    bUseGUI = true;


Is this needed?

no, and in the last version of the PR, this line has been removed.

I can still see bUseGui = true in the diff?

I can image bUseGui is forced to true, independent of command line settings. Apps on IOS must have a GUI.

ngocdh · 2021-04-08T17:33:27Z

Team, I've been trying to understand and cook something up for iOS audio.

From what I understand from android sound.cpp code, we have to write 3 functions init, start, stop, and callbacks.

So I grabbed a sample RemoteIO project from Apple, and started modifying things.

In Apple's sample project, I could indeed find codes to init, start and stop the IOUnit. However, I'm having trouble with compiling/linking. (The sample project alone works fine, just the part I took and put into Jamulus is trouble).

For information, I don't have any experience with developing audio, or even developing. This is my first time using Qt, Xcode, etc. I even discovered some of Github's amazing features thanks to Jamulus (lol). So I'm posting here some information so that anyone interested in getting iOS audio working can continue, or point me into the right direction, or maybe help, or discuss.

Any contribution is welcome.

ann0see · 2021-04-08T19:55:07Z

Same for me unfortunately: Although I've been programming in Objective-C once, I've never done much with it so I doubt to be much help. I think we can merge this Pull Request and do the audio work in another one.

ann0see

Not yet 100% ready to be merged since there's still a code style issue (too many spaces in line 40 clientsettingsdlg.cpp). Otherwise I think all that is ok.

ngocdh · 2021-04-08T20:26:00Z

I made progress haha (it could be nothing though, it’s too early). I managed to build with a part of the code taken from Apple. And right when the app loads, my background music stops. And when I try to connect, there’s clearly an orange mic from iOS showing Jamulus is accessing the mic. I still have the messge “your sound card is not working...”. Also have to figure out the callback part :)

…

On 8 Apr 2021, at 21:55, ann0see ***@***.***> wrote: Same for me unfortunately: Although I've been programming in Objective-C once, I've never done much with it so I doubt to be much help. I think we can merge this Pull Request and do the audio work in another one. — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

spacing issue Co-authored-by: ann0see <20726856+ann0see@users.noreply.github.com>

jeroenvv · 2021-04-10T10:51:12Z

I made progress haha (it could be nothing though, it’s too early). I managed to build with a part of the code taken from Apple. And right when the app loads, my background music stops. And when I try to connect, there’s clearly an orange mic from iOS showing Jamulus is accessing the mic. I still have the messge “your sound card is not working...”. Also have to figure out the callback part :)

Maybe we can work together on this? Can you share your code?

ann0see

Didn't test the last commit, but since it's just spacing, I'll approve this

ann0see · 2021-04-10T11:04:09Z

Thank you very much for working on this!

Next steps:

Since you're a new contributor, you'll need to be added as contributor in the code
Should we already put a note like GUI Only support for iOS into the changelog?

ngocdh · 2021-04-10T11:20:00Z

Maybe we can work together on this? Can you share your code?

That’d be great, I’ll share my code as soon as I get home!

I actually have sound now, but only one way. People can hear me clearly. But I can’t hear people, apart from myself (I think this is default behavior in iOS when dealing with audio data from the mic). I’m having trouble to set the callback to play sound.

Feedback about audio quality: buffer size 128 works fine. 64 is not good. Total delay is high and unstable. But ping is already unstable, which is strange because I thought iOS should be good at this.

Anyway, apologies because my code is very bad, lots of bad practices and uncommented codes. You’ll see very soon.

ngocdh · 2021-04-10T15:10:25Z

@jeroenvv please excuse my lack of dev experience. I have never developed anything serious. Never used github. So here's my rookie way of sharing code:
https://drive.google.com/file/d/1ROSg0Xa6rvrFOVOwD3VPoqAfLBUnMTFJ/view?usp=sharing

It's a zip archive of my jamulus project, because I don't really know how to do the right way (make modifications in a fork of the master branch? -EDIT: @jeroenvv branch).

Some notes: again, the coding is really bad. I took pieces from jamulus' Mac source code and a sample project from Apple.

I removed codes not suitable for iOS in the first and added codes for iOS from the second, then added a few to make them work together.

At first I had some linking issues when compiling, which I solved by setting "Compile sources as" "Objective C++", but I also had to add 4 type conversions in the opus library.

Now the code handling Mic input (which works) is in the performRender function. I tried to do the same thing to the function playbackCallback to make it route audio data from server to speakers, but haven't succeeded.

Don't hesitate to ask questions. Because I'm sure there are pieces of code that make no sense or do nothing, they're just there because I was testing different things.

Also, most of the time I have no idea what I'm doing, so please bear with me.

ann0see · 2021-04-10T16:12:02Z

Anyway, apologies because my code is very bad, lots of bad practices and uncommented codes. You’ll see very soon.

No problem. The Jamulus contributors/devs will probably be able to help out. I'd advise you to get familiar with git/GitHub which is not too difficult. Once you know what a repository/branch/commit/fork/push is.

I might not be able to look into the code yet since there's a lot to do. Others might help out?

ann0see · 2021-04-10T18:30:46Z

Just requested a review from softins I think it's ok, a developer with a bit more C++/Qt experience should have a look at this before we can merge it.

ngocdh · 2021-04-11T20:29:18Z

I need help with some basic buffer operations:

In client.cpp, after the input audio is sent to the server, audio data from server is also fetched into the same buffer vecsTmpAudioSndCrdStereo. It seems we just have to fill data in this buffer in the playback callback function (ref: https://atastypixel.com/blog/using-remoteio-audio-unit/).
However I've been having trouble copying this buffer because of access exception. In the code below, I have access issue at line pRightData[i] = ..., where pLeftData and pRightData are both NULL in the debug window, but the printf show they were indeed allocated:

    Float32* pLeftData             = (Float32*) ( bufferList->mBuffers[0].mData );
    Float32* pRightData            = (Float32*) ( bufferList->mBuffers[1].mData );

    // copy output data
    for ( int i = 0; i < inNumberFrames; i++ )
    {
        printf ("Allocated left data at %lx ",pLeftData);
        printf ("Allocated right data at %lx ",pRightData);

        // copy left and right channels separately
        pLeftData[i]    = (Float32) pSound->vecsTmpAudioSndCrdStereo[2 * i] / _MAXSHORT;
        pRightData[i] = (Float32) pSound->vecsTmpAudioSndCrdStereo[2 * i + 1] / _MAXSHORT;
    }

/cc: @jeroenvv @ann0see

pljones · 2021-04-11T21:14:35Z

It seems to me that loop from 0 to inNumberFrames risks running over the length of bufferList->mBuffers[1].mData -- but it would be easier if you pasted the link to lines of code you were referring to, as well.

ngocdh · 2021-04-11T21:20:13Z

It seems to me that loop from 0 to inNumberFrames risks running over the length of bufferList->mBuffers[1].mData -- but it would be easier if you pasted the link to lines of code you were referring to, as well.

Indeed I have to add a check for actual number of frames. But in this case my concern is more about pLeftData and pRightData access. Allocated, but one line of code later, back to NULL?

softins · 2021-04-12T11:15:49Z

Just requested a review from softins I think it's ok, a developer with a bit more C++/Qt experience should have a look at this before we can merge it.

Happy to look at it at some point, but my time is very limited right now. However, I'm wondering if merging this should be delayed until after we have released the next update anyway.

pljones · 2021-04-12T18:34:06Z

It seems to me that loop from 0 to inNumberFrames risks running over the length of bufferList->mBuffers[1].mData -- but it would be easier if you pasted the link to lines of code you were referring to, as well.

Indeed I have to add a check for actual number of frames. But in this case my concern is more about pLeftData and pRightData access. Allocated, but one line of code later, back to NULL?

Is that what's happening? What I'm saying is that if the index runs off the end, then the address you'll get back will be zero, quite possibly, hence appear to be NULL.

ngocdh · 2021-04-12T18:36:04Z

What I'm saying is that if the index runs off the end, then the address you'll get back will be zero, quite possibly, hence appear to be NULL.
I think you're right. Thanks a lot!

ngocdh · 2021-04-13T01:44:00Z

UPDATE: I finally got sound both ways. I'll clean up the code and try to do a proper pull request. That's a lot of firsts for me so please bear with me :)

ngocdh · 2021-04-13T13:12:13Z

Team I've uploaded my repo here https://github.com/ngocdh/jamulus-ios (clean source code, working sound for iOS) but don't know how to create a pull request. Can you help?

FYI, I'm using source control in Xcode. I first created a clone from https://github.com/jeroenvv/jamulus/tree/ios_gui, but then Xcode seems to treat https://github.com/jeroenvv/jamulus as the master branch, to which I can't create pull request.

jeroenvv · 2021-04-13T16:18:13Z

@ngocdh how did you create your repo? I think you did not start it out as a fork of jamulussoftware/jamulus. When you start your repo as a fork, you can then commit changes and submit a pull request to the original repository.
So I would suggest: create a fork of jamulussoftware/jamulus so you get your own copy of the repo, then apply all changes, commit + push, and then create a pull request.

ngocdh · 2021-04-13T17:49:36Z

Thanks a lot @jeroenvv all good now. I just sent my first pull request.

softins

Happy to approve and merge these changes, as they are simple and are needed for further iOS work.

jeroenvv · 2021-04-22T17:10:23Z

Thanks people!

jeroenvv added 2 commits April 5, 2021 15:00

fixed IOS gui issues

5a3bddc

- no multiple window support - storyboard is for splash screen, not main window - set client flag in main.cpp

add close menu item for secondary windows

9cb1dca

make sure client settings and chat dialog can be closed in IOS

ann0see self-requested a review April 5, 2021 18:00

ann0see self-assigned this Apr 5, 2021

ann0see reviewed Apr 5, 2021

View reviewed changes

ios/Info.plist Show resolved Hide resolved

src/chatdlg.cpp Outdated Show resolved Hide resolved

src/chatdlg.cpp Outdated Show resolved Hide resolved

src/clientsettingsdlg.cpp Outdated Show resolved Hide resolved

src/main.cpp Outdated Show resolved Hide resolved

linting fixes

1a98771

jeroenvv commented Apr 5, 2021

View reviewed changes

ios/Info.plist Show resolved Hide resolved

ann0see reviewed Apr 6, 2021

View reviewed changes

ann0see requested changes Apr 6, 2021

View reviewed changes

src/main.cpp Outdated Show resolved Hide resolved

removed obsolete client flag

1a0181e

ann0see reviewed Apr 7, 2021

View reviewed changes

ann0see requested changes Apr 8, 2021

View reviewed changes

Update src/clientsettingsdlg.cpp

0011d84

spacing issue Co-authored-by: ann0see <20726856+ann0see@users.noreply.github.com>

ann0see approved these changes Apr 10, 2021

View reviewed changes

ann0see requested review from ann0see and a team April 10, 2021 11:08

ann0see approved these changes Apr 10, 2021

View reviewed changes

ann0see requested a review from softins April 10, 2021 18:29

ngocdh mentioned this pull request Apr 13, 2021

iOS sound support #1512

Closed

softins approved these changes Apr 22, 2021

View reviewed changes

softins merged commit d3c5a9d into jamulussoftware:master Apr 22, 2021

jeroenvv deleted the ios_gui branch April 22, 2021 17:10

pljones added this to the Release 3.8.0 milestone Feb 19, 2022

ngocdh mentioned this pull request May 12, 2023

Build: iOS: Use Qt6 and bump build environment #3065

Closed

5 tasks

fixed IOS gui issues, now runs on device (but without sound support) #1450

fixed IOS gui issues, now runs on device (but without sound support) #1450

Conversation

jeroenvv commented Apr 5, 2021

hoffie commented Apr 5, 2021

jeroenvv commented Apr 5, 2021

ann0see commented Apr 5, 2021 • edited Loading

ann0see left a comment

Choose a reason for hiding this comment

ann0see commented Apr 5, 2021

ngocdh commented Apr 5, 2021

jeroenvv left a comment • edited Loading

Choose a reason for hiding this comment

ann0see commented Apr 6, 2021

ann0see left a comment • edited Loading

Choose a reason for hiding this comment

ngocdh commented Apr 6, 2021

jeroenvv commented Apr 6, 2021

ngocdh commented Apr 6, 2021

ann0see Apr 7, 2021

Choose a reason for hiding this comment

jeroenvv Apr 7, 2021

Choose a reason for hiding this comment

ann0see Apr 7, 2021

Choose a reason for hiding this comment

jeroenvv Apr 10, 2021

Choose a reason for hiding this comment

ngocdh commented Apr 8, 2021

ann0see commented Apr 8, 2021

ann0see left a comment

Choose a reason for hiding this comment

ngocdh commented Apr 8, 2021 via email

jeroenvv commented Apr 10, 2021

ann0see left a comment

Choose a reason for hiding this comment

ann0see commented Apr 10, 2021 • edited Loading

ngocdh commented Apr 10, 2021

ngocdh commented Apr 10, 2021 • edited Loading

ann0see commented Apr 10, 2021

ann0see commented Apr 10, 2021 • edited Loading

ngocdh commented Apr 11, 2021

pljones commented Apr 11, 2021

ngocdh commented Apr 11, 2021

softins commented Apr 12, 2021

pljones commented Apr 12, 2021

ngocdh commented Apr 12, 2021

ngocdh commented Apr 13, 2021

ngocdh commented Apr 13, 2021

jeroenvv commented Apr 13, 2021

ngocdh commented Apr 13, 2021

softins left a comment

Choose a reason for hiding this comment

jeroenvv commented Apr 22, 2021

ann0see commented Apr 5, 2021 •

edited

Loading

jeroenvv left a comment •

edited

Loading

ann0see left a comment •

edited

Loading

ann0see commented Apr 10, 2021 •

edited

Loading

ngocdh commented Apr 10, 2021 •

edited

Loading

ann0see commented Apr 10, 2021 •

edited

Loading