Apply improvements from code review

This simplifies the code by not serializing and de-serializing the tags data, and also removes the DownloadManagerService.EXTRA_SOURCE field, which is always inferred from the StreamInfo.

Author: mira-x

app/src/main/java/org/schabi/newpipe/streams/OggFromWebMWriter.java

@@ -1,11 +1,11 @@
 package org.schabi.newpipe.streams;
 
 import android.util.Log;
+import android.util.Pair;
 
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 
-import org.schabi.newpipe.BuildConfig;
 import org.schabi.newpipe.extractor.stream.StreamInfo;
 import org.schabi.newpipe.streams.WebMReader.Cluster;
 import org.schabi.newpipe.streams.WebMReader.Segment;
@@ -17,8 +17,9 @@
 import java.io.IOException;
 import java.nio.ByteBuffer;
 import java.nio.ByteOrder;
-import java.time.OffsetDateTime;
 import java.time.format.DateTimeFormatter;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.stream.Collectors;
 
 /**
@@ -284,24 +285,22 @@ private byte[] makeMetadata() {
         Log.d("OggFromWebMWriter", "Downloading media with codec ID " + webmTrack.codecId);
 
         if ("A_OPUS".equals(webmTrack.codecId)) {
-            var metadata = "";
-            metadata += String.format("COMMENT=Downloaded using NewPipe %s on %s\n",
-                    BuildConfig.VERSION_NAME,
-                    OffsetDateTime.now().toString());
+            final var metadata = new ArrayList<Pair<String, String>>();
             if (streamInfo != null) {
-                metadata += String.format("COMMENT=URL: %s\n", streamInfo.getUrl());
-                metadata += String.format("GENRE=%s\n", streamInfo.getCategory());
-                metadata += String.format("ARTIST=%s\n", streamInfo.getUploaderName());
-                metadata += String.format("TITLE=%s\n", streamInfo.getName());
-                metadata += String.format("DATE=%s\n",
-                        streamInfo
-                                .getUploadDate()
-                                .getLocalDateTime()
-                                .format(DateTimeFormatter.ISO_DATE));
+                metadata.add(Pair.create("COMMENT", streamInfo.getUrl()));
+                metadata.add(Pair.create("GENRE", streamInfo.getCategory()));
+                metadata.add(Pair.create("ARTIST", streamInfo.getUploaderName()));
+                metadata.add(Pair.create("TITLE", streamInfo.getName()));
+                metadata.add(Pair.create("DATE", streamInfo
+                        .getUploadDate()
+                        .getLocalDateTime()
+                        .format(DateTimeFormatter.ISO_DATE)));
             }
 
             Log.d("OggFromWebMWriter", "Creating metadata header with this data:");
-            Log.d("OggFromWebMWriter", metadata);
+            metadata.forEach(p -> {
+                Log.d("OggFromWebMWriter", p.first + "=" + p.second);
+            });
 
             return makeOpusTagsHeader(metadata);
         } else if ("A_VORBIS".equals(webmTrack.codecId)) {
@@ -322,17 +321,13 @@ private byte[] makeMetadata() {
      * byte string length field and includes the string as-is. This cannot be used independently,
      * but must follow a proper "OpusTags" header.
      *
-     * @param keyValue A key-value pair in the format "KEY=some value"
+     * @param pair A key-value pair in the format "KEY=some value"
      * @return The binary data of the encoded metadata tag
      */
-    private static byte[] makeOpusMetadataTag(final String keyValue) {
-        // Ensure the key is uppercase
-        final var delimiterIndex = keyValue.indexOf('=');
-        final var key = keyValue.substring(0, delimiterIndex).toUpperCase();
-        final var value = keyValue.substring(delimiterIndex + 1);
-        final var reconstructedKeyValue = key + "=" + value;
-
-        final var bytes = reconstructedKeyValue.getBytes();
+    private static byte[] makeOpusMetadataTag(final Pair<String, String> pair) {
+        final var keyValue = pair.first.toUpperCase() + "=" + pair.second.trim();
+
+        final var bytes = keyValue.getBytes();
         final var buf = ByteBuffer.allocate(4 + bytes.length);
         buf.order(ByteOrder.LITTLE_ENDIAN);
         buf.putInt(bytes.length);
@@ -341,20 +336,19 @@ private static byte[] makeOpusMetadataTag(final String keyValue) {
     }
 
     /**
-     * This returns a complete "OpusTags" header, created from the provided tags string.
+     * This returns a complete "OpusTags" header, created from the provided metadata tags.
      * <p>
      * You probably want to use makeOpusMetadata(), which uses this function to create
      * a header with sensible metadata filled in.
      *
-     * @param keyValueLines A multiline string with each line containing a key-value pair
-     *                      in the format "KEY=some value". This may also be a blank string.
+     * @param keyValueLines A list of pairs of the tags. This can also be though of as a mapping
+     *                      from one key to multiple values.
      * @return The binary header
      */
-    private static byte[] makeOpusTagsHeader(@NonNull final String keyValueLines) {
+    private static byte[] makeOpusTagsHeader(final List<Pair<String, String>> keyValueLines) {
         final var tags = keyValueLines
-                .lines()
-                .map(String::trim)
-                .filter(s -> !s.isBlank())
+                .stream()
+                .filter(p -> !p.second.isBlank())
                 .map(OggFromWebMWriter::makeOpusMetadataTag)
                 .collect(Collectors.toUnmodifiableList());
 

app/src/main/java/us/shandian/giga/service/DownloadManagerService.java

@@ -75,7 +75,6 @@ public class DownloadManagerService extends Service {
     private static final String EXTRA_THREADS = "DownloadManagerService.extra.threads";
     private static final String EXTRA_POSTPROCESSING_NAME = "DownloadManagerService.extra.postprocessingName";
     private static final String EXTRA_POSTPROCESSING_ARGS = "DownloadManagerService.extra.postprocessingArgs";
-    private static final String EXTRA_SOURCE = "DownloadManagerService.extra.source";
     private static final String EXTRA_NEAR_LENGTH = "DownloadManagerService.extra.nearLength";
     private static final String EXTRA_PATH = "DownloadManagerService.extra.storagePath";
     private static final String EXTRA_PARENT_PATH = "DownloadManagerService.extra.storageParentPath";
@@ -369,7 +368,6 @@ public static void startMission(Context context, String[] urls, StoredFileHelper
                 .putExtra(EXTRA_URLS, urls)
                 .putExtra(EXTRA_KIND, kind)
                 .putExtra(EXTRA_THREADS, threads)
-                .putExtra(EXTRA_SOURCE, streamInfo.getUrl())
                 .putExtra(EXTRA_POSTPROCESSING_NAME, psName)
                 .putExtra(EXTRA_POSTPROCESSING_ARGS, psArgs)
                 .putExtra(EXTRA_NEAR_LENGTH, nearLength)
@@ -390,7 +388,6 @@ private void startMission(Intent intent) {
         char kind = intent.getCharExtra(EXTRA_KIND, '?');
         String psName = intent.getStringExtra(EXTRA_POSTPROCESSING_NAME);
         String[] psArgs = intent.getStringArrayExtra(EXTRA_POSTPROCESSING_ARGS);
-        String source = intent.getStringExtra(EXTRA_SOURCE);
         long nearLength = intent.getLongExtra(EXTRA_NEAR_LENGTH, 0);
         String tag = intent.getStringExtra(EXTRA_STORAGE_TAG);
         StreamInfo streamInfo = (StreamInfo)intent.getSerializableExtra(EXTRA_STREAM_INFO);
@@ -413,7 +410,7 @@ private void startMission(Intent intent) {
 
         final DownloadMission mission = new DownloadMission(urls, storage, kind, ps);
         mission.threadCount = threads;
-        mission.source = source;
+        mission.source = streamInfo.getUrl();
         mission.nearLength = nearLength;
         mission.recoveryInfo = recovery.toArray(new MissionRecoveryInfo[0]);