Skip to content

Commit

Permalink
Addressed code review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
Isira-Seneviratne committed Jul 22, 2024
1 parent da83646 commit 7c83a42
Show file tree
Hide file tree
Showing 6 changed files with 57 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1473,6 +1473,7 @@ public void showLoading() {
CoilUtils.dispose(binding.detailThumbnailImageView);
CoilUtils.dispose(binding.detailSubChannelThumbnailView);
CoilUtils.dispose(binding.overlayThumbnail);
CoilUtils.dispose(binding.detailUploaderThumbnailView);

binding.detailThumbnailImageView.setImageBitmap(null);
binding.detailSubChannelThumbnailView.setImageBitmap(null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
14 changes: 12 additions & 2 deletions app/src/main/java/org/schabi/newpipe/player/Player.java
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,10 @@ public final class Player implements PlaybackListener, Listener {
private MediaItemTag currentMetadata;
@Nullable
private Bitmap currentThumbnail;
@Nullable
private coil.request.Disposable thumbnailDisposable;
@NonNull
private Target thumbnailTarget;

/*//////////////////////////////////////////////////////////////////////////
// Player
Expand Down Expand Up @@ -772,6 +776,11 @@ private void loadCurrentThumbnail(final List<Image> thumbnails) {
+ 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);
Expand All @@ -780,7 +789,7 @@ private void loadCurrentThumbnail(final List<Image> thumbnails) {
}

// scale down the notification thumbnail for performance
final var target = new Target() {
thumbnailTarget = new Target() {
@Override
public void onError(@Nullable final Drawable error) {
Log.e(TAG, "Thumbnail - onError() called");
Expand All @@ -805,7 +814,8 @@ public void onSuccess(@NonNull final Drawable result) {
result.getIntrinsicHeight(), null));
}
};
CoilHelper.INSTANCE.loadScaledDownThumbnail(context, thumbnails, target);
thumbnailDisposable = CoilHelper.INSTANCE
.loadScaledDownThumbnail(context, thumbnails, thumbnailTarget);
}

private void onThumbnailLoaded(@Nullable final Bitmap bitmap) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,8 @@ public void onCreatePreferences(final Bundle savedInstanceState, final String ro
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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,17 @@
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.nio.file.StandardCopyOption;
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();

Expand Down Expand Up @@ -335,7 +339,13 @@ public static void copyToClipboard(@NonNull final Context context, final String
}

/**
* 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
Expand All @@ -351,11 +361,6 @@ public static void copyToClipboard(@NonNull final Context context, final String
* </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>
Expand All @@ -369,33 +374,41 @@ private static ClipData generateClipDataForImagePreview(
@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 appFolder = applicationContext.getCacheDir().toPath();
final var preview = appFolder.resolve("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();
if (value != null) {
// Any existing file will be overwritten
try (var outputStream = Files.newOutputStream(preview)) {
value.getBitmap().compress(Bitmap.CompressFormat.JPEG, 90, outputStream);
}
} else {
try (var snapshot = loader.getDiskCache().openSnapshot(thumbnailUrl)) {
if (snapshot != null) {
final var path = snapshot.getData().toNioPath();
Files.copy(path, preview, StandardCopyOption.REPLACE_EXISTING);
} else {
return null;
}
}
}

final ClipData clipData = ClipData.newUri(applicationContext.getContentResolver(), "",
FileProvider.getUriForFile(applicationContext,
BuildConfig.APPLICATION_ID + ".provider",
thumbnailPreviewFile));
preview.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;
Expand Down
7 changes: 5 additions & 2 deletions app/src/main/java/org/schabi/newpipe/util/image/CoilHelper.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -79,7 +80,7 @@ object CoilHelper {
})
.build()

context.imageLoader.enqueue(request)
return context.imageLoader.enqueue(request)
}

fun loadDetailsThumbnail(target: ImageView, images: List<Image>) {
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 7c83a42

Please sign in to comment.