From ae76af2ec5e6d982ea02685991982bd2b2d9a3ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20Beauz=C3=A9e-Luyssen?= Date: Fri, 24 Sep 2021 13:58:17 +0200 Subject: [PATCH] medialibrary: Use utils::jni::string for native strings This removes the need to manually invoke DeleteLocalRef and fixes a leak on error in mediaToMediaWrapper --- medialibrary/jni/AndroidMediaLibrary.cpp | 52 ++++++-------- medialibrary/jni/medialibrary.cpp | 16 ++--- medialibrary/jni/utils.cpp | 92 +++++++++--------------- medialibrary/jni/utils.h | 2 +- 4 files changed, 63 insertions(+), 99 deletions(-) diff --git a/medialibrary/jni/AndroidMediaLibrary.cpp b/medialibrary/jni/AndroidMediaLibrary.cpp index 3803fbd6f..519ea8040 100644 --- a/medialibrary/jni/AndroidMediaLibrary.cpp +++ b/medialibrary/jni/AndroidMediaLibrary.cpp @@ -927,13 +927,11 @@ void AndroidMediaLibrary::onDiscoveryProgress( const std::string& entryPoint ) { JNIEnv *env = getEnv(); if (env == NULL) return; - jstring ep = vlcNewStringUTF(env, entryPoint.c_str()); + auto ep = vlcNewStringUTF(env, entryPoint.c_str()); if (weak_thiz) { - env->CallVoidMethod(weak_thiz, p_fields->MediaLibrary.onDiscoveryProgressId, ep); + env->CallVoidMethod(weak_thiz, p_fields->MediaLibrary.onDiscoveryProgressId, ep.get()); } - env->DeleteLocalRef(ep); - } void AndroidMediaLibrary::onDiscoveryCompleted() @@ -953,48 +951,44 @@ void AndroidMediaLibrary::onDiscoveryFailed(const std::string &entryPoint) JNIEnv *env = getEnv(); if (env == NULL) return; - jstring ep = vlcNewStringUTF(env, entryPoint.c_str()); + auto ep = vlcNewStringUTF(env, entryPoint.c_str()); if (weak_thiz) { - env->CallVoidMethod(weak_thiz, p_fields->MediaLibrary.onDiscoveryFailedId, ep); + env->CallVoidMethod(weak_thiz, p_fields->MediaLibrary.onDiscoveryFailedId, ep.get()); } - env->DeleteLocalRef(ep); } void AndroidMediaLibrary::onReloadStarted( const std::string& entryPoint ) { JNIEnv *env = getEnv(); if (env == NULL) return; - jstring ep = vlcNewStringUTF(env, entryPoint.c_str()); + auto ep = vlcNewStringUTF(env, entryPoint.c_str()); if (weak_thiz) { - env->CallVoidMethod(weak_thiz, p_fields->MediaLibrary.onReloadStartedId, ep); + env->CallVoidMethod(weak_thiz, p_fields->MediaLibrary.onReloadStartedId, ep.get()); } - env->DeleteLocalRef(ep); } void AndroidMediaLibrary::onReloadCompleted( const std::string& entryPoint, bool success ) { JNIEnv *env = getEnv(); if (env == NULL) return; - jstring ep = vlcNewStringUTF(env, entryPoint.c_str()); + auto ep = vlcNewStringUTF(env, entryPoint.c_str()); if (weak_thiz) { - env->CallVoidMethod(weak_thiz, p_fields->MediaLibrary.onReloadCompletedId, ep); + env->CallVoidMethod(weak_thiz, p_fields->MediaLibrary.onReloadCompletedId, ep.get()); } - env->DeleteLocalRef(ep); } void AndroidMediaLibrary::onEntryPointBanned( const std::string& entryPoint, bool success ) { JNIEnv *env = getEnv(); if (env == NULL) return; - jstring ep = vlcNewStringUTF(env, entryPoint.c_str()); + auto ep = vlcNewStringUTF(env, entryPoint.c_str()); if (weak_thiz) { - env->CallVoidMethod(weak_thiz, p_fields->MediaLibrary.onEntryPointBannedId, ep, success); + env->CallVoidMethod(weak_thiz, p_fields->MediaLibrary.onEntryPointBannedId, ep.get(), success); } - env->DeleteLocalRef(ep); } void AndroidMediaLibrary::onEntryPointUnbanned( const std::string& entryPoint, bool success ) @@ -1002,36 +996,33 @@ void AndroidMediaLibrary::onEntryPointUnbanned( const std::string& entryPoint, b JNIEnv *env = getEnv(); if (env == NULL) return; - jstring ep = vlcNewStringUTF(env, entryPoint.c_str()); + auto ep = vlcNewStringUTF(env, entryPoint.c_str()); if (weak_thiz) { - env->CallVoidMethod(weak_thiz, p_fields->MediaLibrary.onEntryPointUnbannedId, ep, success); + env->CallVoidMethod(weak_thiz, p_fields->MediaLibrary.onEntryPointUnbannedId, ep.get(), success); } - env->DeleteLocalRef(ep); } void AndroidMediaLibrary::onEntryPointAdded( const std::string& entryPoint, bool success ) { JNIEnv *env = getEnv(); if (env == NULL) return; - jstring ep = vlcNewStringUTF(env, entryPoint.c_str()); + auto ep = vlcNewStringUTF(env, entryPoint.c_str()); if (weak_thiz) { - env->CallVoidMethod(weak_thiz, p_fields->MediaLibrary.onEntryPointAddedId, ep, success); + env->CallVoidMethod(weak_thiz, p_fields->MediaLibrary.onEntryPointAddedId, ep.get(), success); } - env->DeleteLocalRef(ep); } void AndroidMediaLibrary::onEntryPointRemoved( const std::string& entryPoint, bool success ) { JNIEnv *env = getEnv(); if (env == NULL) return; - jstring ep = vlcNewStringUTF(env, entryPoint.c_str()); + auto ep = vlcNewStringUTF(env, entryPoint.c_str()); if (weak_thiz) { - env->CallVoidMethod(weak_thiz, p_fields->MediaLibrary.onEntryPointRemovedId, ep, success); + env->CallVoidMethod(weak_thiz, p_fields->MediaLibrary.onEntryPointRemovedId, ep.get(), success); } - env->DeleteLocalRef(ep); } void AndroidMediaLibrary::onParsingStatsUpdated( uint32_t opsDone, uint32_t opsScheduled ) @@ -1113,18 +1104,17 @@ void AndroidMediaLibrary::onMediaThumbnailReady( medialibrary::MediaPtr media, m if (env != NULL && weak_thiz) { auto item = mediaToMediaWrapper(env, p_fields, media); - env->CallVoidMethod(weak_thiz, p_fields->MediaLibrary.onMediaThumbnailReadyId, item, success); + env->CallVoidMethod(weak_thiz, p_fields->MediaLibrary.onMediaThumbnailReadyId, item.get(), success); } } bool AndroidMediaLibrary::onUnhandledException( const char* context, const char* errMsg, bool clearSuggested ) { JNIEnv *env = getEnv(); - jstring ctx = vlcNewStringUTF(env, context); - jstring msg = vlcNewStringUTF(env, errMsg); - env->CallVoidMethod(weak_thiz, p_fields->MediaLibrary.onUnhandledExceptionId, ctx, msg, clearSuggested); - env->DeleteLocalRef(ctx); - env->DeleteLocalRef(msg); + auto ctx = vlcNewStringUTF(env, context); + auto msg = vlcNewStringUTF(env, errMsg); + env->CallVoidMethod(weak_thiz, p_fields->MediaLibrary.onUnhandledExceptionId, + ctx.get(), msg.get(), clearSuggested); return true; } diff --git a/medialibrary/jni/medialibrary.cpp b/medialibrary/jni/medialibrary.cpp index accce0854..528000c8b 100644 --- a/medialibrary/jni/medialibrary.cpp +++ b/medialibrary/jni/medialibrary.cpp @@ -132,9 +132,8 @@ devices(JNIEnv* env, jobject thiz) jobjectArray deviceRefs = (jobjectArray) env->NewObjectArray(devices.size(), env->FindClass("java/lang/String"), NULL); int index = -1; for(auto device : devices) { - jstring path = vlcNewStringUTF(env, std::get<1>(device).c_str()); - env->SetObjectArrayElement(deviceRefs, ++index, path); - env->DeleteLocalRef(path); + auto path = vlcNewStringUTF(env, std::get<1>(device).c_str()); + env->SetObjectArrayElement(deviceRefs, ++index, path.get()); } return deviceRefs; } @@ -190,9 +189,8 @@ entryPoints(JNIEnv* env, jobject thiz) jobjectArray mediaRefs = (jobjectArray) env->NewObjectArray(mrls.size(), env->FindClass("java/lang/String"), NULL); int index = -1; for( const std::string& m : mrls ) { - jstring mrl = vlcNewStringUTF(env, m.c_str()); - env->SetObjectArrayElement(mediaRefs, ++index, mrl); - env->DeleteLocalRef(mrl); + auto mrl = vlcNewStringUTF(env, m.c_str()); + env->SetObjectArrayElement(mediaRefs, ++index, mrl.get()); } return mediaRefs; } @@ -1493,7 +1491,9 @@ getMediaStringMetadata(JNIEnv* env, jobject thiz, jobject medialibrary, jlong id medialibrary::MediaPtr media = aml->media(id); if (media == nullptr) return 0L; const medialibrary::IMetadata& metadata = media->metadata((medialibrary::IMedia::MetadataType)metadataType); - return metadata.isSet() ? vlcNewStringUTF(env, metadata.asStr().c_str()) : nullptr; + if (!metadata.isSet()) + return nullptr; + return vlcNewStringUTF(env, metadata.asStr().c_str()).release(); } void @@ -1961,7 +1961,7 @@ groupRemoveId(JNIEnv* env, jobject thiz, jobject medialibrary, jlong id, jlong m jstring groupName(JNIEnv* env, jobject thiz, jobject medialibrary, jlong id) { - return vlcNewStringUTF(env, MediaLibrary_getInstance(env, medialibrary)->groupName(id).c_str()); + return vlcNewStringUTF(env, MediaLibrary_getInstance(env, medialibrary)->groupName(id).c_str()).release(); } jboolean diff --git a/medialibrary/jni/utils.cpp b/medialibrary/jni/utils.cpp index 4354e55a3..d5f5e763c 100644 --- a/medialibrary/jni/utils.cpp +++ b/medialibrary/jni/utils.cpp @@ -25,7 +25,7 @@ mediaToMediaWrapper(JNIEnv* env, fields *fields, medialibrary::MediaPtr const& m break; } medialibrary::AlbumTrackPtr p_albumTrack = mediaPtr->albumTrack(); - jstring artist = NULL, genre = NULL, album = NULL, albumArtist = NULL, mrl = NULL, title = NULL, thumbnail = NULL, filename = NULL; + utils::jni::string artist, genre, album, albumArtist, mrl, title, thumbnail, filename; jint trackNumber = 0, discNumber = 0; const bool isPresent = mediaPtr->isPresent(); @@ -71,118 +71,92 @@ mediaToMediaWrapper(JNIEnv* env, fields *fields, medialibrary::MediaPtr const& m auto hasThumbnail = mediaPtr->thumbnailStatus(medialibrary::ThumbnailSizeType::Thumbnail) == medialibrary::ThumbnailStatus::Available; jobject item = env->NewObject(fields->MediaWrapper.clazz, fields->MediaWrapper.initID, - (jlong) mediaPtr->id(), mrl, (jlong) mediaPtr->lastTime(), (jfloat) mediaPtr->lastPosition(), (jlong) duration, type, - title, filename, artist, genre, album, - albumArtist, width, height, thumbnail, + (jlong) mediaPtr->id(), mrl.get(), (jlong) mediaPtr->lastTime(), (jfloat) mediaPtr->lastPosition(), (jlong) duration, type, + title.get(), filename.get(), artist.get(), genre.get(), album.get(), + albumArtist.get(), width, height, thumbnail.get(), audioTrack, spuTrack, trackNumber, discNumber, (jlong) files.at(0)->lastModificationDate(), (jlong) mediaPtr->playCount(), hasThumbnail, mediaPtr->releaseDate(), isPresent); - if (artist != NULL) - env->DeleteLocalRef(artist); - if (genre != NULL) - env->DeleteLocalRef(genre); - if (album != NULL) - env->DeleteLocalRef(album); - if (albumArtist != NULL) - env->DeleteLocalRef(albumArtist); - if (title != NULL) - env->DeleteLocalRef(title); - if (mrl != NULL) - env->DeleteLocalRef(mrl); - if (thumbnail != NULL) - env->DeleteLocalRef(thumbnail); - if (filename != NULL) - env->DeleteLocalRef(filename); return item; } jobject convertAlbumObject(JNIEnv* env, fields *fields, medialibrary::AlbumPtr const& albumPtr) { - jstring title = vlcNewStringUTF(env, albumPtr->title().c_str()); - jstring thumbnailMrl = vlcNewStringUTF(env, albumPtr->thumbnailMrl(medialibrary::ThumbnailSizeType::Thumbnail).c_str()); + auto title = vlcNewStringUTF(env, albumPtr->title().c_str()); + auto thumbnailMrl = vlcNewStringUTF(env, albumPtr->thumbnailMrl(medialibrary::ThumbnailSizeType::Thumbnail).c_str()); medialibrary::ArtistPtr artist = albumPtr->albumArtist(); jlong albumArtistId = artist != nullptr ? albumPtr->albumArtist()->id() : 0; - jstring artistName = artist != nullptr ? vlcNewStringUTF(env, artist->name().c_str()) : NULL; + utils::jni::string artistName; + if ( artist != nullptr ) + artistName = vlcNewStringUTF(env, artist->name().c_str()); jobject item = env->NewObject(fields->Album.clazz, fields->Album.initID, - (jlong) albumPtr->id(), title, albumPtr->releaseYear(), thumbnailMrl, artistName, albumArtistId, (jint) albumPtr->nbTracks(), (jint) albumPtr->nbPresentTracks(), albumPtr->duration()); - env->DeleteLocalRef(title); - env->DeleteLocalRef(thumbnailMrl); - env->DeleteLocalRef(artistName); + (jlong) albumPtr->id(), title.get(), albumPtr->releaseYear(), + thumbnailMrl.get(), artistName.get(), albumArtistId, (jint) albumPtr->nbTracks(), (jint) albumPtr->nbPresentTracks(), albumPtr->duration()); return item; } jobject convertArtistObject(JNIEnv* env, fields *fields, medialibrary::ArtistPtr const& artistPtr) { - jstring name = vlcNewStringUTF(env, artistPtr->name().c_str()); - jstring thumbnailMrl = vlcNewStringUTF(env, artistPtr->thumbnailMrl(medialibrary::ThumbnailSizeType::Thumbnail).c_str()); - jstring shortBio = vlcNewStringUTF(env, artistPtr->shortBio().c_str()); - jstring musicBrainzId = vlcNewStringUTF(env, artistPtr->musicBrainzId().c_str()); + auto name = vlcNewStringUTF(env, artistPtr->name().c_str()); + auto thumbnailMrl = vlcNewStringUTF(env, artistPtr->thumbnailMrl(medialibrary::ThumbnailSizeType::Thumbnail).c_str()); + auto shortBio = vlcNewStringUTF(env, artistPtr->shortBio().c_str()); + auto musicBrainzId = vlcNewStringUTF(env, artistPtr->musicBrainzId().c_str()); jobject item = env->NewObject(fields->Artist.clazz, fields->Artist.initID, - (jlong) artistPtr->id(), name, shortBio, thumbnailMrl, musicBrainzId, (jint) artistPtr->nbAlbums(), (jint) artistPtr->nbTracks(), (jint) artistPtr->nbPresentTracks()); - env->DeleteLocalRef(name); - env->DeleteLocalRef(thumbnailMrl); - env->DeleteLocalRef(shortBio); - env->DeleteLocalRef(musicBrainzId); + (jlong) artistPtr->id(), name.get(), shortBio.get(), thumbnailMrl.get(), + musicBrainzId.get(), (jint) artistPtr->nbAlbums(), (jint) artistPtr->nbTracks(), (jint) artistPtr->nbPresentTracks()); return item; } jobject convertGenreObject(JNIEnv* env, fields *fields, medialibrary::GenrePtr const& genrePtr) { - jstring name = vlcNewStringUTF(env, genrePtr->name().c_str()); + auto name = vlcNewStringUTF(env, genrePtr->name().c_str()); jobject item = env->NewObject(fields->Genre.clazz, fields->Genre.initID, - (jlong) genrePtr->id(), name, (jint) genrePtr->nbTracks(), (jint) genrePtr->nbPresentTracks()); - env->DeleteLocalRef(name); + (jlong) genrePtr->id(), name.get(), (jint) genrePtr->nbTracks(), (jint) genrePtr->nbPresentTracks()); return item; } jobject convertPlaylistObject(JNIEnv* env, fields *fields, medialibrary::PlaylistPtr const& playlistPtr, jboolean includeMissing) { - jstring name = vlcNewStringUTF(env, playlistPtr->name().c_str()); + auto name = vlcNewStringUTF(env, playlistPtr->name().c_str()); medialibrary::QueryParameters params { medialibrary::SortingCriteria::Default, false, static_cast( includeMissing ) }; jobject item = env->NewObject(fields->Playlist.clazz, fields->Playlist.initID, - (jlong) playlistPtr->id(), name, (jint)playlistPtr->media(¶ms)->count()); - env->DeleteLocalRef(name); + (jlong) playlistPtr->id(), name.get(), (jint)playlistPtr->media(¶ms)->count()); return item; } jobject convertFolderObject(JNIEnv* env, fields *fields, medialibrary::FolderPtr const& folderPtr, int count) { - jstring name = vlcNewStringUTF(env, folderPtr->name().c_str()); - jstring mrl = vlcNewStringUTF(env, folderPtr->mrl().c_str()); + auto name = vlcNewStringUTF(env, folderPtr->name().c_str()); + auto mrl = vlcNewStringUTF(env, folderPtr->mrl().c_str()); jobject item = env->NewObject(fields->Folder.clazz, fields->Folder.initID, - (jlong) folderPtr->id(), name, mrl, (jint) count); - env->DeleteLocalRef(name); - env->DeleteLocalRef(mrl); + (jlong) folderPtr->id(), name.get(), mrl.get(), (jint) count); return item; } jobject convertVideoGroupObject(JNIEnv* env, fields *fields, medialibrary::MediaGroupPtr const& videogroupPtr) { - jstring name = vlcNewStringUTF(env, videogroupPtr->name().c_str()); + auto name = vlcNewStringUTF(env, videogroupPtr->name().c_str()); jobject item = env->NewObject(fields->VideoGroup.clazz, fields->VideoGroup.initID, - (jlong) videogroupPtr->id(), name, (jint)videogroupPtr->nbVideo(), (jint)videogroupPtr->nbPresentVideo()); - env->DeleteLocalRef(name); + (jlong) videogroupPtr->id(), name.get(), (jint)videogroupPtr->nbVideo(), (jint)videogroupPtr->nbPresentVideo()); return item; } jobject convertBookmarkObject(JNIEnv* env, fields *fields, medialibrary::BookmarkPtr const& bookmarkPtr) { - jstring name = vlcNewStringUTF(env, bookmarkPtr->name().c_str()); - jstring description = vlcNewStringUTF(env, bookmarkPtr->description().c_str()); + auto name = vlcNewStringUTF(env, bookmarkPtr->name().c_str()); + auto description = vlcNewStringUTF(env, bookmarkPtr->description().c_str()); jobject item = env->NewObject(fields->Bookmark.clazz, fields->Bookmark.initID, - (jlong) bookmarkPtr->id(), name, description, (jlong) bookmarkPtr->mediaId(), (jlong) bookmarkPtr->time()); - env->DeleteLocalRef(name); - env->DeleteLocalRef(description); + (jlong) bookmarkPtr->id(), name.get(), description.get(), (jlong) bookmarkPtr->mediaId(), (jlong) bookmarkPtr->time()); return item; } @@ -312,7 +286,7 @@ filteredArray(JNIEnv* env, jobjectArray array, jclass clazz, int removalCount) return mediaRefs; } -jstring +utils::jni::string vlcNewStringUTF(JNIEnv* env, const char* psz_string) { for (int i = 0 ; psz_string[i] != '\0' ; ) { @@ -328,15 +302,15 @@ vlcNewStringUTF(JNIEnv* env, const char* psz_string) nbBytes = 3; else { LOGE("Invalid UTF lead character\n"); - return NULL; + return {}; } for (int j = 0 ; j < nbBytes && psz_string[i] != '\0' ; j++) { uint8_t byte = psz_string[i++]; if ((byte & 0x80) == 0) { LOGE("Invalid UTF byte\n"); - return NULL; + return {}; } } } - return env->NewStringUTF(psz_string); + return utils::jni::string{ env, env->NewStringUTF(psz_string) }; } diff --git a/medialibrary/jni/utils.h b/medialibrary/jni/utils.h index 8a955d98c..86457f370 100644 --- a/medialibrary/jni/utils.h +++ b/medialibrary/jni/utils.h @@ -238,6 +238,6 @@ jobject convertBookmarkObject(JNIEnv* env, fields *fields, medialibrary::Bookmar jobject convertSearchAggregateObject(JNIEnv* env, fields *fields, medialibrary::SearchAggregate const& searchAggregatePtr, jboolean includeMissing); jobjectArray filteredArray(JNIEnv* env, jobjectArray array, jclass clazz, int removalCount = -1); jlongArray idArray(JNIEnv* env, std::set ids); -jstring vlcNewStringUTF(JNIEnv* env, const char* psz_string); +utils::jni::string vlcNewStringUTF(JNIEnv* env, const char* psz_string); #endif //VLC_MEDIALIB_UTILS_H