[vlc-devel] [PATCH] opensles_android: conditional compile for native sample rate check

[Alexandre Janniaux]

Hi,

On Mon, Feb 01, 2021 at 09:29:02PM +0800, "zhilizhao(赵志立)" wrote:
> Hi Thomas,
>
> > On Feb 1, 2021, at 5:19 PM, Thomas Guillem <thomas at gllm.fr> wrote:
> >
> > Hello Zhao,
> >
> > You need to check the sdk dynamically and not at compile time (__ANDROID_API__ is fixed at compile time, a minimum value, then the build run on different API verions).
> >
>
> Yes, the disadvantage of compile time check is that it still do the check on higher Android version.
> Since most devices support arm64 and Android support arm64 since 5.0, the disadvantage affect
> apps with only armv7 shared library. The native sample rate check has a little cost, and runtime
> device api level check has a little cost too. Here is another solution, do runtime check only if
> compile time check failed:
>
> #if CHECK_NATIVE_SAMPLE_RATE
> if (android_get_device_api_level() < 21 && aout_get_native_sample_rate(aout) < (int)fmt->i_rate) {
>     result = SL_RESULT_UNKNOWN_ERROR;
> } else
> # endif
>     …
>
> How about this?

This looks much better. Much like how it is working on iOS/tvOS too,
though @available clang macro makes it easier to achieve both at the
same time.

Regards,
--
Alexandre Janniaux
Videolabs


> > On Sat, Jan 30, 2021, at 16:15, Zhao Zhili wrote:
> >> From: Zhao Zhili <zhilizhao at tencent.com>
> >>
> >> The android bug has been fixed since Android 5.0.
> >> ---
> >> modules/audio_output/opensles_android.c | 19 ++++++++++++-------
> >> 1 file changed, 12 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/modules/audio_output/opensles_android.c
> >> b/modules/audio_output/opensles_android.c
> >> index 5459c40f04..72a0ada7ae 100644
> >> --- a/modules/audio_output/opensles_android.c
> >> +++ b/modules/audio_output/opensles_android.c
> >> @@ -58,6 +58,8 @@ JNIEnv *android_getEnv(vlc_object_t *p_obj, const
> >> char *psz_thread_name);
> >>         goto error; \
> >>     }
> >>
> >> +#define CHECK_NATIVE_SAMPLE_RATE (__ANDROID_API__ < 21)
> >> +
> >> typedef SLresult (*slCreateEngine_t)(
> >>         SLObjectItf*, SLuint32, const SLEngineOption*, SLuint32,
> >>         const SLInterfaceID*, const SLboolean*);
> >> @@ -356,6 +358,7 @@ static void PlayedCallback
> >> (SLAndroidSimpleBufferQueueItf caller, void *pContext
> >>     vlc_mutex_unlock(&sys->lock);
> >> }
> >>
> >> +#if CHECK_NATIVE_SAMPLE_RATE
> >> static int aout_get_native_sample_rate(audio_output_t *aout)
> >> {
> >>     JNIEnv *p_env;
> >> @@ -376,6 +379,7 @@ static int aout_get_native_sample_rate(audio_output_t *aout)
> >>     msg_Dbg(aout, "%s: %d", __func__, sample_rate);
> >>     return sample_rate;
> >> }
> >> +#endif
> >>
> >>
> >> /*****************************************************************************
> >>  *
> >> @@ -417,19 +421,20 @@ static int Start(audio_output_t *aout,
> >> audio_sample_format_t *restrict fmt)
> >>     const SLInterfaceID ids2[] = {
> >> sys->SL_IID_ANDROIDSIMPLEBUFFERQUEUE, sys->SL_IID_VOLUME };
> >>     static const SLboolean req2[] = { SL_BOOLEAN_TRUE, SL_BOOLEAN_TRUE
> >> };
> >>
> >> -    if (aout_get_native_sample_rate(aout) >= (int)fmt->i_rate) {
> >> -        result = CreateAudioPlayer(sys->engineEngine,
> >> &sys->playerObject, &audioSrc,
> >> -                                    &audioSnk, sizeof(ids2) /
> >> sizeof(*ids2),
> >> -                                    ids2, req2);
> >> -    } else {
> >> +#if CHECK_NATIVE_SAMPLE_RATE
> >> +    if (aout_get_native_sample_rate(aout) < (int)fmt->i_rate) {
> >>         // Don't try to play back a sample rate higher than the native
> >> one,
> >>         // since OpenSL ES will try to use the fast path, which
> >> AudioFlinger
> >>         // will reject (fast path can't do resampling), and will end
> >> up with
> >>         // too small buffers for the resampling. See
> >> http://b.android.com/59453
> >> -        // for details. This bug is still present in 4.4. If it is
> >> fixed later
> >> -        // this workaround could be made conditional.
> >> +        // for details. This bug is fixed in 5.0
> >>         result = SL_RESULT_UNKNOWN_ERROR;
> >>     }
> >> +    else
> >> +#endif
> >> +        result = CreateAudioPlayer(sys->engineEngine,
> >> &sys->playerObject, &audioSrc,
> >> +                                    &audioSnk, sizeof(ids2) /
> >> sizeof(*ids2),
> >> +                                    ids2, req2);
> >>     if (unlikely(result != SL_RESULT_SUCCESS)) {
> >>         /* Try again with a more sensible samplerate */
> >>         fmt->i_rate = 44100;
> >> --
> >> 2.28.0
> >>
> >> _______________________________________________
> >> vlc-devel mailing list
> >> To unsubscribe or modify your subscription options:
> >> https://mailman.videolan.org/listinfo/vlc-devel
> > _______________________________________________
> > vlc-devel mailing list
> > To unsubscribe or modify your subscription options:
> > https://mailman.videolan.org/listinfo/vlc-devel
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel