Rewrite chapter parser for testability

7 files changed, 425 insertions, 332 deletions

diff --git a/core/src/main/java/de/danoeh/antennapod/core/util/ChapterUtils.java b/core/src/main/java/de/danoeh/antennapod/core/util/ChapterUtils.java
index ee07759d2..e76d0b024 100644
--- a/core/src/main/java/de/danoeh/antennapod/core/util/ChapterUtils.java
+++ b/core/src/main/java/de/danoeh/antennapod/core/util/ChapterUtils.java
@@ -95,14 +95,9 @@ public class ChapterUtils {
 
     @NonNull
     private static List<Chapter> readId3ChaptersFrom(CountingInputStream in) throws IOException, ID3ReaderException {
-        ChapterReader reader = new ChapterReader();
-        reader.readInputStream(in);
+        ChapterReader reader = new ChapterReader(in);
+        reader.readInputStream();
         List<Chapter> chapters = reader.getChapters();
-
-        if (chapters == null) {
-            Log.i(TAG, "ChapterReader could not find any ID3 chapters");
-            return Collections.emptyList();
-        }
         Collections.sort(chapters, new ChapterStartTimeComparator());
         enumerateEmptyChapterTitles(chapters);
         if (!chaptersValid(chapters)) {
@@ -117,10 +112,6 @@ public class ChapterUtils {
         VorbisCommentChapterReader reader = new VorbisCommentChapterReader();
         reader.readInputStream(input);
         List<Chapter> chapters = reader.getChapters();
-        if (chapters == null) {
-            Log.i(TAG, "ChapterReader could not find any Ogg vorbis chapters");
-            return Collections.emptyList();
-        }
         Collections.sort(chapters, new ChapterStartTimeComparator());
         enumerateEmptyChapterTitles(chapters);
         if (chaptersValid(chapters)) {
diff --git a/core/src/main/java/de/danoeh/antennapod/core/util/id3reader/ChapterReader.java b/core/src/main/java/de/danoeh/antennapod/core/util/id3reader/ChapterReader.java
index b3ef4d40a..69d8316c2 100644
--- a/core/src/main/java/de/danoeh/antennapod/core/util/id3reader/ChapterReader.java
+++ b/core/src/main/java/de/danoeh/antennapod/core/util/id3reader/ChapterReader.java
@@ -2,149 +2,114 @@ package de.danoeh.antennapod.core.util.id3reader;
 
 import android.text.TextUtils;
 import android.util.Log;
+import androidx.annotation.NonNull;
 import de.danoeh.antennapod.core.feed.Chapter;
 import de.danoeh.antennapod.core.feed.ID3Chapter;
 import de.danoeh.antennapod.core.util.EmbeddedChapterImage;
 import de.danoeh.antennapod.core.util.id3reader.model.FrameHeader;
-import de.danoeh.antennapod.core.util.id3reader.model.TagHeader;
+import org.apache.commons.io.input.CountingInputStream;
 
 import java.io.IOException;
 import java.net.URLDecoder;
 import java.util.ArrayList;
 import java.util.List;
-import org.apache.commons.io.input.CountingInputStream;
 
+/**
+ * Reads ID3 chapters.
+ * See https://id3.org/id3v2-chapters-1.0
+ */
 public class ChapterReader extends ID3Reader {
     private static final String TAG = "ID3ChapterReader";
 
-    private static final String FRAME_ID_CHAPTER = "CHAP";
-    private static final String FRAME_ID_TITLE = "TIT2";
-    private static final String FRAME_ID_LINK = "WXXX";
-    private static final String FRAME_ID_PICTURE = "APIC";
-    private static final int IMAGE_TYPE_COVER = 3;
+    public static final String FRAME_ID_CHAPTER = "CHAP";
+    public static final String FRAME_ID_TITLE = "TIT2";
+    public static final String FRAME_ID_LINK = "WXXX";
+    public static final String FRAME_ID_PICTURE = "APIC";
+    public static final String MIME_IMAGE_URL = "-->";
+    public static final int IMAGE_TYPE_COVER = 3;
 
-    private List<Chapter> chapters;
-    private ID3Chapter currentChapter;
+    private final List<Chapter> chapters = new ArrayList<>();
 
-    @Override
-    public int onStartTagHeader(TagHeader header) {
-        chapters = new ArrayList<>();
-        Log.d(TAG, "header: " + header);
-        return ID3Reader.ACTION_DONT_SKIP;
+    public ChapterReader(CountingInputStream input) {
+        super(input);
     }
 
     @Override
-    public int onStartFrameHeader(FrameHeader header, CountingInputStream input) throws IOException, ID3ReaderException {
-        Log.d(TAG, "header: " + header);
-        switch (header.getId()) {
-            case FRAME_ID_CHAPTER:
-                if (currentChapter != null) {
-                    if (!hasId3Chapter(currentChapter)) {
-                        chapters.add(currentChapter);
-                        Log.d(TAG, "Found chapter: " + currentChapter);
-                        currentChapter = null;
-                    }
-                }
-                StringBuilder elementId = new StringBuilder();
-                readISOString(elementId, input, Integer.MAX_VALUE);
-                char[] startTimeSource = readChars(input, 4);
-                long startTime = ((int) startTimeSource[0] << 24)
-                        | ((int) startTimeSource[1] << 16)
-                        | ((int) startTimeSource[2] << 8) | startTimeSource[3];
-                currentChapter = new ID3Chapter(elementId.toString(), startTime);
-                skipBytes(input, 12);
-                return ID3Reader.ACTION_DONT_SKIP; // Let reader discover the sub-frames
-            case FRAME_ID_TITLE:
-                if (currentChapter != null && currentChapter.getTitle() == null) {
-                    StringBuilder title = new StringBuilder();
-                    readString(title, input, header.getSize());
-                    currentChapter
-                            .setTitle(title.toString());
-                    Log.d(TAG, "Found title: " + currentChapter.getTitle());
+    protected void readFrame(@NonNull FrameHeader frameHeader) throws IOException, ID3ReaderException {
+        if (FRAME_ID_CHAPTER.equals(frameHeader.getId())) {
+            Log.d(TAG, "Handling frame: " + frameHeader.toString());
+            Chapter chapter = readChapter(frameHeader);
+            Log.d(TAG, "Chapter done: " + chapter);
+            chapters.add(chapter);
+        } else {
+            super.readFrame(frameHeader);
+        }
+    }
 
-                    return ID3Reader.ACTION_SKIP;
-                }
+    public Chapter readChapter(@NonNull FrameHeader frameHeader) throws IOException, ID3ReaderException {
+        int chapterStartedPosition = getPosition();
+        String elementId = readIsoStringNullTerminated(100);
+        long startTime = readInt();
+        skipBytes(12); // Ignore end time, start offset, end offset
+        ID3Chapter chapter = new ID3Chapter(elementId, startTime);
+
+        // Read sub-frames
+        while (getPosition() < chapterStartedPosition + frameHeader.getSize()) {
+            FrameHeader subFrameHeader = readFrameHeader();
+            readChapterSubFrame(subFrameHeader, chapter);
+        }
+        return chapter;
+    }
+
+    public void readChapterSubFrame(@NonNull FrameHeader frameHeader, @NonNull Chapter chapter)
+            throws IOException, ID3ReaderException {
+        Log.d(TAG, "Handling subframe: " + frameHeader.toString());
+        int frameStartPosition = getPosition();
+        switch (frameHeader.getId()) {
+            case FRAME_ID_TITLE:
+                chapter.setTitle(readEncodingAndString(frameHeader.getSize()));
+                Log.d(TAG, "Found title: " + chapter.getTitle());
                 break;
             case FRAME_ID_LINK:
-                if (currentChapter != null) {
-                    // skip description
-                    int descriptionLength = readString(null, input, header.getSize());
-                    StringBuilder link = new StringBuilder();
-                    readISOString(link, input, header.getSize() - descriptionLength);
-                    try {
-                        String decodedLink = URLDecoder.decode(link.toString(), "UTF-8");
-                        currentChapter.setLink(decodedLink);
-                        Log.d(TAG, "Found link: " + currentChapter.getLink());
-                    } catch (IllegalArgumentException iae) {
-                        Log.w(TAG, "Bad URL found in ID3 data");
-                    }
-
-                    return ID3Reader.ACTION_SKIP;
+                readEncodingAndString(frameHeader.getSize()); // skip description
+                String url = readIsoStringNullTerminated(frameStartPosition + frameHeader.getSize() - getPosition());
+                try {
+                    String decodedLink = URLDecoder.decode(url, "ISO-8859-1");
+                    chapter.setLink(decodedLink);
+                    Log.d(TAG, "Found link: " + chapter.getLink());
+                } catch (IllegalArgumentException iae) {
+                    Log.w(TAG, "Bad URL found in ID3 data");
                 }
                 break;
             case FRAME_ID_PICTURE:
-                if (currentChapter != null) {
-                    Log.d(TAG, header.toString());
-                    StringBuilder mime = new StringBuilder();
-                    int read = readString(mime, input, header.getSize());
-                    byte type = (byte) readChars(input, 1)[0];
-                    read++;
-                    StringBuilder description = new StringBuilder();
-                    read += readISOString(description, input, header.getSize()); // Should use same encoding as mime
-
-                    Log.d(TAG, "Found apic: " + mime + "," + description);
-                    if (mime.toString().equals("-->")) {
-                        // Data contains a link to a picture
-                        StringBuilder link = new StringBuilder();
-                        readISOString(link, input, header.getSize());
-                        Log.d(TAG, "link: " + link.toString());
-                        if (TextUtils.isEmpty(currentChapter.getImageUrl()) || type == IMAGE_TYPE_COVER) {
-                            currentChapter.setImageUrl(link.toString());
-                        }
-                    } else {
-                        // Data contains the picture
-                        int length = header.getSize() - read;
-                        if (TextUtils.isEmpty(currentChapter.getImageUrl()) || type == IMAGE_TYPE_COVER) {
-                            currentChapter.setImageUrl(EmbeddedChapterImage.makeUrl(input.getCount(), length));
-                        }
-                        skipBytes(input, length);
+                byte encoding = readByte();
+                String mime = readEncodedString(encoding, frameHeader.getSize());
+                byte type = readByte();
+                String description = readEncodedString(encoding, frameHeader.getSize());
+                Log.d(TAG, "Found apic: " + mime + "," + description);
+                if (MIME_IMAGE_URL.equals(mime)) {
+                    String link = readIsoStringNullTerminated(frameHeader.getSize());
+                    Log.d(TAG, "Link: " + link);
+                    if (TextUtils.isEmpty(chapter.getImageUrl()) || type == IMAGE_TYPE_COVER) {
+                        chapter.setImageUrl(link);
+                    }
+                } else {
+                    int alreadyConsumed = getPosition() - frameStartPosition;
+                    int rawImageDataLength = frameHeader.getSize() - alreadyConsumed;
+                    if (TextUtils.isEmpty(chapter.getImageUrl()) || type == IMAGE_TYPE_COVER) {
+                        chapter.setImageUrl(EmbeddedChapterImage.makeUrl(getPosition(), rawImageDataLength));
                     }
-                    return ID3Reader.ACTION_SKIP;
                 }
                 break;
+            default:
+                Log.d(TAG, "Unknown chapter sub-frame.");
+                break;
         }
 
-        return super.onStartFrameHeader(header, input);
-    }
-
-    private boolean hasId3Chapter(ID3Chapter chapter) {
-        for (Chapter c : chapters) {
-            if (((ID3Chapter) c).getId3ID().equals(chapter.getId3ID())) {
-                return true;
-            }
-        }
-        return false;
-    }
-
-    @Override
-    public void onEndTag() {
-        if (currentChapter != null) {
-            if (!hasId3Chapter(currentChapter)) {
-                chapters.add(currentChapter);
-            }
-        }
-        Log.d(TAG, "Reached end of tag");
-        if (chapters != null) {
-            for (Chapter c : chapters) {
-                Log.d(TAG, "chapter: " + c);
-            }
-        }
-    }
-
-    @Override
-    public void onNoTagHeaderFound() {
-        Log.d(TAG, "No tag header found");
-        super.onNoTagHeaderFound();
+        // Skip garbage to fill frame completely
+        // This also asserts that we are not reading too many bytes from this frame.
+        int alreadyConsumed = getPosition() - frameStartPosition;
+        skipBytes(frameHeader.getSize() - alreadyConsumed);
     }
 
     public List<Chapter> getChapters() {
diff --git a/core/src/main/java/de/danoeh/antennapod/core/util/id3reader/ID3Reader.java b/core/src/main/java/de/danoeh/antennapod/core/util/id3reader/ID3Reader.java
index 155376d85..18cbb1dbf 100644
--- a/core/src/main/java/de/danoeh/antennapod/core/util/id3reader/ID3Reader.java
+++ b/core/src/main/java/de/danoeh/antennapod/core/util/id3reader/ID3Reader.java
@@ -1,163 +1,112 @@
 package de.danoeh.antennapod.core.util.id3reader;
 
+import android.util.Log;
+import androidx.annotation.NonNull;
+import de.danoeh.antennapod.core.util.id3reader.model.FrameHeader;
+import de.danoeh.antennapod.core.util.id3reader.model.TagHeader;
 import org.apache.commons.io.IOUtils;
+import org.apache.commons.io.input.CountingInputStream;
 
+import java.io.ByteArrayOutputStream;
 import java.io.IOException;
-import java.io.InputStream;
 import java.nio.ByteBuffer;
 import java.nio.charset.Charset;
 
-import de.danoeh.antennapod.core.util.id3reader.model.FrameHeader;
-import de.danoeh.antennapod.core.util.id3reader.model.TagHeader;
-import org.apache.commons.io.input.CountingInputStream;
-
 /**
- * Reads the ID3 Tag of a given file. In order to use this class, you should
- * create a subclass of it and overwrite the onStart* - or onEnd* - methods.
+ * Reads the ID3 Tag of a given file.
+ * See https://id3.org/id3v2.3.0
  */
 public class ID3Reader {
-    private static final int HEADER_LENGTH = 10;
-    private static final int ID3_LENGTH = 3;
+    private static final String TAG = "ID3Reader";
     private static final int FRAME_ID_LENGTH = 4;
-
-    /**
-     * Should skip remaining bytes of the current frame.
-     */
-    static final int ACTION_SKIP = 1;
-
-    /**
-     * Should not skip remaining bytes of the current frame. Can be used to parse sub-frames.
-     */
-    static final int ACTION_DONT_SKIP = 2;
-
-    private int readerPosition;
-
-    private static final byte ENCODING_UTF16_WITH_BOM = 1;
-    private static final byte ENCODING_UTF16_WITHOUT_BOM = 2;
-    private static final byte ENCODING_UTF8 = 3;
+    public static final byte ENCODING_ISO = 0;
+    public static final byte ENCODING_UTF16_WITH_BOM = 1;
+    public static final byte ENCODING_UTF16_WITHOUT_BOM = 2;
+    public static final byte ENCODING_UTF8 = 3;
 
     private TagHeader tagHeader;
+    private final CountingInputStream inputStream;
 
-    ID3Reader() {
+    public ID3Reader(CountingInputStream input) {
+        inputStream = input;
     }
 
-    public final void readInputStream(CountingInputStream input) throws IOException, ID3ReaderException {
-        int rc;
-        readerPosition = 0;
-        char[] tagHeaderSource = readChars(input, HEADER_LENGTH);
-        tagHeader = createTagHeader(tagHeaderSource);
-        if (tagHeader == null) {
-            onNoTagHeaderFound();
-        } else {
-            rc = onStartTagHeader(tagHeader);
-            if (rc != ACTION_SKIP) {
-                while (readerPosition < tagHeader.getSize()) {
-                    FrameHeader frameHeader = createFrameHeader(readChars(input, HEADER_LENGTH));
-                    if (checkForNullString(frameHeader.getId())) {
-                        break;
-                    }
-                    int readerPositionBeforeFrame = input.getCount();
-                    rc = onStartFrameHeader(frameHeader, input);
-                    if (rc == ACTION_SKIP) {
-                        if (frameHeader.getSize() + readerPosition > tagHeader.getSize()) {
-                            break;
-                        }
-                        int bytesAlreadyHandled = input.getCount() - readerPositionBeforeFrame;
-                        int bytesLeftToSkip = frameHeader.getSize() - bytesAlreadyHandled;
-                        if (bytesLeftToSkip > 0) {
-                            skipBytes(input, bytesLeftToSkip);
-                        }
-                    }
-                }
+    public void readInputStream() throws IOException, ID3ReaderException {
+        tagHeader = readTagHeader();
+        int tagContentStartPosition = getPosition();
+        while (getPosition() < tagContentStartPosition + tagHeader.getSize()) {
+            FrameHeader frameHeader = readFrameHeader();
+            if (frameHeader.getId().charAt(0) < '0' || frameHeader.getId().charAt(0) > 'z') {
+                Log.d(TAG, "Stopping because of invalid frame: " + frameHeader.toString());
+                return;
             }
-            onEndTag();
+            readFrame(frameHeader);
         }
     }
 
-    /** Returns true if string only contains null-bytes. */
-    private boolean checkForNullString(String s) {
-        if (!s.isEmpty()) {
-            int i = 0;
-            if (s.charAt(i) == 0) {
-                for (i = 1; i < s.length(); i++) {
-                    if (s.charAt(i) != 0) {
-                        return false;
-                    }
-                }
-                return true;
-            }
-            return false;
-        } else {
-            return true;
-        }
+    protected void readFrame(@NonNull FrameHeader frameHeader) throws IOException, ID3ReaderException {
+        Log.d(TAG, "Skipping frame: " + frameHeader.toString());
+        skipBytes(frameHeader.getSize());
+    }
 
+    int getPosition() {
+        return inputStream.getCount();
     }
 
     /**
-     * Read a certain number of chars from the given input stream. This method
-     * changes the readerPosition-attribute.
+     * Skip a certain number of bytes on the given input stream.
      */
-    char[] readChars(InputStream input, int number) throws IOException, ID3ReaderException {
-        char[] header = new char[number];
-        for (int i = 0; i < number; i++) {
-            int b = input.read();
-            readerPosition++;
-            if (b != -1) {
-                header[i] = (char) b;
-            } else {
-                throw new ID3ReaderException("Unexpected end of stream");
-            }
+    void skipBytes(int number) throws IOException, ID3ReaderException {
+        if (number < 0) {
+            throw new ID3ReaderException("Trying to read a negative number of bytes");
         }
-        return header;
+        IOUtils.skipFully(inputStream, number);
     }
 
-    /**
-     * Skip a certain number of bytes on the given input stream. This method
-     * changes the readerPosition-attribute.
-     */
-    void skipBytes(InputStream input, int number) throws IOException {
-        if (number <= 0) {
-            number = 1;
-        }
-        IOUtils.skipFully(input, number);
+    byte readByte() throws IOException {
+        return (byte) inputStream.read();
+    }
 
-        readerPosition += number;
+    short readShort() throws IOException {
+        char firstByte = (char) inputStream.read();
+        char secondByte = (char) inputStream.read();
+        return (short) ((firstByte << 8) | secondByte);
     }
 
-    private TagHeader createTagHeader(char[] source) throws ID3ReaderException {
-        boolean hasTag = (source[0] == 0x49) && (source[1] == 0x44)
-                && (source[2] == 0x33);
-        if (source.length != HEADER_LENGTH) {
-            throw new ID3ReaderException("Length of header must be "
-                    + HEADER_LENGTH);
-        }
-        if (hasTag) {
-            String id = new String(source, 0, ID3_LENGTH);
-            char version = (char) ((source[3] << 8) | source[4]);
-            byte flags = (byte) source[5];
-            int size = (source[6] << 24) | (source[7] << 16) | (source[8] << 8)
-                    | source[9];
-            size = unsynchsafe(size);
-            return new TagHeader(id, size, version, flags);
-        } else {
-            return null;
-        }
+    int readInt() throws IOException {
+        char firstByte = (char) inputStream.read();
+        char secondByte = (char) inputStream.read();
+        char thirdByte = (char) inputStream.read();
+        char fourthByte = (char) inputStream.read();
+        return (firstByte << 24) | (secondByte << 16) | (thirdByte << 8) | fourthByte;
     }
 
-    private FrameHeader createFrameHeader(char[] source)
-            throws ID3ReaderException {
-        if (source.length != HEADER_LENGTH) {
-            throw new ID3ReaderException("Length of header must be "
-                    + HEADER_LENGTH);
+    void expectChar(char expected) throws ID3ReaderException, IOException {
+        char read = (char) inputStream.read();
+        if (read != expected) {
+            throw new ID3ReaderException("Expected " + expected + " and got " + read);
         }
-        String id = new String(source, 0, FRAME_ID_LENGTH);
+    }
 
-        int size = (((int) source[4]) << 24) | (((int) source[5]) << 16)
-                | (((int) source[6]) << 8) | source[7];
+    @NonNull
+    TagHeader readTagHeader() throws ID3ReaderException, IOException {
+        expectChar('I');
+        expectChar('D');
+        expectChar('3');
+        short version = readShort();
+        byte flags = readByte();
+        int size = unsynchsafe(readInt());
+        return new TagHeader("ID3", size, version, flags);
+    }
+
+    @NonNull
+    FrameHeader readFrameHeader() throws IOException {
+        String id = readIsoStringFixed(FRAME_ID_LENGTH);
+        int size = readInt();
         if (tagHeader != null && tagHeader.getVersion() >= 0x0400) {
             size = unsynchsafe(size);
         }
-        char flags = (char) ((source[8] << 8) | source[9]);
+        short flags = readShort();
         return new FrameHeader(id, size, flags);
     }
 
@@ -174,81 +123,74 @@ public class ID3Reader {
         return out;
     }
 
-    protected int readString(StringBuilder buffer, InputStream input, int max) throws IOException,
-            ID3ReaderException {
-        if (max > 0) {
-            char[] encoding = readChars(input, 1);
-            max--;
-
-            if (encoding[0] == ENCODING_UTF16_WITH_BOM || encoding[0] == ENCODING_UTF16_WITHOUT_BOM) {
-                return readUnicodeString(buffer, input, max, Charset.forName("UTF-16")) + 1; // take encoding byte into account
-            } else if (encoding[0] == ENCODING_UTF8) {
-                return readUnicodeString(buffer, input, max, Charset.forName("UTF-8")) + 1; // take encoding byte into account
-            } else {
-                return readISOString(buffer, input, max) + 1; // take encoding byte into account
-            }
-        } else {
-            if (buffer != null) {
-                buffer.append("");
-            }
-            return 0;
-        }
+    /**
+     * Reads a null-terminated string with encoding.
+     */
+    protected String readEncodingAndString(int max) throws IOException {
+        byte encoding = readByte();
+        return readEncodedString(encoding, max - 1);
     }
 
-    protected int readISOString(StringBuilder buffer, InputStream input, int max)
-            throws IOException, ID3ReaderException {
+    @SuppressWarnings("CharsetObjectCanBeUsed")
+    protected String readIsoStringFixed(int length) throws IOException {
+        ByteArrayOutputStream bytes = new ByteArrayOutputStream();
         int bytesRead = 0;
-        char c;
-        while (++bytesRead <= max && (c = (char) input.read()) > 0) {
-            if (buffer != null) {
-                buffer.append(c);
-            }
-        }
-        return bytesRead;
-    }
-
-    private int readUnicodeString(StringBuilder strBuffer, InputStream input, int max, Charset charset)
-            throws IOException, ID3ReaderException {
-        byte[] buffer = new byte[max];
-        int c;
-        int cZero = -1;
-        int i = 0;
-        for (; i < max; i++) {
-            c = input.read();
-            if (c == -1) {
-                break;
-            } else if (c == 0) {
-                if (cZero == 0) {
-                    // termination character found
-                    break;
-                } else {
-                    cZero = 0;
-                }
-            } else {
-                buffer[i] = (byte) c;
-                cZero = -1;
-            }
+        while (bytesRead < length) {
+            bytes.write(readByte());
+            bytesRead++;
         }
-        if (strBuffer != null) {
-            strBuffer.append(charset.newDecoder().decode(ByteBuffer.wrap(buffer)).toString());
-        }
-        return i;
+        return Charset.forName("ISO-8859-1").newDecoder().decode(ByteBuffer.wrap(bytes.toByteArray())).toString();
     }
 
-    int onStartTagHeader(TagHeader header) {
-        return ACTION_SKIP;
+    protected String readIsoStringNullTerminated(int max) throws IOException {
+        return readEncodedString(ENCODING_ISO, max);
     }
 
-    int onStartFrameHeader(FrameHeader header, CountingInputStream input) throws IOException, ID3ReaderException {
-        return ACTION_SKIP;
+    @SuppressWarnings("CharsetObjectCanBeUsed")
+    String readEncodedString(int encoding, int max) throws IOException {
+        if (encoding == ENCODING_UTF16_WITH_BOM || encoding == ENCODING_UTF16_WITHOUT_BOM) {
+            return readEncodedString2(Charset.forName("UTF-16"), max);
+        } else if (encoding == ENCODING_UTF8) {
+            return readEncodedString2(Charset.forName("UTF-8"), max);
+        } else {
+            return readEncodedString1(Charset.forName("ISO-8859-1"), max);
+        }
     }
 
-    void onEndTag() {
-
+    /**
+     * Reads chars where the encoding uses 1 char per symbol.
+     */
+    private String readEncodedString1(Charset charset, int max) throws IOException {
+        ByteArrayOutputStream bytes = new ByteArrayOutputStream();
+        int bytesRead = 0;
+        while (bytesRead < max) {
+            byte c = readByte();
+            bytesRead++;
+            if (c == 0) {
+                break;
+            }
+            bytes.write(c);
+        }
+        return charset.newDecoder().decode(ByteBuffer.wrap(bytes.toByteArray())).toString();
     }
 
-    void onNoTagHeaderFound() {
-
+    /**
+     * Reads chars where the encoding uses 2 chars per symbol.
+     */
+    private String readEncodedString2(Charset charset, int max) throws IOException {
+        ByteArrayOutputStream bytes = new ByteArrayOutputStream();
+        int bytesRead = 0;
+        while (bytesRead < max) {
+            byte c1 = readByte();
+            bytesRead++;
+            if (c1 == 0) {
+                break;
+            }
+            byte c2 = readByte();
+            bytesRead++;
+            bytes.write(c1);
+            bytes.write(c2);
+        }
+        return charset.newDecoder().decode(ByteBuffer.wrap(bytes.toByteArray())).toString();
     }
-
 }
diff --git a/core/src/main/java/de/danoeh/antennapod/core/util/id3reader/model/FrameHeader.java b/core/src/main/java/de/danoeh/antennapod/core/util/id3reader/model/FrameHeader.java
index 3823d1427..e4af89a86 100644
--- a/core/src/main/java/de/danoeh/antennapod/core/util/id3reader/model/FrameHeader.java
+++ b/core/src/main/java/de/danoeh/antennapod/core/util/id3reader/model/FrameHeader.java
@@ -3,10 +3,9 @@ package de.danoeh.antennapod.core.util.id3reader.model;
 import androidx.annotation.NonNull;
 
 public class FrameHeader extends Header {
+    private final short flags;
 
-    private final char flags;
-
-    public FrameHeader(String id, int size, char flags) {
+    public FrameHeader(String id, int size, short flags) {
         super(id, size);
         this.flags = flags;
     }
diff --git a/core/src/main/java/de/danoeh/antennapod/core/util/id3reader/model/TagHeader.java b/core/src/main/java/de/danoeh/antennapod/core/util/id3reader/model/TagHeader.java
index b652a139c..2590db029 100644
--- a/core/src/main/java/de/danoeh/antennapod/core/util/id3reader/model/TagHeader.java
+++ b/core/src/main/java/de/danoeh/antennapod/core/util/id3reader/model/TagHeader.java
@@ -1,26 +1,25 @@
 package de.danoeh.antennapod.core.util.id3reader.model;
 
-public class TagHeader extends Header {
-
-	private final char version;
-	private final byte flags;
+import androidx.annotation.NonNull;
 
-	public TagHeader(String id, int size, char version, byte flags) {
-		super(id, size);
-		this.version = version;
-		this.flags = flags;
-	}
-
-	@Override
-	public String toString() {
-		return "TagHeader [version=" + version + ", flags=" + flags + ", id="
-				+ id + ", size=" + size + "]";
-	}
+public class TagHeader extends Header {
+    private final short version;
+    private final byte flags;
 
-	public char getVersion() {
-		return version;
-	}
+    public TagHeader(String id, int size, short version, byte flags) {
+        super(id, size);
+        this.version = version;
+        this.flags = flags;
+    }
 
-	
+    @Override
+    @NonNull
+    public String toString() {
+        return "TagHeader [version=" + version + ", flags=" + flags + ", id="
+                + id + ", size=" + size + "]";
+    }
 
+    public short getVersion() {
+        return version;
+    }
 }
diff --git a/core/src/test/java/de/danoeh/antennapod/core/util/id3reader/ChapterReaderTest.java b/core/src/test/java/de/danoeh/antennapod/core/util/id3reader/ChapterReaderTest.java
new file mode 100644
index 000000000..34580146e
--- /dev/null
+++ b/core/src/test/java/de/danoeh/antennapod/core/util/id3reader/ChapterReaderTest.java
@@ -0,0 +1,105 @@
+package de.danoeh.antennapod.core.util.id3reader;
+
+import de.danoeh.antennapod.core.feed.Chapter;
+import de.danoeh.antennapod.core.feed.ID3Chapter;
+import de.danoeh.antennapod.core.util.id3reader.model.FrameHeader;
+import org.apache.commons.io.input.CountingInputStream;
+import org.junit.Test;
+
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+
+import static de.danoeh.antennapod.core.util.id3reader.Id3ReaderTest.concat;
+import static de.danoeh.antennapod.core.util.id3reader.Id3ReaderTest.generateFrameHeader;
+import static de.danoeh.antennapod.core.util.id3reader.Id3ReaderTest.generateId3Header;
+import static org.junit.Assert.assertEquals;
+
+public class ChapterReaderTest {
+    private static final byte CHAPTER_WITHOUT_SUBFRAME_START_TIME = 23;
+    private static final byte[] CHAPTER_WITHOUT_SUBFRAME = {
+            'C', 'H', '1', 0, // String ID for mapping to CTOC
+            0, 0, 0, CHAPTER_WITHOUT_SUBFRAME_START_TIME, // Start time
+            0, 0, 0, 0, // End time
+            0, 0, 0, 0, // Start offset
+            0, 0, 0, 0 // End offset
+    };
+
+    @Test
+    public void testReadFullTagWithChapter() throws IOException, ID3ReaderException {
+        byte[] chapter = concat(
+                generateFrameHeader(ChapterReader.FRAME_ID_CHAPTER, CHAPTER_WITHOUT_SUBFRAME.length),
+                CHAPTER_WITHOUT_SUBFRAME);
+        byte[] data = concat(
+                generateId3Header(chapter.length),
+                chapter);
+        CountingInputStream inputStream = new CountingInputStream(new ByteArrayInputStream(data));
+        ChapterReader reader = new ChapterReader(inputStream);
+        reader.readInputStream();
+        assertEquals(1, reader.getChapters().size());
+        assertEquals(CHAPTER_WITHOUT_SUBFRAME_START_TIME, reader.getChapters().get(0).getStart());
+    }
+
+    @Test
+    public void testReadFullTagWithMultipleChapters() throws IOException, ID3ReaderException {
+        byte[] chapter = concat(
+                generateFrameHeader(ChapterReader.FRAME_ID_CHAPTER, CHAPTER_WITHOUT_SUBFRAME.length),
+                CHAPTER_WITHOUT_SUBFRAME);
+        byte[] data = concat(
+                generateId3Header(2 * chapter.length),
+                chapter,
+                chapter);
+        CountingInputStream inputStream = new CountingInputStream(new ByteArrayInputStream(data));
+        ChapterReader reader = new ChapterReader(inputStream);
+        reader.readInputStream();
+        assertEquals(2, reader.getChapters().size());
+        assertEquals(CHAPTER_WITHOUT_SUBFRAME_START_TIME, reader.getChapters().get(0).getStart());
+        assertEquals(CHAPTER_WITHOUT_SUBFRAME_START_TIME, reader.getChapters().get(1).getStart());
+    }
+
+    @Test
+    public void testReadChapterWithoutSubframes() throws IOException, ID3ReaderException {
+        FrameHeader header = new FrameHeader(ChapterReader.FRAME_ID_CHAPTER,
+                CHAPTER_WITHOUT_SUBFRAME.length, (short) 0);
+        CountingInputStream inputStream = new CountingInputStream(new ByteArrayInputStream(CHAPTER_WITHOUT_SUBFRAME));
+        Chapter chapter = new ChapterReader(inputStream).readChapter(header);
+        assertEquals(CHAPTER_WITHOUT_SUBFRAME_START_TIME, chapter.getStart());
+    }
+
+    @Test
+    public void testReadChapterWithTitle() throws IOException, ID3ReaderException {
+        byte[] title = {
+            ID3Reader.ENCODING_ISO,
+            'H', 'e', 'l', 'l', 'o', // Title
+            0 // Null-terminated
+        };
+        byte[] chapterData = concat(
+            CHAPTER_WITHOUT_SUBFRAME,
+            generateFrameHeader(ChapterReader.FRAME_ID_TITLE, title.length),
+            title);
+        FrameHeader header = new FrameHeader(ChapterReader.FRAME_ID_CHAPTER, chapterData.length, (short) 0);
+        CountingInputStream inputStream = new CountingInputStream(new ByteArrayInputStream(chapterData));
+        ChapterReader reader = new ChapterReader(inputStream);
+        Chapter chapter = reader.readChapter(header);
+        assertEquals(CHAPTER_WITHOUT_SUBFRAME_START_TIME, chapter.getStart());
+        assertEquals("Hello", chapter.getTitle());
+    }
+
+    @Test
+    public void testReadTitleWithGarbage() throws IOException, ID3ReaderException {
+        byte[] titleSubframeContent = {
+                ID3Reader.ENCODING_ISO,
+                'A', // Title
+                0, // Null-terminated
+                42, 42, 42, 42 // Garbage, should be ignored
+        };
+        FrameHeader header = new FrameHeader(ChapterReader.FRAME_ID_TITLE, titleSubframeContent.length, (short) 0);
+        CountingInputStream inputStream = new CountingInputStream(new ByteArrayInputStream(titleSubframeContent));
+        ChapterReader reader = new ChapterReader(inputStream);
+        Chapter chapter = new ID3Chapter("", 0);
+        reader.readChapterSubFrame(header, chapter);
+        assertEquals("A", chapter.getTitle());
+
+        // Should skip the garbage and point to the next frame
+        assertEquals(titleSubframeContent.length, reader.getPosition());
+    }
+}
diff --git a/core/src/test/java/de/danoeh/antennapod/core/util/id3reader/Id3ReaderTest.java b/core/src/test/java/de/danoeh/antennapod/core/util/id3reader/Id3ReaderTest.java
new file mode 100644
index 000000000..53e338416
--- /dev/null
+++ b/core/src/test/java/de/danoeh/antennapod/core/util/id3reader/Id3ReaderTest.java
@@ -0,0 +1,92 @@
+package de.danoeh.antennapod.core.util.id3reader;
+
+import de.danoeh.antennapod.core.util.id3reader.model.FrameHeader;
+import de.danoeh.antennapod.core.util.id3reader.model.TagHeader;
+import org.apache.commons.io.input.CountingInputStream;
+import org.junit.Test;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+public class Id3ReaderTest {
+    @Test
+    public void testReadString() throws IOException {
+        byte[] data = {
+            ID3Reader.ENCODING_ISO,
+            'T', 'e', 's', 't',
+            0 // Null-terminated
+        };
+        CountingInputStream inputStream = new CountingInputStream(new ByteArrayInputStream(data));
+        String string = new ID3Reader(inputStream).readEncodingAndString(1000);
+        assertEquals("Test", string);
+    }
+
+    @Test
+    public void testReadUtf16WithBom() throws IOException {
+        byte[] data = {
+            ID3Reader.ENCODING_UTF16_WITH_BOM,
+            (byte) 0xff, (byte) 0xfe, // BOM
+            'A', 0, 'B', 0, 'C', 0,
+            0, 0, // Null-terminated
+        };
+        CountingInputStream inputStream = new CountingInputStream(new ByteArrayInputStream(data));
+        String string = new ID3Reader(inputStream).readEncodingAndString(1000);
+        assertEquals("ABC", string);
+    }
+
+    @Test
+    public void testReadTagHeader() throws IOException, ID3ReaderException {
+        byte[] data = generateId3Header(23);
+        CountingInputStream inputStream = new CountingInputStream(new ByteArrayInputStream(data));
+        TagHeader header = new ID3Reader(inputStream).readTagHeader();
+        assertEquals("ID3", header.getId());
+        assertEquals(42, header.getVersion());
+        assertEquals(23, header.getSize());
+    }
+
+    @Test
+    public void testReadFrameHeader() throws IOException {
+        byte[] data = generateFrameHeader("CHAP", 42);
+        CountingInputStream inputStream = new CountingInputStream(new ByteArrayInputStream(data));
+        FrameHeader header = new ID3Reader(inputStream).readFrameHeader();
+        assertEquals("CHAP", header.getId());
+        assertEquals(42, header.getSize());
+    }
+
+    public static byte[] generateFrameHeader(String id, int size) {
+        return concat(
+            id.getBytes(StandardCharsets.ISO_8859_1), // Frame ID
+            new byte[] {
+                (byte) (size >> 24), (byte) (size >> 16),
+                (byte) (size >> 8), (byte) (size), // Size
+                0, 0 // Flags
+            });
+    }
+
+    static byte[] generateId3Header(int size) {
+        return new byte[] {
+                'I', 'D', '3', // Identifier
+                0, 42, // Version
+                0, // Flags
+                (byte) (size >> 24), (byte) (size >> 16),
+                (byte) (size >> 8), (byte) (size), // Size
+        };
+    }
+
+    static byte[] concat(byte[]... arrays) {
+        ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
+        try {
+            for (byte[] array : arrays) {
+                outputStream.write(array);
+            }
+        } catch (IOException e) {
+            fail(e.getMessage());
+        }
+        return outputStream.toByteArray();
+    }
+}