From 2aad00df858d3f1bb52c5bd44f0eb60e0a809007 Mon Sep 17 00:00:00 2001 From: Cody Henthorne Date: Mon, 3 May 2021 11:44:20 -0400 Subject: [PATCH] Add ability to configure locale specific media quality settings. Part 1 of improve media quality controls. User selection coming soon. --- .../jobs/AttachmentCompressionJob.java | 2 +- .../securesms/megaphone/Megaphones.java | 6 +- .../securesms/mms/MediaConstraints.java | 5 + .../securesms/mms/PushMediaConstraints.java | 77 +++++++++++++-- .../securesms/util/FeatureFlags.java | 12 ++- .../securesms/util/LocaleFeatureFlags.java | 99 +++++++++++++++++++ .../util/PopulationFeatureFlags.java | 84 ---------------- ...caleFeatureFlagsTest_getCountryValue.java} | 10 +- ...eFeatureFlagsTest_parseCountryValues.java} | 6 +- 9 files changed, 195 insertions(+), 106 deletions(-) create mode 100644 app/src/main/java/org/thoughtcrime/securesms/util/LocaleFeatureFlags.java delete mode 100644 app/src/main/java/org/thoughtcrime/securesms/util/PopulationFeatureFlags.java rename app/src/test/java/org/thoughtcrime/securesms/util/{PopulationFeatureFlagsTest_determineCountEnabled.java => LocaleFeatureFlagsTest_getCountryValue.java} (83%) rename app/src/test/java/org/thoughtcrime/securesms/util/{PopulationFeatureFlagsTest_parseCountryCounts.java => LocaleFeatureFlagsTest_parseCountryValues.java} (85%) diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/AttachmentCompressionJob.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/AttachmentCompressionJob.java index 2e231a1a3f..1e11f22ed1 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/AttachmentCompressionJob.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/AttachmentCompressionJob.java @@ -319,7 +319,7 @@ public final class AttachmentCompressionJob extends BaseJob { new DecryptableStreamUriLoader.DecryptableUri(uri), size, mediaConstraints.getImageMaxSize(context), - 70); + mediaConstraints.getImageCompressionQualitySetting(context)); if (result != null) { break; } diff --git a/app/src/main/java/org/thoughtcrime/securesms/megaphone/Megaphones.java b/app/src/main/java/org/thoughtcrime/securesms/megaphone/Megaphones.java index 341f980b2a..face7921aa 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/megaphone/Megaphones.java +++ b/app/src/main/java/org/thoughtcrime/securesms/megaphone/Megaphones.java @@ -29,7 +29,7 @@ import org.thoughtcrime.securesms.profiles.ProfileName; import org.thoughtcrime.securesms.recipients.Recipient; import org.thoughtcrime.securesms.util.CommunicationActions; import org.thoughtcrime.securesms.util.FeatureFlags; -import org.thoughtcrime.securesms.util.PopulationFeatureFlags; +import org.thoughtcrime.securesms.util.LocaleFeatureFlags; import org.thoughtcrime.securesms.util.TextSecurePreferences; import org.thoughtcrime.securesms.util.VersionTracker; import org.thoughtcrime.securesms.util.dynamiclanguage.DynamicLanguageContextWrapper; @@ -308,11 +308,11 @@ public final class Megaphones { } private static boolean shouldShowResearchMegaphone(@NonNull Context context) { - return VersionTracker.getDaysSinceFirstInstalled(context) > 7 && PopulationFeatureFlags.isInResearchMegaphone(); + return VersionTracker.getDaysSinceFirstInstalled(context) > 7 && LocaleFeatureFlags.isInResearchMegaphone(); } private static boolean shouldShowDonateMegaphone(@NonNull Context context) { - return VersionTracker.getDaysSinceFirstInstalled(context) > 7 && PopulationFeatureFlags.isInDonateMegaphone(); + return VersionTracker.getDaysSinceFirstInstalled(context) > 7 && LocaleFeatureFlags.isInDonateMegaphone(); } private static boolean shouldShowLinkPreviewsMegaphone(@NonNull Context context) { diff --git a/app/src/main/java/org/thoughtcrime/securesms/mms/MediaConstraints.java b/app/src/main/java/org/thoughtcrime/securesms/mms/MediaConstraints.java index 54184ea8de..bd5b3dffd1 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/mms/MediaConstraints.java +++ b/app/src/main/java/org/thoughtcrime/securesms/mms/MediaConstraints.java @@ -5,6 +5,7 @@ import android.net.Uri; import android.os.Build; import android.util.Pair; +import androidx.annotation.IntRange; import androidx.annotation.NonNull; import org.signal.core.util.logging.Log; @@ -43,6 +44,10 @@ public abstract class MediaConstraints { public abstract int getGifMaxSize(Context context); public abstract int getVideoMaxSize(Context context); + public @IntRange(from = 0, to = 100) int getImageCompressionQualitySetting(@NonNull Context context) { + return 70; + } + public int getUncompressedVideoMaxSize(Context context) { return getVideoMaxSize(context); } diff --git a/app/src/main/java/org/thoughtcrime/securesms/mms/PushMediaConstraints.java b/app/src/main/java/org/thoughtcrime/securesms/mms/PushMediaConstraints.java index 736368e455..592bebc56e 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/mms/PushMediaConstraints.java +++ b/app/src/main/java/org/thoughtcrime/securesms/mms/PushMediaConstraints.java @@ -2,21 +2,30 @@ package org.thoughtcrime.securesms.mms; import android.content.Context; +import androidx.annotation.IntRange; +import androidx.annotation.NonNull; +import androidx.annotation.Nullable; + +import org.thoughtcrime.securesms.dependencies.ApplicationDependencies; +import org.thoughtcrime.securesms.util.LocaleFeatureFlags; import org.thoughtcrime.securesms.util.Util; +import java.util.Arrays; + public class PushMediaConstraints extends MediaConstraints { - private static final int MAX_IMAGE_DIMEN_LOWMEM = 768; - private static final int MAX_IMAGE_DIMEN = 1600; private static final int KB = 1024; private static final int MB = 1024 * KB; - private static final int[] FALLBACKS = { MAX_IMAGE_DIMEN, 1024, 768, 512 }; - private static final int[] FALLBACKS_LOWMEM = { MAX_IMAGE_DIMEN_LOWMEM, 512 }; + private final MediaConfig currentConfig; + + public PushMediaConstraints() { + currentConfig = getCurrentConfig(ApplicationDependencies.getApplication()); + } @Override public int getImageMaxWidth(Context context) { - return Util.isLowMemory(context) ? MAX_IMAGE_DIMEN_LOWMEM : MAX_IMAGE_DIMEN; + return currentConfig.imageSizeTargets[0]; } @Override @@ -26,13 +35,12 @@ public class PushMediaConstraints extends MediaConstraints { @Override public int getImageMaxSize(Context context) { - //noinspection PointlessArithmeticExpression - return 1 * MB; + return currentConfig.maxImageFileSize; } @Override public int[] getImageDimensionTargets(Context context) { - return Util.isLowMemory(context) ? FALLBACKS_LOWMEM : FALLBACKS; + return currentConfig.imageSizeTargets; } @Override @@ -66,4 +74,57 @@ public class PushMediaConstraints extends MediaConstraints { public int getDocumentMaxSize(Context context) { return 100 * MB; } + + @Override + public int getImageCompressionQualitySetting(@NonNull Context context) { + return currentConfig.qualitySetting; + } + + private static @NonNull MediaConfig getCurrentConfig(@NonNull Context context) { + if (Util.isLowMemory(context)) { + return MediaConfig.LEVEL_1_LOW_MEMORY; + } + + return LocaleFeatureFlags.getMediaQualityLevel().orElse(MediaConfig.getDefault(context)); + } + + public enum MediaConfig { + LEVEL_1_LOW_MEMORY(true, 1, MB, new int[] { 768, 512 }, 70), + + LEVEL_1(false, 1, MB, new int[] { 1600, 1024, 768, 512 }, 70), + LEVEL_2(false, 2, (int) (1.5 * MB), new int[] { 2048, 1600, 1024, 768, 512 }, 75), + LEVEL_3(false, 3, (int) (2.5 * MB), new int[] { 3072, 2048, 1600, 1024, 768, 512 }, 80); + + private final boolean isLowMemory; + private final int level; + private final int maxImageFileSize; + private final int[] imageSizeTargets; + private final int qualitySetting; + + MediaConfig(boolean isLowMemory, + int level, + int maxImageFileSize, + @NonNull int[] imageSizeTargets, + @IntRange(from = 0, to = 100) int qualitySetting) + { + this.isLowMemory = isLowMemory; + this.level = level; + this.maxImageFileSize = maxImageFileSize; + this.imageSizeTargets = imageSizeTargets; + this.qualitySetting = qualitySetting; + } + + public static @Nullable MediaConfig forLevel(int level) { + boolean isLowMemory = Util.isLowMemory(ApplicationDependencies.getApplication()); + + return Arrays.stream(values()) + .filter(v -> v.level == level && v.isLowMemory == isLowMemory) + .findFirst() + .orElse(null); + } + + public static @NonNull MediaConfig getDefault(Context context) { + return Util.isLowMemory(context) ? LEVEL_1_LOW_MEMORY : LEVEL_1; + } + } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/util/FeatureFlags.java b/app/src/main/java/org/thoughtcrime/securesms/util/FeatureFlags.java index 2a70344ea5..66b1617125 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/util/FeatureFlags.java +++ b/app/src/main/java/org/thoughtcrime/securesms/util/FeatureFlags.java @@ -4,6 +4,7 @@ import android.os.Build; import android.text.TextUtils; import androidx.annotation.NonNull; +import androidx.annotation.Nullable; import androidx.annotation.VisibleForTesting; import com.annimon.stream.Stream; @@ -77,6 +78,7 @@ public final class FeatureFlags { private static final String MESSAGE_PROCESSOR_DELAY = "android.messageProcessor.foregroundDelayMs"; private static final String NOTIFICATION_REWRITE = "android.notificationRewrite"; private static final String MP4_GIF_SEND_SUPPORT = "android.mp4GifSendSupport"; + private static final String MEDIA_QUALITY_LEVELS = "android.mediaQuality.levels"; /** * We will only store remote values for flags in this set. If you want a flag to be controllable @@ -109,7 +111,8 @@ public final class FeatureFlags { MESSAGE_PROCESSOR_ALARM_INTERVAL, MESSAGE_PROCESSOR_DELAY, NOTIFICATION_REWRITE, - MP4_GIF_SEND_SUPPORT + MP4_GIF_SEND_SUPPORT, + MEDIA_QUALITY_LEVELS ); @VisibleForTesting @@ -154,7 +157,8 @@ public final class FeatureFlags { MESSAGE_PROCESSOR_DELAY, GV1_FORCED_MIGRATE, NOTIFICATION_REWRITE, - MP4_GIF_SEND_SUPPORT + MP4_GIF_SEND_SUPPORT, + MEDIA_QUALITY_LEVELS ); /** @@ -350,6 +354,10 @@ public final class FeatureFlags { return getBoolean(MP4_GIF_SEND_SUPPORT, false); } + public static @Nullable String getMediaQualityLevels() { + return getString(MEDIA_QUALITY_LEVELS, ""); + } + /** Only for rendering debug info. */ public static synchronized @NonNull Map getMemoryValues() { return new TreeMap<>(REMOTE_VALUES); diff --git a/app/src/main/java/org/thoughtcrime/securesms/util/LocaleFeatureFlags.java b/app/src/main/java/org/thoughtcrime/securesms/util/LocaleFeatureFlags.java new file mode 100644 index 0000000000..c415741d84 --- /dev/null +++ b/app/src/main/java/org/thoughtcrime/securesms/util/LocaleFeatureFlags.java @@ -0,0 +1,99 @@ +package org.thoughtcrime.securesms.util; + +import androidx.annotation.NonNull; +import androidx.annotation.VisibleForTesting; + +import com.google.i18n.phonenumbers.NumberParseException; +import com.google.i18n.phonenumbers.PhoneNumberUtil; + +import org.signal.core.util.logging.Log; +import org.thoughtcrime.securesms.mms.PushMediaConstraints; +import org.thoughtcrime.securesms.recipients.Recipient; + +import java.util.HashMap; +import java.util.Map; +import java.util.Optional; + +/** + * Provide access to locale specific values within feature flags following the locale CSV-Colon format. + * + * Example: countryCode:integerValue,countryCode:integerValue,*:integerValue + */ +public final class LocaleFeatureFlags { + + private static final String TAG = Log.tag(LocaleFeatureFlags.class); + + private static final String COUNTRY_WILDCARD = "*"; + private static final int NOT_FOUND = -1; + + /** + * In research megaphone group for given country code + */ + public static boolean isInResearchMegaphone() { + return false; + } + + /** + * In donate megaphone group for given country code + */ + public static boolean isInDonateMegaphone() { + return isEnabled(FeatureFlags.DONATE_MEGAPHONE, FeatureFlags.donateMegaphone()); + } + + public static @NonNull Optional getMediaQualityLevel() { + Map countryValues = parseCountryValues(FeatureFlags.getMediaQualityLevels(), NOT_FOUND); + int level = getCountryValue(countryValues, Recipient.self().getE164().or(""), NOT_FOUND); + + return Optional.ofNullable(PushMediaConstraints.MediaConfig.forLevel(level)); + } + + /** + * Parses a comma-separated list of country codes colon-separated from how many buckets out of 1 million + * should be enabled to see this megaphone in that country code. At the end of the list, an optional + * element saying how many buckets out of a million should be enabled for all countries not listed previously + * in the list. For example, "1:20000,*:40000" would mean 2% of the NANPA phone numbers and 4% of the rest of + * the world should see the megaphone. + */ + private static boolean isEnabled(@NonNull String flag, @NonNull String serialized) { + Map countryCodeValues = parseCountryValues(serialized, 0); + Recipient self = Recipient.self(); + + if (countryCodeValues.isEmpty() || !self.getE164().isPresent() || !self.getUuid().isPresent()) { + return false; + } + + long countEnabled = getCountryValue(countryCodeValues, self.getE164().or(""), 0); + long currentUserBucket = BucketingUtil.bucket(flag, self.requireUuid(), 1_000_000); + + return countEnabled > currentUserBucket; + } + + @VisibleForTesting + static @NonNull Map parseCountryValues(@NonNull String buckets, int defaultValue) { + Map countryCountEnabled = new HashMap<>(); + + for (String bucket : buckets.split(",")) { + String[] parts = bucket.split(":"); + if (parts.length == 2 && !parts[0].isEmpty()) { + countryCountEnabled.put(parts[0], Util.parseInt(parts[1], defaultValue)); + } + } + return countryCountEnabled; + } + + @VisibleForTesting + static int getCountryValue(@NonNull Map countryCodeValues, @NonNull String e164, int defaultValue) { + Integer countEnabled = countryCodeValues.get(COUNTRY_WILDCARD); + try { + String countryCode = String.valueOf(PhoneNumberUtil.getInstance().parse(e164, "").getCountryCode()); + if (countryCodeValues.containsKey(countryCode)) { + countEnabled = countryCodeValues.get(countryCode); + } + } catch (NumberParseException e) { + Log.d(TAG, "Unable to determine country code for bucketing."); + return defaultValue; + } + + return countEnabled != null ? countEnabled : defaultValue; + } +} diff --git a/app/src/main/java/org/thoughtcrime/securesms/util/PopulationFeatureFlags.java b/app/src/main/java/org/thoughtcrime/securesms/util/PopulationFeatureFlags.java deleted file mode 100644 index 43d69cd1a1..0000000000 --- a/app/src/main/java/org/thoughtcrime/securesms/util/PopulationFeatureFlags.java +++ /dev/null @@ -1,84 +0,0 @@ -package org.thoughtcrime.securesms.util; - -import androidx.annotation.NonNull; -import androidx.annotation.VisibleForTesting; - -import com.google.i18n.phonenumbers.NumberParseException; -import com.google.i18n.phonenumbers.PhoneNumberUtil; - -import org.signal.core.util.logging.Log; -import org.thoughtcrime.securesms.recipients.Recipient; - -import java.util.HashMap; -import java.util.Map; - -/** - * Parses a comma-separated list of country codes colon-separated from how many buckets out of 1 million - * should be enabled to see this megaphone in that country code. At the end of the list, an optional - * element saying how many buckets out of a million should be enabled for all countries not listed previously - * in the list. For example, "1:20000,*:40000" would mean 2% of the NANPA phone numbers and 4% of the rest of - * the world should see the megaphone. - */ -public final class PopulationFeatureFlags { - - private static final String TAG = Log.tag(PopulationFeatureFlags.class); - - private static final String COUNTRY_WILDCARD = "*"; - - /** - * In research megaphone group for given country code - */ - public static boolean isInResearchMegaphone() { - return false; - } - - /** - * In donate megaphone group for given country code - */ - public static boolean isInDonateMegaphone() { - return isEnabled(FeatureFlags.DONATE_MEGAPHONE, FeatureFlags.donateMegaphone()); - } - - private static boolean isEnabled(@NonNull String flag, @NonNull String serialized) { - Map countryCountEnabled = parseCountryCounts(serialized); - Recipient self = Recipient.self(); - - if (countryCountEnabled.isEmpty() || !self.getE164().isPresent() || !self.getUuid().isPresent()) { - return false; - } - - long countEnabled = determineCountEnabled(countryCountEnabled, self.getE164().or("")); - long currentUserBucket = BucketingUtil.bucket(flag, self.requireUuid(), 1_000_000); - - return countEnabled > currentUserBucket; - } - - @VisibleForTesting - static @NonNull Map parseCountryCounts(@NonNull String buckets) { - Map countryCountEnabled = new HashMap<>(); - - for (String bucket : buckets.split(",")) { - String[] parts = bucket.split(":"); - if (parts.length == 2 && !parts[0].isEmpty()) { - countryCountEnabled.put(parts[0], Util.parseInt(parts[1], 0)); - } - } - return countryCountEnabled; - } - - @VisibleForTesting - static long determineCountEnabled(@NonNull Map countryCountEnabled, @NonNull String e164) { - Integer countEnabled = countryCountEnabled.get(COUNTRY_WILDCARD); - try { - String countryCode = String.valueOf(PhoneNumberUtil.getInstance().parse(e164, "").getCountryCode()); - if (countryCountEnabled.containsKey(countryCode)) { - countEnabled = countryCountEnabled.get(countryCode); - } - } catch (NumberParseException e) { - Log.d(TAG, "Unable to determine country code for bucketing."); - return 0; - } - - return countEnabled != null ? countEnabled : 0; - } -} diff --git a/app/src/test/java/org/thoughtcrime/securesms/util/PopulationFeatureFlagsTest_determineCountEnabled.java b/app/src/test/java/org/thoughtcrime/securesms/util/LocaleFeatureFlagsTest_getCountryValue.java similarity index 83% rename from app/src/test/java/org/thoughtcrime/securesms/util/PopulationFeatureFlagsTest_determineCountEnabled.java rename to app/src/test/java/org/thoughtcrime/securesms/util/LocaleFeatureFlagsTest_getCountryValue.java index 7158fb9a35..e8de1a767d 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/util/PopulationFeatureFlagsTest_determineCountEnabled.java +++ b/app/src/test/java/org/thoughtcrime/securesms/util/LocaleFeatureFlagsTest_getCountryValue.java @@ -17,7 +17,7 @@ import java.util.Map; import static org.junit.Assert.assertEquals; @RunWith(Parameterized.class) -public class PopulationFeatureFlagsTest_determineCountEnabled { +public class LocaleFeatureFlagsTest_getCountryValue { private final String phoneNumber; private final Map countryCounts; @@ -66,9 +66,9 @@ public class PopulationFeatureFlagsTest_determineCountEnabled { Log.initialize(new EmptyLogger()); } - public PopulationFeatureFlagsTest_determineCountEnabled(@NonNull String phoneNumber, - @NonNull Map countryCounts, - long output) + public LocaleFeatureFlagsTest_getCountryValue(@NonNull String phoneNumber, + @NonNull Map countryCounts, + long output) { this.phoneNumber = phoneNumber; this.countryCounts = countryCounts; @@ -77,7 +77,7 @@ public class PopulationFeatureFlagsTest_determineCountEnabled { @Test public void determineCountEnabled() { - assertEquals(output, PopulationFeatureFlags.determineCountEnabled(countryCounts, phoneNumber)); + assertEquals(output, LocaleFeatureFlags.getCountryValue(countryCounts, phoneNumber, 0)); } } diff --git a/app/src/test/java/org/thoughtcrime/securesms/util/PopulationFeatureFlagsTest_parseCountryCounts.java b/app/src/test/java/org/thoughtcrime/securesms/util/LocaleFeatureFlagsTest_parseCountryValues.java similarity index 85% rename from app/src/test/java/org/thoughtcrime/securesms/util/PopulationFeatureFlagsTest_parseCountryCounts.java rename to app/src/test/java/org/thoughtcrime/securesms/util/LocaleFeatureFlagsTest_parseCountryValues.java index ce82279079..e942475a4e 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/util/PopulationFeatureFlagsTest_parseCountryCounts.java +++ b/app/src/test/java/org/thoughtcrime/securesms/util/LocaleFeatureFlagsTest_parseCountryValues.java @@ -12,7 +12,7 @@ import java.util.Map; import static org.junit.Assert.assertEquals; @RunWith(Parameterized.class) -public class PopulationFeatureFlagsTest_parseCountryCounts { +public class LocaleFeatureFlagsTest_parseCountryValues { private final String input; private final Map output; @@ -46,14 +46,14 @@ public class PopulationFeatureFlagsTest_parseCountryCounts { }); } - public PopulationFeatureFlagsTest_parseCountryCounts(String input, Map output) { + public LocaleFeatureFlagsTest_parseCountryValues(String input, Map output) { this.input = input; this.output = output; } @Test public void parseCountryCounts() { - assertEquals(output, PopulationFeatureFlags.parseCountryCounts(input)); + assertEquals(output, LocaleFeatureFlags.parseCountryValues(input, 0)); } }