Limit number of Oboe auto restart

[sauwming]

To fix #4218.

It is reported that Oboe will keep restarting indefinitely upon error.

Also in this PR:
minor: remove unused label on_skip_dev.

[nanangizz]

"permanent" because the nrestart will never be reset, so the instance is no longer usable?
Perhaps it can be avoided by adding param to Start(), something like inc_counter flag, which should be true only if it is called by error case? Not sure if it is necessary though.
This may also avoid putting nrestart = 0 in many places?

[sauwming]

OK, remove the log. Now it will use the already existing log message Oboe stream restart failed. Will also notify the callback via PJMEDIA_EVENT_AUD_DEV_ERROR event, so this should be better.

Should I still modify the nrestart counter handling?
IMO, it still needs to be reset when the stream is working (i.e. if it successfully captures or plays an audio frame).

[nanangizz]

OK, remove the log. Now it will use the already existing log message Oboe stream restart failed. Will also notify the callback via PJMEDIA_EVENT_AUD_DEV_ERROR event, so this should be better.

Okay.

Just a note: putting ++nrestart inside PJ_LOG(3, ..) is not ideal, as it will not be executed (even not compiled) when PJ_MAX_LOG_LEVEL is set to <3?

Should I still modify the nrestart counter handling? IMO, it still needs to be reset when the stream is working (i.e. if it successfully captures or plays an audio frame).

Yes, it needs to be reset, just the reset places looks rather strange (multiple lines inside the callback), can it be put just once around here for example? Or there are cases that Start() may succeed but somehow the callback is not invoked?

pjproject/pjmedia/src/pjmedia-audiodev/oboe_dev.cpp

Lines 685 to 687 in 43bdeaa

	pj_mutex_unlock(mutex);
	return PJ_SUCCESS;

[sauwming]

It can't be put there (in ~L685) because then Start() will immediately reset the counter (after just incremented).

I move it to onAudioReady() instead.

[nanangizz]

Still don't understand why it can't, as the reset is supposed to be done after the start succeeds (and not reset when fails). And putting it in callback make the reset will be done every callback instead of once only, should work just fine just rather seem strange/unnecessary :)
Also there seem to still another line that reset it in the thread loop too.

Anyway it is just a minor.

[sauwming]

Start() usually just returns success, this is because requestStart() is asynchronous and returns immediately.
(https://google.github.io/oboe/classoboe_1_1_audio_stream.html#a3c484e314dee8dfed1d419f487b5d601)

[nanangizz]

Ah okay, now it makes sense.

[sauwming]

Alternatively, perhaps we can use blocking start() instead?

virtual Result oboe::AudioStream::start(int64_t timeoutNanoseconds = kDefaultTimeoutNanos)

Start the stream. This will block until the stream has been started, an error occurs or 
timeoutNanoseconds has been reached.

I didn't change it because I don't know the reason why the non-blocking version was selected to be used in the first place.

[nanangizz]

I can't remember it either, perhaps to avoid UI hiccup, e.g: make call always tries opening sound device first. I think better to just leave it for now?