welle.io icon indicating copy to clipboard operation
welle.io copied to clipboard

fixed bug where /mp3 request would be caught in while loop

Open comcloudway opened this issue 2 years ago • 10 comments

this is because my old code would also introduce the while to the /mp3 request and if the user forgot to tune before, the request would now wait instead of failing. Caused by: #740

comcloudway avatar Jun 16 '22 08:06 comcloudway

NOTE: I haven't yet had time to test it (with an antenna connected), but given that I only moved a couple lines of code around nothing should have changed. I requested /mp3 and it told me that my request was invalid (which it was supposed to be) and /mp3-channel was unable to complete (as I obviously didn't connect an antenna) Also the code does compile (but AppVeyor still appears to be broken)

comcloudway avatar Jun 16 '22 08:06 comcloudway

Please test it with real signals. You're locking rx_mut twice now, not sure that's ok.

mpbraendli avatar Jun 17 '22 15:06 mpbraendli

Oh yeah, totally didn't think of that whilst I do think that it should be working (when I was working on version1, I locked it at the beginning and in the middle of the function, but then decided that it was redundant), I'm not 100% positive it will work - thanks for pointing it out.

I'll try get it tested till Tuesday

comcloudway avatar Jun 18 '22 07:06 comcloudway

You were right, it turns out locking the mutex twice results in a dead lock (which I should have expected, as I ran into this problem in rust already) I fixed it by unlocking the mutex and got audio playback working.

comcloudway avatar Jun 21 '22 12:06 comcloudway

Something I realized is that the playback is not fluent (it keeps halting to buffer), I believe that this might be a different problem as I also experience this problem when using the normal mp3 function.

NOTE: I do have reception, welle-gui has no problems playing back the audio stream.

Additionally I noticed that the welle-cli crashed after I closed the browser tab, I used for testing, while it was still trying to fill the buffer. I suspect this might be a problem relating to him not closing the audio device properly - and I'm unsure if it is caused by my code

comcloudway avatar Jun 21 '22 12:06 comcloudway

I tested a couple of other channels now and they seem to play back just fine - so I guess it might have been a problem related to the other channel.

But the welle-cli still crashes when I remove the receiver (firefox/vlc) - and I have no idea what might be causing this

Failed to send audio for 55768
Removing mp3 sender
free(): invalid pointer
zsh: IOT instruction  ./build/welle-cli -w 8080

comcloudway avatar Jun 21 '22 12:06 comcloudway

OK, So I found that the segfault is caused by line 909 in the webradiointerface.cpp file (apparently I missed the error message telling me that the pointer is invalid).

Removing mp3 sender
free(): invalid pointer

I also tried the /mp3 endpoint (build from the current master) but it did not throw an error. But I fail to explain why this is happening, as I'm able to borrow it a couple of lines above: I guess that the second thread might delete it before the send_mp3had time to unregister the sender.

I'll try to investigate this further – but I'm kind of clueless, because I'm moving s, so it should only point nowhere if I remove the sender beforehand – but why doesn't this happen in master

comcloudway avatar Jul 04 '22 10:07 comcloudway

The sender will not disappear on line 909, it's not a pointer, and it's still in scope.

But what about ph ? It's a reference to an element in the collection phs that could be deleted elsewhere.

Maybe it's something else altogether. Compiling with -fsanitize=address can facilitate the investigation.

mpbraendli avatar Jul 04 '22 11:07 mpbraendli

I just realized that I forgot the post my old comment.

As it's been about three weeks since I wrote it (and apparently deleted it) the details might be a bit fuzzy: I basically went through and debugged every single pointer (according to a c++ wiki I've read &sender is in fact a pointer, although the problem is not caused by that part. From what I can tell you were right in assuming that ph was dropped at some point. I'm guessing that the WebProgrammeHandler I've manually created here is being dropped. But I could not figure out why.

comcloudway avatar Aug 18 '22 07:08 comcloudway

What I think is weird, though, is the fact that I want the ph to disappear. Which would mean that ignoring the error (using try and catch) should solve the problem, but to be able to add the missing try and catch block I would have to find the line from which the error originates.

comcloudway avatar Aug 18 '22 13:08 comcloudway

Why did you close it? Just cloned next branch and compiled. Using /mp3 takes over cpu to max.

SzymonZy avatar Oct 01 '22 11:10 SzymonZy

As mentioned in #748 , this PR and #740 are supposed to be undone, so in preparation for that I closed this

EDIT: I'm hoping to have the PR ready today

comcloudway avatar Oct 01 '22 16:10 comcloudway