Addressed code review comments

This commit is contained in:
Isira Seneviratne 2024-07-22 09:41:07 +05:30
parent da83646303
commit 4ec7532126
6 changed files with 60 additions and 35 deletions

View File

@ -1473,6 +1473,7 @@ public final class VideoDetailFragment
CoilUtils.dispose(binding.detailThumbnailImageView);
CoilUtils.dispose(binding.detailSubChannelThumbnailView);
CoilUtils.dispose(binding.overlayThumbnail);
CoilUtils.dispose(binding.detailUploaderThumbnailView);
binding.detailThumbnailImageView.setImageBitmap(null);
binding.detailSubChannelThumbnailView.setImageBitmap(null);

View File

@ -76,8 +76,7 @@ class NotificationHelper(val context: Context) {
summaryBuilder.setLargeIcon(avatarIcon)
// Show individual stream notifications, set channel icon only if there is actually
// one
// Show individual stream notifications, set channel icon only if there is actually one
showStreamNotifications(newStreams, data.serviceId, avatarIcon)
// Show summary notification
manager.notify(data.pseudoId, summaryBuilder.build())

View File

@ -192,6 +192,8 @@ public final class Player implements PlaybackListener, Listener {
private MediaItemTag currentMetadata;
@Nullable
private Bitmap currentThumbnail;
@Nullable
private coil.request.Disposable thumbnailDisposable;
/*//////////////////////////////////////////////////////////////////////////
// Player
@ -772,6 +774,11 @@ public final class Player implements PlaybackListener, Listener {
+ thumbnails.size() + "]");
}
// Cancel any ongoing image loading
if (thumbnailDisposable != null) {
thumbnailDisposable.dispose();
}
// Unset currentThumbnail, since it is now outdated. This ensures it is not used in media
// session metadata while the new thumbnail is being loaded by Coil.
onThumbnailLoaded(null);
@ -780,7 +787,7 @@ public final class Player implements PlaybackListener, Listener {
}
// scale down the notification thumbnail for performance
final var target = new Target() {
final var thumbnailTarget = new Target() {
@Override
public void onError(@Nullable final Drawable error) {
Log.e(TAG, "Thumbnail - onError() called");
@ -805,7 +812,8 @@ public final class Player implements PlaybackListener, Listener {
result.getIntrinsicHeight(), null));
}
};
CoilHelper.INSTANCE.loadScaledDownThumbnail(context, thumbnails, target);
thumbnailDisposable = CoilHelper.INSTANCE
.loadScaledDownThumbnail(context, thumbnails, thumbnailTarget);
}
private void onThumbnailLoaded(@Nullable final Bitmap bitmap) {

View File

@ -42,12 +42,8 @@ public class ContentSettingsFragment extends BasePreferenceFragment {
ImageStrategy.setPreferredImageQuality(PreferredImageQuality
.fromPreferenceKey(requireContext(), (String) newValue));
final var loader = Coil.imageLoader(preference.getContext());
if (loader.getMemoryCache() != null) {
loader.getMemoryCache().clear();
}
if (loader.getDiskCache() != null) {
loader.getDiskCache().clear();
}
loader.getMemoryCache().clear();
loader.getDiskCache().clear();
Toast.makeText(preference.getContext(),
R.string.thumbnail_cache_wipe_complete_notice, Toast.LENGTH_SHORT)
.show();

View File

@ -10,6 +10,7 @@ import android.content.Intent;
import android.content.pm.PackageManager;
import android.content.pm.ResolveInfo;
import android.graphics.Bitmap;
import android.graphics.BitmapFactory;
import android.net.Uri;
import android.os.Build;
import android.text.TextUtils;
@ -24,13 +25,16 @@ import androidx.core.content.FileProvider;
import org.schabi.newpipe.BuildConfig;
import org.schabi.newpipe.R;
import org.schabi.newpipe.extractor.Image;
import org.schabi.newpipe.util.image.CoilHelper;
import org.schabi.newpipe.util.image.ImageStrategy;
import java.io.File;
import java.io.FileOutputStream;
import java.nio.file.Files;
import java.util.Collections;
import java.util.List;
import coil.Coil;
import coil.disk.DiskCache;
import coil.memory.MemoryCache;
public final class ShareUtils {
private static final String TAG = ShareUtils.class.getSimpleName();
@ -335,7 +339,13 @@ public final class ShareUtils {
}
/**
* Generate a {@link ClipData} with the image of the content shared.
* Generate a {@link ClipData} with the image of the content shared, if it's in the app cache.
*
* <p>
* In order not to worry about network issues (timeouts, DNS issues, low connection speed, ...)
* when sharing a content, only images in the {@link MemoryCache} or {@link DiskCache}
* used by the Coil library are used as preview images. If the thumbnail image is not in the
* cache, no {@link ClipData} will be generated and {@code null} will be returned.
*
* <p>
* In order to display the image in the content preview of the Android share sheet, an URI of
@ -351,11 +361,6 @@ public final class ShareUtils {
* </p>
*
* <p>
* This method will call {@link CoilHelper#loadBitmapBlocking(Context, String)} to get the
* thumbnail of the content.
* </p>
*
* <p>
* Using the result of this method when sharing has only an effect on the system share sheet (if
* OEMs didn't change Android system standard behavior) on Android API 29 and higher.
* </p>
@ -369,33 +374,46 @@ public final class ShareUtils {
@NonNull final Context context,
@NonNull final String thumbnailUrl) {
try {
final var bitmap = CoilHelper.INSTANCE.loadBitmapBlocking(context, thumbnailUrl);
if (bitmap == null) {
return null;
}
// Save the image in memory to the application's cache because we need a URI to the
// image to generate a ClipData which will show the share sheet, and so an image file
final Context applicationContext = context.getApplicationContext();
final String appFolder = applicationContext.getCacheDir().getAbsolutePath();
final File thumbnailPreviewFile = new File(appFolder
+ "/android_share_sheet_image_preview.jpg");
final var loader = Coil.imageLoader(context);
final var value = loader.getMemoryCache()
.get(new MemoryCache.Key(thumbnailUrl, Collections.emptyMap()));
// Any existing file will be overwritten with FileOutputStream
final FileOutputStream fileOutputStream = new FileOutputStream(thumbnailPreviewFile);
bitmap.compress(Bitmap.CompressFormat.JPEG, 90, fileOutputStream);
fileOutputStream.close();
final Bitmap cachedBitmap;
if (value != null) {
cachedBitmap = value.getBitmap();
} else {
try (var snapshot = loader.getDiskCache().openSnapshot(thumbnailUrl)) {
if (snapshot != null) {
cachedBitmap = BitmapFactory.decodeFile(snapshot.getData().toString());
} else {
cachedBitmap = null;
}
}
}
if (cachedBitmap == null) {
return null;
}
final var path = applicationContext.getCacheDir().toPath()
.resolve("android_share_sheet_image_preview.jpg");
// Any existing file will be overwritten
try (var outputStream = Files.newOutputStream(path)) {
cachedBitmap.compress(Bitmap.CompressFormat.JPEG, 90, outputStream);
}
final ClipData clipData = ClipData.newUri(applicationContext.getContentResolver(), "",
FileProvider.getUriForFile(applicationContext,
BuildConfig.APPLICATION_ID + ".provider",
thumbnailPreviewFile));
path.toFile()));
if (DEBUG) {
Log.d(TAG, "ClipData successfully generated for Android share sheet: " + clipData);
}
return clipData;
} catch (final Exception e) {
Log.w(TAG, "Error when setting preview image for share sheet", e);
return null;

View File

@ -8,6 +8,7 @@ import androidx.annotation.DrawableRes
import androidx.core.graphics.drawable.toBitmapOrNull
import coil.executeBlocking
import coil.imageLoader
import coil.request.Disposable
import coil.request.ImageRequest
import coil.size.Size
import coil.target.Target
@ -47,7 +48,7 @@ object CoilHelper {
loadImageDefault(target, url, R.drawable.placeholder_thumbnail_video)
}
fun loadScaledDownThumbnail(context: Context, images: List<Image>, target: Target) {
fun loadScaledDownThumbnail(context: Context, images: List<Image>, target: Target): Disposable {
val url = ImageStrategy.choosePreferredImage(images)
val request = getImageRequest(context, url, R.drawable.placeholder_thumbnail_video)
.target(target)
@ -79,7 +80,7 @@ object CoilHelper {
})
.build()
context.imageLoader.enqueue(request)
return context.imageLoader.enqueue(request)
}
fun loadDetailsThumbnail(target: ImageView, images: List<Image>) {
@ -133,6 +134,8 @@ object CoilHelper {
return ImageRequest.Builder(context)
.data(takenUrl)
.error(placeholderResId)
.memoryCacheKey(takenUrl)
.diskCacheKey(takenUrl)
.apply {
if (takenUrl != null || showPlaceholderWhileLoading) {
placeholder(placeholderResId)