Chapter and ctoc frames

[NCrusher74]

I think I may have written myself into a corner here with regards to how I'm handling subframes.

I've got this subscript here, which creates a subframe "tag" that enables us to handle subframes as a tag and thus use all the frame getter-setters from Tag on the subframes. It's associated with a parent frame by using the parent frame's elementID.

    /** creates a "subframe tag" instance to use when accessing data within the embedded subframes of a `CHAP` or `CTOC` frame */
    subscript(embeddedSubframes parentFrameElementID: String) -> Tag? {
        get {
            var frames: [FrameKey:Frame] = [:]
            if let parentFrame = self[chapters: parentFrameElementID] {
                frames = parentFrame.embeddedSubframes
            } else if let parentFrame = self[tableOfContents: parentFrameElementID] {
                frames = parentFrame.embeddedSubframes
            } else {
                return nil
            }
            let subframeTag = Tag(readFromEmbeddedSubframes: frames)
            return subframeTag
        }
    }

But I'm not quite sure how I should handle things at this point of creating the parent frame:

        var tag = try tagNoMeta()

        tag[tableOfContents: "ctoc"]?.topLevelFlag = true
        tag[tableOfContents: "ctoc"]?.orderedFlag = true
        tag[tableOfContents: "ctoc"]?.childElementIDs = ["ch01", "ch02", "ch03"]
        var tocSubframeTag = tag[embeddedSubframes: "ctoc"]
        tocSubframeTag?.title = "Table Of Contents"
        // need to tie this back into tag somehow

I don't think(?) I can do it as a setter for the embeddedSubframes subscript, because I won't know what KIND of frame I'm dealing with. Could be a string frame, could be an image, who knows? But somehow I have to fold tocSubframeTag back into tag before I can write tag to a file, right?

[NCrusher74]

I... may have figured it out? Still needs testing, but if I turn all the embeddedSubframes: [FrameKey: Frame] properties into embeddedSubframesTag: Tag? properties, perhaps I could handle it like this?

    /** creates a "subframe tag" instance to use when accessing data within the embedded subframes of a `CHAP` or `CTOC` frame */
    public subscript(embeddedSubframes parentFrameElementID: String) -> Tag? {
        get {
            var tag = Tag(readFromEmbeddedSubframes: [:])
            if let parentFrame = self[chapters: parentFrameElementID] {
                tag = parentFrame.embeddedSubframesTag ?? Tag(readFromEmbeddedSubframes: [:])
            } else if let parentFrame = self[tableOfContents: parentFrameElementID] {
                tag = parentFrame.embeddedSubframesTag ?? Tag(readFromEmbeddedSubframes: [:])
            } else {
                return nil
            }
            let subframeTag = tag
            return subframeTag
        }
        set {
            if let new = newValue {
                if var parentFrame = self[chapters: parentFrameElementID] {
                    parentFrame.embeddedSubframesTag = new
                } else if var parentFrame = self[tableOfContents: parentFrameElementID] {
                    parentFrame.embeddedSubframesTag = new
                }
            }
        }
    }

That way, I wouldn't need to specify what frames we're working with, right?

[SDGGiesbrecht]

Well, it works in theory. Except it doesn't work at all.

Is that because neither if statement in the setter has an else statement? It would currently do nothing if the ID didn’t match any existing chapter or table of contents (the inner if didn’t match), or if you tried to set the entire subframe tag to nil (the outer if didn’t match).

[NCrusher74]

While I'm sure that's probably one reason why it wouldn't work, I don't think it's causing the issue I'm having at present, which is that the frame isn't writing at all. When I try to write a TableOfContents frame to a file, nothing writes. The file is fine, other frames will write, but the TOC won't. (I haven't worked on the chapter frame much yet, but last time I tested it a week or so ago, it wasn't writing either.)

[SDGGiesbrecht]

If there were no pre‐existing frame, then it would not contain the relevant ID either, so the inner if wouldn’t match anything, and the setter would do nothing. Thus nothing would be actually added, and there would still be no frame. Or do I misunderstand something about how the subscripts in those conditions work?

[NCrusher74]

I'm...not sure. I'm a little turned around because I havent really messed with this frame for a few weeks and apparently that's all it takes for me to forget what I was trying to do with it.

TableOfContents writes if I do it this way:

    internal mutating func set(_ layout: FrameLayoutIdentifier,
                               _ frameKey: FrameKey,
                               elementID: String,
                               topLevelFlag: Bool,
                               orderedFlag: Bool,
                               childElementIDs: [String],
                               embeddedSubframeTag: Tag?) {
        let key = FrameKey.tableOfContents(elementID: elementID)
        self.frames[key] = Frame.tocFrame(
                .init(.known(.tableOfContents),
                      elementID: elementID,
                      topLevelFlag: topLevelFlag,
                      orderedFlag: orderedFlag,
                      childElementIDs: childElementIDs,
                      embeddedSubframesTag: embeddedSubframeTag))
    }

    /// TableOfContents frame getter-setter. Valid for tag versions 2.3 and 2.4 only.
    /// ID3 Identifier `CTOC`
    public subscript(tableOfContents elementID: String) -> TableOfContentsFrame? {
        get {
            if let frame = self.frames[.tableOfContents(elementID: elementID)],
                case .tocFrame(let tocFrame) = frame {
                return tocFrame
            } else {
                return nil
            }
        }
        set {
            set(.known(.tableOfContents),
                .tableOfContents(elementID: elementID),
                elementID: elementID,
                topLevelFlag: newValue?.topLevelFlag ?? true,
                orderedFlag: newValue?.orderedFlag ?? true,
                childElementIDs: newValue?.childElementIDs ?? [],
                embeddedSubframeTag: newValue?.embeddedSubframesTag)
        }
    }

But when I wrote it using this:

    func testWritingTableOfContentsOnBlankFile() throws {
        var tag = try tagNoMeta()

        tag[tableOfContents: "ctoc"]?.topLevelFlag = true
        tag[tableOfContents: "ctoc"]?.orderedFlag = true
        tag[tableOfContents: "ctoc"]?.childElementIDs = ["ch01", "ch02", "ch03"]

        var subframeTag = tag[subframeTag: "ctoc"]
        subframeTag.title = "Table Of Contents"
        tag[tableOfContents: "ctoc"]?.embeddedSubframesTag = subframeTag

        let outputUrl = try localDirectory(fileName: "ctoctest", fileExtension: "mp3")
        XCTAssertNoThrow(try mp3NoMeta().write(tagVersion: .v2_4,
                                               using: tag,
                                               writingTo: outputUrl))

the childElementIDs array would start with the childElementIDs I'd set (with the exception of the fact that the first letter of the first one would be missing) then there would be several empty strings, then the string "Table Of Contents" and then a whole bunch more empty strings in the array. And there would be nothing in the embedded subframes, which is where the "Table Of Contents" Title subframe should have been.

I assume the first letter of the first child element ID was being cut off the beginning because I was encoding the strings when there's no encoding for the TableOfContents frame. So I fixed that.

The rest I assume was happening because it was somehow folding all the frames from the mostly-empty Tag instance into the child element IDs array. I'm...not sure why, but I created a empty initializer in Tag to replace the readFromEmbeddedSubframes initializer as the public facing initializer (because I can't make that initializer public without also making the FrameKey and Frame enums public, and I can't make the Frame enum public without making ALL the individual frame types public.)

So now I have this in Tag:

    init(readFromEmbeddedSubframes subframes: [FrameKey : Frame]) {
        self.frames = subframes
    }

    public init() {
        self.init(readFromEmbeddedSubframes: [:])
    }

And I build the subframe "tag" using that:

        var subframeTag = Tag()
        subframeTag.title = "Table Of Contents"
        tag[tableOfContents: "ctoc"]?.embeddedSubframesTag = subframeTag

Now when I try to print the TableOfContents frame, it prints nil. However, the frame is in the file when I view the unhandled frames in Yate. So...I'm...sort of getting there? I think?

[NCrusher74]

Here's what I'm seeing in Yate, incidentally:

CTOC

 0: 63 74 6f 63 03 63 68 30 31 63 68 30 32 63 68 30 33 00 54 61  |ctoc.ch01ch02ch03.Ta|
20: 62 6c 65 20 4f 66 20 43 6f 6e 74 65 6e 74 73                 |ble Of Contents     |

I'm pretty sure the 03 byte after ctoc is the entryCount (the count of entries in the childElementIDs array, which is supposed to be stored as UInt8.) So that appears to be working properly. Yay?

However, getting rid of the encoding of the childElementIDs also got rid of the null termination and that's going to be a problem for parsing, I think.

[NCrusher74]

Sorry, my son was talking to me while I was trying to write those last two comments, and I never got around to addressing the missing else in the setter.

I'm actually not sure how I'm supposed to handle this.

There are two frames which might need an embedded subframe. I was trying to avoid repeating as much nearly-identical code as possible but I'm not sure I can get away with that here without creating a parameter for the frametype?

    public subscript(subframeTag parentFrameElementID: String) -> Tag? {
        get {
            var tag = Tag(subframes: [:])
            if let parentFrame = self[chapters: parentFrameElementID] {
                tag = parentFrame.embeddedSubframesTag ?? Tag(subframes: [:])
            } else if let parentFrame = self[tableOfContents: parentFrameElementID] {
                tag = parentFrame.embeddedSubframesTag ?? Tag(subframes: [:])
            } else {
                return nil
            }
            let subframeTag = tag
            return subframeTag
        }
        set {
            if let new = newValue {
                // I thought doing it this way would create the frame if one doesn't already exist?
                // but maybe I was wrong?
                if var parentFrame = self[chapters: parentFrameElementID] {
                    parentFrame.embeddedSubframesTag = new
                } else {
                    var parentFrame = self[tableOfContents: parentFrameElementID]
                    parentFrame?.embeddedSubframesTag = new
                }
            }
        }
    }

[NCrusher74]

Well, I'm pretty much back where I started after rewriting things about six times. Everything is building, but the tag isn't writing to the file. Or rather, it is, but it doesn't contain anything except, apparently, the entryCount byte.

Once I made the embeddedSubframes: [FrameKey: Frame] property into embeddedSubframesTag: Tag that simplified things immensely. Instead of having to use a separate getter/setter for the subframes, I was able to do this:

tag[tableOfContents: "ctoc"]?.embeddedSubframesTag?.title = "Table Of Contents"

At least, in theory.

Here is what I have:

    init(subframes: [FrameKey: Frame]) {
        self.frames = subframes
    }

^^^ is used when parsing, so the embeddedSubframesTag is initialized like so:

        self.embeddedSubframesTag = Tag(subframes: subframes)

Whether or not that is working, I can't say, because at present, when I try to query the tag, the print is just nil

    // the elementID is used to differentiate potential multiple toc frames, like `description` in `LocalizedFrame`
    public subscript(tableOfContents elementID: String) -> TableOfContentsFrame? {
        get {
            if let frame = self.frames[.tableOfContents(elementID: elementID)],
                case .tocFrame(let tocFrame) = frame {
                return tocFrame
            } else {
                return nil
            }
        }
        set {
            let key: FrameKey = .tableOfContents(elementID: elementID)
            let frame = TableOfContentsFrame(
                .known(.tableOfContents),
                elementID: elementID,
                topLevelFlag: newValue?.topLevelFlag ?? true,
                orderedFlag: newValue?.orderedFlag ?? true,
                childElementIDs: newValue?.childElementIDs ?? [],
                embeddedSubframesTag: newValue?.embeddedSubframesTag)
            self.frames[key] = .tocFrame(frame)
        }
    }

And here's how I'm trying to use it:

    func testWritingTableOfContentsOnBlankFile() throws {
        var tag = try tagNoMeta()

        tag[tableOfContents: "ctoc"]?.topLevelFlag = false
        tag[tableOfContents: "ctoc"]?.orderedFlag = false
        tag[tableOfContents: "ctoc"]?.childElementIDs = ["ch1", "ch2","ch3"]
        tag[tableOfContents: "ctoc"]?.embeddedSubframesTag?.title = "Table Of Contents"

        let outputUrl = try localDirectory(fileName: "ctoctest", fileExtension: "mp3")
        XCTAssertNoThrow(try mp3NoMeta().write(tagVersion: .v2_4,
                                               using: tag,
                                               writingTo: outputUrl))

        // MARK: Confirm accuracy
        let mp3UrlWritten = outputUrl
        let mp3FileWritten = try Mp3File(location: mp3UrlWritten)
        let tagWritten = try Tag(readFrom: mp3FileWritten)

        print(tagWritten[tableOfContents: "ctoc"]) // nil

I'll poke at it more tomorrow, but let me know if anything catches your eye.

[NCrusher74]

I'm still trying to puzzle out why nothing is writing to the frame. The frame is being written, according to Yate and Kid3, but it has no content, except for the entries-count byte.

Could it be my change to the way the strings are being encoded?

Here's what Yate is showing me:

CTOC

0: 00 03 00 00 00                                               |.....               |

The 03 is the entry count byte, so it's correctly detecting that the childElementIDs array has three entries.

But the elementID that should precede that is missing, as are the child element IDs and subframes. However, it looks like maybe the null terminator for both the elementID and the childElementIDs is there.

Which suggests that maybe I screwed up when I added this to Data:

    func addingNullTerminationToASCIIEncodedString() -> Data {
        let null = Data(repeating: 0x00, count: StringEncoding.utf8.sizeOfTermination)
        return null
    }

See, there's no encoding specified for the CTOC frame, and when I was encoding it isoLatin1, the first character of the first entry in childElementIDs was being cut off. So I changed the encoding to ASCII.

However, encodedASCII doesn't have an option for termination, so I created the function above.

But perhaps I did it wrong, so all I'm getting is the nulls instead of the strings?

(it doesn't explain why the subframes aren't being added, but that may be another issue.)

[NCrusher74]

Okay, yeah it looks like that was the issue. I got rid of the Data extension and changed the encodedASCII extension to this:

    func encodedASCII(withNullTermination: Bool) -> Data {
        var data = Data()
        data.append(contentsOf: self.utf8)
        if withNullTermination == true {
            let null = Data(repeating: 0x00, count: StringEncoding.utf8.sizeOfTermination)
            data.append(contentsOf: null)
        }
        // UTF‐8 is a superset of ASCII.
        return data
    }

And now I've got data in the frame again...sort of.

Optional(SwiftTaggerID3.TableOfContentsFrame(flags: 2 bytes, layout: 
SwiftTaggerID3.FrameLayoutIdentifier.known(SwiftTaggerID3.KnownFrameLayoutIdentifier
.tableOfContents), frameKey: SwiftTaggerID3.FrameKey.tableOfContents(elementID: "ctoc"),
 allowMultipleFrames: true, elementID: "ctoc", topLevelFlag: true, orderedFlag: true,
 childElementIDs: ["h1", "ch2", "ch3", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", 
"", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", 
"", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", 
"", "", "", "", "", "", "", "", "", "", "", ""], embeddedSubframesTag:
 Optional(SwiftTaggerID3.Tag(frames: [:]))))

It's not seeing my changes to the booleans (they should be false) or the subframes, the first character is being cut off the first entry in the childElementIDs array again...fun.

However, I think the problem (with the exception of the missing subframe) may be on the parsing side. Because here is what Yate sees written to the file:

63 74 6f 63 00 - ctoc + null
03 - count of child element ids?
63 68 31 00 - ch1 + null
63 68 32 00 - ch2 + null
63 68 33 00 - ch3 + null

By comparison, here is what Yate shows me when reading a file chapterized by an audio editor:

54 4f 43 00 - "TOC"
03 - flags byte? Not sure why it would be 03, but the boolean values on bits 1 and 2 are true
02 - count of child element IDs
63 68 30 00 = "ch0"
63 68 31 00 = "ch1"

So it looks like maybe my flags byte isn't being encoded into the file and that's why the first character of the array is being cut off.

[NCrusher74]

Yep, the flags byte was missing from the encodeContents function (maybe I accidentally deleted it when changing something else?)

Once I added it back in, everything started working with the exception of the subframe. That's still missing.

Except...

 0: 63 74 6f 63 00 00 00 00 02 03 63 68 31 00 63 68 32 00 63 68  |ctoc......ch1.ch2.ch|
20: 33 00                                                        |3.                  |

What's with all the nulls after the elementID? There should be one, not four.

[NCrusher74]

Okay I see what I did wrong.

I was so used to dealing with integers that needed to be converted to 4 bytes by way of UInt32 that I used the same approach with the flag byte, which only needs to be one byte:

    // convert the boolean flags to a single byte of data
    var encodedFlagByte: Data {
        switch self.topLevelFlag {
            case true:
                switch self.orderedFlag {
                    case true:
                        let flagByteBinaryInt = 0b00000011
                        return flagByteBinaryInt.truncatedUInt32.bigEndianData
                    case false:
                        let flagByteBinaryInt = 0b00000010
                        return flagByteBinaryInt.truncatedUInt32.bigEndianData
            }
            case false:
                switch self.orderedFlag {
                    case true:
                        let flagByteBinaryInt = 0b00000001
                        return flagByteBinaryInt.truncatedUInt32.bigEndianData
                    case false:
                        let flagByteBinaryInt = 0b00000000
                        return flagByteBinaryInt.truncatedUInt32.bigEndianData
            }
        }
    }

Problem is...I don't actually know another way to get from Int to Data except by converting to UInt32.

[NCrusher74]

Okay, this is how I got it to work as a single byte. Hopefully I did it right:

    // convert the boolean flags to a single byte of data
    var encodedFlagByte: Data {
        var flagByteBinaryInt: UInt8 = 0b00000000
        switch self.topLevelFlag {
            case true:
                switch self.orderedFlag {
                    case true:
                        flagByteBinaryInt = 0b00000011
                    case false:
                        flagByteBinaryInt = 0b00000010
            }
            case false:
                switch self.orderedFlag {
                    case true:
                        flagByteBinaryInt = 0b00000001
                    case false:
                        flagByteBinaryInt = 0b00000000
            }
        }
        let uInt8Array = [flagByteBinaryInt]
        return Data(uInt8Array)
    }

[SDGGiesbrecht]

Yes. The var can be let instead if you leave the = [placeholder] off. Then the compiler will enforce that you assign it a value exactly once in each code branch, and that it has been assigned by the time you try to use it.

[NCrusher74]

Interesting! I would have thought that would have told me flagByteBinaryInt was an immutable value, or that it was being used before being initialized. I probably should have figured that out by now.

side-eyes all my other variables

[SDGGiesbrecht]

Not sure why it would be 03, but the boolean values on bits 1 and 2 are true

Because 0b00000011 (as a binary integer) means (0 × 2⁷) + (0 × 2⁶) + (0 × 2⁵) + (0 × 2⁴) + (0 × 2³) + (0 × 2²) + (1 × 2¹) + (1 × 2⁰).

Simplifying yields (0 × 128) + (0 × 64) + (0 × 32) + (0 × 16) + (0 × 8) + (0 × 4) + (1 × 2) + (1 × 1).

And then 0 + 0 + 0 + 0 + 0 + 0 + 2 + 1.

And then 3.

---

It’s just like in our familiar decimal notation where 0011 means (0 × 10³) + (0 × 10²) + (1 × 10¹) + (1 × 10⁰).

Simplifying yields (0 × 1000) + (0 × 100) + (1 × 10) + (1 × 1).

And then 0 + 0 + 10 + 1.

And then 11.

---

It sounds like you got everything fixed? Or is there still a question hiding somewhere in all those messages that you still need an answer for?

[NCrusher74]

So all that remains to get TableOfContentsFrame working is the subframes. Which will probably be the main thing that keeps ChapterFrame from working as well, so 2 birds, 1 stone, yadda yadda.

I suspect the problem is in my encoding, since the subframe reads properly on a file written by another app. I was sort of taking a stab in the dark with the subframes, anyway, so I'm not surprised.

Here's the portion of the encodeContents function for the subframes(Tag):

        // encode and append the subframes to data
        var encodedSubframes = Data()
        for subframe in self.embeddedSubframesTag?.frames ?? [:] {
            encodedSubframes.append(
                try encodeSubframes(
                    subframe: subframe.value.asFrameProtocol,
                    version: version))
        }
        frameData.append(encodedSubframes)

which uses this function:

    // use FrameProtocol `encodeContents` method to encode subframes
    func encodeSubframes(subframe: FrameProtocol, version: Version) throws -> Data {
        return try subframe.encodeContents(version: version)
    }

walking through it step-by-step:

// embeddedSubframesTag?.frames is a dictionary
// so for each key:value pair in the dictionary...
for subframe in self.embeddedSubframesTag?.frames ?? [:] {

// handle the subframe value (i.e. a Frame instance) as through it conforms to FrameProtocol
// and feed it to the encodeSubframes function
            encodedSubframes.append(
                try encodeSubframes(
                    subframe: subframe.value.asFrameProtocol,
                    version: version))
        }
// (which in turn calls the version-appropriate encodeContents function for the specific frame type that is the subframe)
// and append it to the frameData

At a glance, I suspect where this is going wrong is in the handling of the subframe as FrameProtocol? Which, again, doesn't surprise me, because I'm not entirely clear on how it works:

    /// A property that permits a subframe to be encoded as a frame
    var asFrameProtocol: FrameProtocol {
        switch self {
            case .stringFrame(let stringFrame):
                return stringFrame
            case .partOfTotalFrame(let partOfTotalFrame):
                return partOfTotalFrame
            case .localizedFrame(let localizedFrame):
                return localizedFrame
            case .languageFrame(let languageFrame):
                return languageFrame
            case .creditsListFrame(let creditsListFrame):
                return creditsListFrame
            case .dateFrame(let dateFrame):
                return dateFrame
            case .presetOptionsFrame(let presetOptionsFrame):
                return presetOptionsFrame
            case .tocFrame(let tocFrame):
                return tocFrame
            case .chapterFrame(let chapterFrame):
                return chapterFrame
            case .unknownFrame(let unknownFrame):
                return unknownFrame
            case .imageFrame(let imageFrame):
                return imageFrame
        }
    }

(on second glance, I suspect I should be using encode instead of encodeContents, but that doesn't solve the issue.)

[NCrusher74]

Sorry, I've been spamming you. Feel free to tell me to knock it off.

Once I ask a question, I feel like I need to update you on my progress so that you know whether or not I'm still needing help with the issue, but sometimes I go overboard.

[NCrusher74]

I did it...sort of?

Turns out, it was an issue with unwrapping the optional property in the encode function.

I changed

    public var embeddedSubframesTag: Tag?

to:

    public var embeddedSubframesTag: Tag = Tag(subframes: [:])

Which let me get rid of the fallback default on:

         for subframe in self.embeddedSubframesTag.frames {

And suddenly it worked.

There is some other tag data getting folded in:

Optional(SwiftTaggerID3.TableOfContentsFrame(flags: 2 bytes, layout: SwiftTaggerID3.FrameLayoutIdentifier.known(SwiftTaggerID3.KnownFrameLayoutIdentifier.tableOfContents), frameKey: SwiftTaggerID3.FrameKey.tableOfContents(elementID: "ctoc"), allowMultipleFrames: true, elementID: "ctoc", topLevelFlag: true, orderedFlag: false, childElementIDs: ["ch1", "ch2", "ch3"], embeddedSubframesTag: SwiftTaggerID3.Tag(frames: [SwiftTaggerID3.FrameKey.title: SwiftTaggerID3.Frame.stringFrame(SwiftTaggerID3.StringFrame(flags: 2 bytes, layout: SwiftTaggerID3.FrameLayoutIdentifier.known(SwiftTaggerID3.KnownFrameLayoutIdentifier.title), frameKey: SwiftTaggerID3.FrameKey.title, allowMultipleFrames: false, contentString: "Table Of Contents", urlFrameKeys: [SwiftTaggerID3.FrameKey.artistWebpage, SwiftTaggerID3.FrameKey.audioFileWebpage, SwiftTaggerID3.FrameKey.audioSourceWebpage, SwiftTaggerID3.FrameKey.copyrightWebpage, SwiftTaggerID3.FrameKey.paymentWebpage, SwiftTaggerID3.FrameKey.publisherWebpage, SwiftTaggerID3.FrameKey.radioStationWebpage]))])))

But I suspect that may be because there are bits and pieces of tags that didn't get fully removed when I was trying to create a tag-less file. When I print the tag on the "noMeta" file, here is what I get:

Tag(frames: [SwiftTaggerID3.FrameKey.encodingSettings: SwiftTaggerID3.Frame.stringFrame(SwiftTaggerID3.StringFrame(flags: 2 bytes, layout: SwiftTaggerID3.FrameLayoutIdentifier.known(SwiftTaggerID3.KnownFrameLayoutIdentifier.encodingSettings), frameKey: SwiftTaggerID3.FrameKey.encodingSettings, allowMultipleFrames: false, contentString: "Lavf58.29.100", urlFrameKeys: [SwiftTaggerID3.FrameKey.artistWebpage, SwiftTaggerID3.FrameKey.audioFileWebpage, SwiftTaggerID3.FrameKey.audioSourceWebpage, SwiftTaggerID3.FrameKey.copyrightWebpage, SwiftTaggerID3.FrameKey.paymentWebpage, SwiftTaggerID3.FrameKey.publisherWebpage, SwiftTaggerID3.FrameKey.radioStationWebpage]))])

So... I need to come up with a better way to wipe a file.

ETA: Actually, it looks like those URL frameKeys aren't part of the tag, but are somehow being read from an array I created in StringFrame to differentiate URL frames for encoding purposes.

[SDGGiesbrecht]

It sounds like you got the main problem fixed? Then I would merge this and start a new one for dealing with the new issue you found.

It might be worth looking into conforming some of your types to CustomStringConvertible to make the print output more legible. It could make debugging things like this easier. Those last two pieces of pasted output are dizzying to look at.

[NCrusher74]

Yeah those printouts have been driving me nuts. I will work on that.

[NCrusher74]

Okay, this is what it looks like when I print that frame in the tag now:

Optional(ElementID: ctoc,
TOCisTopLevel: true,
ChildElementsAreOrdered: false,
ChildElementIDs: ["ch1", "ch2", "ch3"],
EmbeddedSubframes:
[SwiftTaggerID3.FrameKey.title],
[SwiftTaggerID3.Frame.stringFrame(FrameKey: title,
Content: Table Of Contents)])

I need to get to work, so I'll work on the other frames later. I'll close and merge this branch after I get home tonight, once I've had a chance to look over the ChapterFrame and make sure it's working now too.

[SDGGiesbrecht]

I added a bunch of print statements to ChapterFrame’s init(decodingContents:version:layout:flags: Data). That method receives the following data, interprets it as described below, and results in a chapter from 0 to 3360 with no subframes:

63 68 30 33 00 (Id of “ch03”, with null terminator)
00 00 00 00 (start time of 0)
00 00 0D 20 (end time of 3360)
(start byte offset) looks at a slice dropping the first 4 bytes, without removing them.
(end byte offset) looks at a slice dropping the first 4 bytes, without removing them.
00 00 00 00 (possible frame identifier determined to be padding, remainder is skipped)
00 00 13 B0 FF FF FF FF FF FF FF FF 00 43 68 61 70 74 65 72 20 54 68 72 65 65

So...

1. How many bytes are the start and end times supposed to be? It looks like each is eight in the file, but you’re reading them as four each. That’s why the start time ends up in the end time.
2. _ = parsing.dropFirst(4) creates (and doesn’t use) a separate slice. Did you mean parsing = parsing.dropFirst(4)?

[NCrusher74]

headdesk I was certain the parsing wasn't the problem, and it turns out it is.

The start and end times are supposed to be four bytes each, so I need to figure out why they're coming out as eight kick myself a few times for seeing that they were eight when looking at the raw data from Yate and not noticing that was wrong.

• Fixed the length of the start and end times (I was trying to use a different extension but apparently it wasn't doing what I thought it was).
• Fixed the parsing = parsing.dropFirst(4) issue.

Now I just need to figure out why the subframes are missing.

Thank you.

[NCrusher74]

I keep thinking in circles about how to handle the overwriting of chapter and toc frames. None of which is helped by the fact that I've literally been interrupted at least once every sixty seconds since I woke up this morning by other stir-crazy, bored members of the household, so it's impossible for me to follow a train of thought for long. (it's taken me over an hour to get this paragraph written, I'm not even joking.)

I've got remove functions for frames with subscript accessors, like so:

    public mutating func removeChapterFrame(withElementID: String) {
        self.frames[.chapter(elementID: withElementID)] = nil
    }

But right now, if I try to create a chapter frame with the same start time as another chapter frame, or a TOC frame with the topLevelFlag set when there is already a top-level TOC frame, as long as the elementID is different, there's no way to prevent that, and there probably needs to be.

I've got these errors that I can throw:

            case .TopLevelTOCAlreadyPresent:
                return NSLocalizedString("A top-level TOC frame already exists.", comment: "Only one TOC frame may have a top-level flag set. Please delete the existing TOC frame, or set the top-level flag boolean to false.")
            case .ChapterAlreadyExistsAtStartTime:
                return NSLocalizedString("A chapter already exists at that start time.", comment: "Please delete the existing Chapter frame, or select a different start time.")

But I'm not sure where I can throw them from. The logical place would be when the frame is being encoded, here:

    // convert the boolean flags to a single byte of data
    var encodedFlagByte: Data {
        let flagByteBinaryInt: [UInt8]
        switch self.topLevelFlag {
            case true:
                switch self.orderedFlag {
                    case true:
                        flagByteBinaryInt = [0b00000011]
                    case false:
                        flagByteBinaryInt = [0b00000010]
            }
            case false:
                switch self.orderedFlag {
                    case true:
                        flagByteBinaryInt = [0b00000001]
                    case false:
                        flagByteBinaryInt = [0b00000000]
            }
        }
        return Data(flagByteBinaryInt)
    }

Creating a check to see if any other TOC frames exist with the flag set to "true", or here:

        // convert integers to UInt32 and then to Data and append
        frameData.append(self.startTime.truncatedUInt32.bigEndianData)
        frameData.append(self.endTime.truncatedUInt32.bigEndianData)

Creating a check to see if a chapter frame exists with that start time.

However...at that point, the encode function only sees that particular frame it's encoding for, not any of the other frames, so I don't know how I would do that. Possibly in Tag somewhere, such as here where it's pulling together all the encoded frames?

    /// Pulls all the data from all the frames together into the tag's contents data
    /// - Parameter version: The `Version` of the ID3 tag being created
    /// - Throws: Caller will determine how to handle any errors
    /// - Returns: `data` containing all the frames of the tag
    func framesData(version: Version) throws -> Data {
        var framesData = Data()
        for frameKey in self.frames.keys {
            if let frame = self.frames[frameKey] {
                framesData.append(try frame.framesData(version: version))
            }
        }
        return framesData
    }

Or is there a better way to handle it?

[NCrusher74]

(Note: how we handle this may also play a role in how I handle duplicates for other frame types that don't allow duplicates, which I believe I've mentioned before?)

[SDGGiesbrecht]

Now I just need to figure out why the subframes are missing.

That didn’t automatically fix it? I figured it was just the null bytes that hadn’t been removed were causing it not to look at the right place for the beginning of the first frame. Since the first byte was 0x00, it all appeared to be padding.

[NCrusher74]

Now I just need to figure out why the subframes are missing.

That didn’t automatically fix it? I figured it was just the null bytes that hadn’t been removed were causing it not to look at the right place for the beginning of the first frame. Since the first byte was 0x00, it all appeared to be padding.

No, I had forgotten to copy one change I'd made in the subframe handling when working on the TOC frame over to the Chapter frame. I was using encodeContents from FrameProtocol to encode the subframe, instead of encode which meant the frame header information wasn't being included for the subframes. So the first byte WAS 0x00, because that was the encoding byte.

[NCrusher74]

Btw, as far as adding the check in Tag, I get this far:

    /// Pulls all the data from all the frames together into the tag's contents data
    /// - Parameter version: The `Version` of the ID3 tag being created
    /// - Throws: Caller will determine how to handle any errors
    /// - Returns: `data` containing all the frames of the tag
    func framesData(version: Version) throws -> Data {
        var framesData = Data()
        for frame in self.frames.values {
            if frame == Frame.tocFrame(<#T##TableOfContentsFrame#>) {

            }
        }
        for frameKey in self.frames.keys {
            if let frame = self.frames[frameKey] {
                framesData.append(try frame.framesData(version: version))
            }
        }
        return framesData
    }

and realize I don't have an initializer for the frame in question that doesn't require specific data about that frame that isn't relevant for the check. I need something like an initializer based on the top-level flag for the TOC frame, right? And one based on the startTime for the chapter frame?

[SDGGiesbrecht]

So it is fixed now though?

[NCrusher74]

Yes. Everything is reading and writing properly. All I need to do is figure out how to handle situations where a frame that shouldn't be duplicated might be duped anyway because the elementID is different.

[NCrusher74]

I don't know if it's the constant interruptions today (my 12yo is bored out of his mind) or it a solution truly doesn't exist as things stand now, but I cannot find a place where I can compare one instance of a frame to another, at least not without all the properties to initialize the frame.

For instance, here in Frame.framesData(version:) before calling the `encode function seems a good place:

            case .chapterFrame(let chapterFrame):
                if chapterFrame.startTime // ... and then what?
                // how do I compare two instances of chapterFrame from here?
                return try chapterFrame.encode(version: version)

It seems like this only really "sees" a single frame at a time. Okay, so we go up a level to Tag. But by then we're looking at having to pull the value (Frame) out of the [FrameKey: Frame] property, and to do that, we need to initialize Frame again, and how do I initialize two instances of the same frame to compare their startTime properties and see if they're the same?

I actually briefly toyed with the idea of making the FrameKey for the chapter frame dependent upon startTime instead of elementID, so then it would be impossible to have two frames with the same startTime, but that wouldn't work with the TableOfContentsFrame with regard to the topLevelFlag boolean. Or, I mean, it could, but ultimately you'd be restricted to two TableOfContents frames, one for a true topLevelFlag and one for a false.

Unless...

Maybe I could have a third frame-identifier (the elementID) for the TOC frame if the topLevelFlag is false?

Like, there would be two frameKey cases for tableOfContents.

enum FrameKey {
// this is the frame key initialized if the `topLevelFlag` is set to true
case tableOfContentsTopLevelTOC 
// this is the frame key initialized if the `topLevelFlag` is set to false, 
// it uses a secondary frame identifier since there can be multiple non-top-level TOCs
case tableOfContentsSecondaryTOC(elementID: String)
// and then of course
case chapter(startTime: Int)
}

(though, actually, I would probably convert the Int to a String for ChapterFrame startTime, because otherwise it would mess up some other places where I'm looking for a String as the secondary frame identifier for establishing the FrameKey.)

Hmm...yeah, that might work. Amazing the way I can finally start figuring things out when everyone else goes to bed. There's a reason why I tend to send you so many comments in the middle of the night. It's the only time I have to myself anymore.

[NCrusher74]

Okay, that is done, and it seems to work. It overwrites an existing frame with the same frameKey based on the topLevelFlag boolean value (for TOC frames) or startTime (for chapter frames).

There are other issues, however.

For example: if the file already has a TOC+chapter frames, and I overwrite the TOC with a new TOC that doesn't include all the chapters in the childElementID list (both the ones I created and the ones that were already there and not overwritten because the start time is different) then none of the chapters will be recognized by other apps that normally would recognize the chapters (in this case, I'm using Fission to check, because apps that handle MP3 chapters are rarer than hen's teeth.)

So, if I write a file with TOC + chapters from scratch, Fission will recognize the chapter breaks. But if I write a file where not all the chapters are accounted for in the TOC, Fission won't recognize ANY chapter breaks, even the ones listed in the TOC.

Honestly, it would probably be better to have a way of listing all the chapter and/or TOC frames, but again, I'm not sure how to write the logic to make that happen, because all our logic for querying frames relies on either knowing the FrameKey or initializing the Frame. I suspect most people are just going to want a list of the chapters, and will want to generate a fresh TOC after removing/adding whichever chapters they choose. Frankly, I think most people would prefer the generation of the TOC to be automatic, rather than something they have to do manually. So, maybe that's tomorrow's project.

But I'm not sure how much of this I should handle here, and how much I should just leave to the user to work out in whatever app they use SwiftTaggerID3 for (though in this case, the user is going to be me. Who knows who else will eventually adopt this.)

[SDGGiesbrecht]

I think I would create a TableOfContents structure that is completely unrelated to how it will be encoded in ID3. It wouldn’t care about identifiers, and its contents could have inherent order.

Then your Tag would vend a property of that type instead, and you would only internally convert it to and from the frame‐based format, assigning arbitrary IDs as needed. Something like this:

public struct TableOfContents {
  // The keys are the start times.
  // If you need to be able to have two chapters start at the same instant
  // —that means a zero‐length chapter—,
  // then make the keys `Double` so you can have both 1 and 1.5,
  // rounding down when encoding.
  // That would allow them to co‐exist and preserve order between them. 
  public var chapters: [Int: Chapter]

  public struct Chapter {
    public var name: String
    // No end time property.
    // Always derive it from the start of the next chapter instead.

    // Whatever other properties there need to be.
    // This one is just an example.
    // I don’t know what they actually need.
    public var subsections: [Int: Chapter]
  }

  /// The chapters in chronological order.
  public func sortedChapters() -> [(startTime: Int, startTime: Chapter)] {
    return chapters.keys.sorted().map { ($0, chapters[$0]!) }
  }
}

extension Tag {
  var tableOfContents: TableOfContents {
    get {
      // Find the frame and assemble an instance of your more intuitive type to return.
    }
    set {
      // Assemble a frame based on the new value of your type and overwrite the old frame.
      // You’ll create arbitrary identifiers of your own here whenever needed.
    }
  }
}

[NCrusher74]

Thank you!

This is the first time I've been able to get on since early this afternoon, because it was a long work day, but I will work on this in a new branch, since this branch is stable and all the tests are passing, so this is a good point to merge before taking on something else of that magnitude.