working on the date frame

[NCrusher74]

Okay, I could use some guidance on this, because I feel like I'm overthinking it or making it more complicated than it needs to be.

I'm starting off with a variation on the Date extension you created for AudiobookTagger:

extension Date {

    internal init?(id3TimeStamp: (year: Int?, month: Int?, day: Int?, hour: Int?, minute: Int?)) {
        if let date = DateComponents(
            calendar: Calendar(identifier: .iso8601),
            timeZone: TimeZone(secondsFromGMT: 0),
            year: id3TimeStamp.year,
            month: id3TimeStamp.month,
            day: id3TimeStamp.day,
            hour: id3TimeStamp.hour,
            minute: id3TimeStamp.minute
        ).date {
            self = date
        } else {
            return nil
        }
    }

    internal var id3TimeStamp: (year: Int?, month: Int?, day: Int?, hour: Int?, minute: Int?) {
        let components = Calendar(identifier: .iso8601)
            .dateComponents(
                in: TimeZone(secondsFromGMT: 0)!,
                from: self)
        return (year: components.year!,
                month: components.month!,
                day: components.day!,
                components.hour!,
                components.minute!)
    }
}

According to the specs, a date in an ID3 tag is supposed to be in ISO-8601 format, so I changed the calendar to that, and added the hour/minute ints, because with the exception of the v2.2/2.3 time/date frames, they're supposed to be in there.

I also created these formatters for DateFormatter, though I haven't found any use for them yet. Again, this is going off what the spec calls for.

extension DateFormatter {
    // This is the format used in the ID3 "Date" frame
    static let id3DayMonth: DateFormatter = {
        let formatter = DateFormatter()
        formatter.dateFormat = "dd-MM"
        formatter.calendar = Calendar(identifier: .iso8601)
        formatter.timeZone = TimeZone(secondsFromGMT: 0)
        formatter.locale = Locale(identifier: "en_US_POSIX")
        return formatter
    }()

    // This is the format used in the ID3 "Time" frame
    static let id3HourMinute: DateFormatter = {
        let formatter = DateFormatter()
        formatter.dateFormat = "HH-mm"
        formatter.calendar = Calendar(identifier: .iso8601)
        formatter.timeZone = TimeZone(secondsFromGMT: 0)
        formatter.locale = Locale(identifier: "en_US_POSIX")
        return formatter
    }()

    // This is the format used in the ID3 "Year" frame
    static let id3Year: DateFormatter = {
        let formatter = DateFormatter()
        formatter.dateFormat = "yyyy"
        formatter.calendar = Calendar(identifier: .iso8601)
        formatter.timeZone = TimeZone(secondsFromGMT: 0)
        formatter.locale = Locale(identifier: "en_US_POSIX")
        return formatter
    }()

    // this is the format used in all other ID3 date-related frames
    static let id3TimeStamp: DateFormatter = {
        let formatter = DateFormatter()
        formatter.dateFormat = "yyyy-MM-dd'T'HH:mm"
        formatter.calendar = Calendar(identifier: .iso8601)
        formatter.timeZone = TimeZone(secondsFromGMT: 0)
        formatter.locale = Locale(identifier: "en_US_POSIX")
        return formatter
    }()

My properties for the DateFrame are where I'm probably making things more complicated than they need to be. My initial thinking was that if I can parse whatever is coming in from a frame as a string into a timestamp, I can then use that to initialize all the individual components:

    var year: Int?
    var month: Int?
    var day: Int?
    var hour: Int?
    var minute: Int?
    var timeStamp: Date
    var timeStampString: String?

Where things start breaking down is in the parsing, which I don't think I've implemented properly:

        // assuming the timestamp is in ISO-8601 format as per the ID3 spec
        self.timeStampString = parsing.extractPrefixAsStringUntilNullTermination(encoding) ?? ""
        let formatter = DateFormatter()
        let formattedTimeStamp = formatter.date(from: timeStampString ?? "")?.id3TimeStamp ?? (year: 0000, month: 00, day: 00, hour: 00, minute: 00)
        self.timeStamp = Date(id3TimeStamp: formattedTimeStamp) ?? Date()

        self.year = timeStamp.id3TimeStamp.year
        self.month = timeStamp.id3TimeStamp.month
        self.day = timeStamp.id3TimeStamp.day
        self.hour = timeStamp.id3TimeStamp.hour
        self.minute = timeStamp.id3TimeStamp.minute

And then the individual getter/setter properties can use whichever variables they need for a particular frame:

    /// - (Release) Date frame getter-setter. Valid for versions 2.2 and 2.3 only.
    /// ID3 Identifier: `TDA`/`TDAT`
    public var date: (day: Int?, month: Int?)? {
        get {
            if let frame = self.frames[.date],
                case .dateFrame(let dateFrame) = frame {
                return (dateFrame.day, dateFrame.month)
            } else {
                return nil
            }
        }
        set {
            let date = Date(id3TimeStamp: (year: nil, month: newValue?.month, day: newValue?.day, hour: nil, minute: nil)) ?? Date()
            let frame = DateFrame(.known(.date), timeStamp: date)
            frames[.date] = .dateFrame(frame)
        }
    }

But yeah, like I said, I don't think I'm implementing it properly.

[SDGGiesbrecht]

Except for all the uses of ? and ?? to fall back to “now” or year 0, nothing looks wrong to me.

[NCrusher74]

Yeah, I think that's one of my biggest issues with dealing with dates. I'm still having trouble unwrapping optionals in a date context.

I was overthinking things quite a bit, since apparently everything I need for dealing with dates in ISO8601 format is already built into Swift. If I can just find an effective way to deal with optionals, I think I'll be golden.

[NCrusher74]

For example, here:

        let dateString = self.timeStampString
        var dateForFrame = ""
        let formatter = ISO8601DateFormatter()
        if let date = formatter.date(from: dateString) {
            let components = Calendar(identifier: .iso8601).dateComponents([.year,.month,.day,.hour,.minute], from: date)
            // TDA/TDAT frame uses DDMM 4-character formatted string
            if self.frameKey == .date {
                dateForFrame = "\(components.day ?? 00)\(components.month ?? 00)"
            }
            // TIM/TIME frame uses HHmm 4 character formatted string
            if self.frameKey == .time {
                dateForFrame = "\(components.hour ?? 00)\(components.minute ?? 00)"
            }
            // TYE/TYER frame uses yyyy 4 character formatted string
            if self.frameKey == .year {
                dateForFrame = "\(components.year ?? 0000)"
            }
            if self.frameKey == .encodingTime ||
                self.frameKey == .originalReleaseTime ||
                self.frameKey == .recordingDate ||
                self.frameKey == .releaseTime ||
                self.frameKey == .taggingTime {
                dateForFrame = "\(components.year ?? 0000)-\(components.month ?? 00)-\(components.day ?? 00)T\(components.hour ?? 00):\(components.minute ?? 00)"
            }
        }
        frameData.append(dateForFrame.encoded(withNullTermination: false))
        return frameData
    }

Should I unwrap each of the components before putting them in the strings, and if so, what should I unwrap them to?

[SDGGiesbrecht]

It’s your choice. If you are falling back to something, just make sure it makes sense. Hour 0 and minute 0 are sensible defaults to use when they are left out. Day 0, month 0 and year 0 aren’t (none of them exist). Remember you can throw if you’re asked to do something impossible or nonsensical.

[NCrusher74]

Okay, I will work on tweaking those defaults to be more sensible.

Interestingly, what I have is working...sort of. My tests are failing, because they're not finding what they expect to see there, but when viewing the date strings in Yate or MediaInfo, this is what I see:

I write this:

        tag.releaseDateTime = (year: 2015, month: 02, day: 03, hour: 11, minute: 11)

and get this:

2015-2-3T11:112015-2-3T11:11

So the data is all there, it's just...repeated. And without the leading zeroes in the month and day.

I don't get why it would be repeated. Is this some oddity with the ISO8601 formatter I missed?

[SDGGiesbrecht]

No idea about the doubling. Nothing in what you’ve posted would explain it from what I can see.

(As for the leading zeroes, you’ll have to add those yourself. You can use these if you want, applied like this.)

[NCrusher74]

Ah I figured out what is causing the doubling, I think. I think I accidentally selected the wrong initializer for calendar.dateComponents.

[NCrusher74]

What I posted initially is all pretty much completely gone, because I'm relying on the ISO8601DateFormatter(). I don't think I even need any extensions.

It seems to be working now for all the frames that get a full date string, except for the releaseTime frame, which is weird, because it literally uses identical code to all the other frames (in fact, I cut/pasted the code for the other frames FROM releaseTime) with the exception of the fact that that's the frame where I'm testing out writing an hour/minute value as well, and the others just have nil for those values.

I need to run out the door to work here soon, but if you spot anything that would be causing me to get nil returns on all five values for that particular frame , let me know.

[SDGGiesbrecht]

This line appears twice, once inside one of the if statements, and then also after all the if statements.

frameData.append(dateForFrame.encoded(withNullTermination: false))

Whenever that particular if statement triggers, it would be appended twice.

[NCrusher74]

This line appears twice, once inside one of the if statements, and then also after all the if statements.

frameData.append(dateForFrame.encoded(withNullTermination: false))

Whenever that particular if statement triggers, it would be appended twice.

Thank you. I did catch that one just before my last push; I think it was an artifact of an earlier version of the encodeContents function.

I haven't tested versions 2.2 and 2.3 yet, because I know they're going to give me trouble. Most of my issues with 2.4 were fixed when, instead of assembling the string being encoded manually (i.e. "(year)-(month)-(day)T(hour):(minute)") I stopped trying to fool myself that I was smarter than the computer and did it this way instead:

dateForFrame = formatter.string(from: date)

Now the only thing that remains an issue with version 2.4 is this. All the 2.4 getter-setter properties are identical, like these two here:

    /// (Release) DateTime frame getter-setter. ID3 Identifier: `TDRL` Valid for version 2.4 only
    public var releaseDateTime: (
        year: Int?,
        month: Int?,
        day: Int?,
        hour: Int?,
        minute: Int?)? {
        get {
            if let date = date(for: .releaseTime) {
                let calendar = Calendar(identifier: .iso8601)
                let timeZone = TimeZone(secondsFromGMT: 0) ?? .current
                let components = calendar.dateComponents(in: timeZone, from: date)
                return (components.year,
                        components.month,
                        components.day,
                        components.hour,
                        components.minute)
            } else {
                return nil
            }
        }
        set {
            set(.known(.releaseTime), .releaseTime,
                year: newValue?.year,
                month: newValue?.month,
                day: newValue?.day,
                hour: newValue?.hour,
                minute: newValue?.minute)
        }
    }

    /// RecordingDateTime frame getter-setter. ID3 Identifier: `TRD`/`TRDA`/`TDRC`
    public var recordingDateTime: (
        year: Int?,
        month: Int?,
        day: Int?,
        hour: Int?,
        minute: Int?)? {
        get {
            if let date = date(for: .recordingDate) {
                let calendar = Calendar(identifier: .iso8601)
                let timeZone = TimeZone(secondsFromGMT: 0) ?? .current
                let components = calendar.dateComponents(in: timeZone, from: date)
                return (components.year,
                        components.month,
                        components.day,
                        components.hour,
                        components.minute)
            } else {
                return nil
            }
        }
        set {
            set(.known(.recordingDate), .recordingDate,
                year: newValue?.year,
                month: newValue?.month,
                day: newValue?.day,
                hour: newValue?.hour,
                minute: newValue?.minute)
        }
    }

And they're encoded identically:

            // all other frames get a full timestamp
            if self.frameKey == .encodingTime ||
                self.frameKey == .originalReleaseTime ||
                self.frameKey == .recordingDate ||
                self.frameKey == .releaseTime ||
                self.frameKey == .taggingTime {
                dateForFrame = formatter.string(from: date)
            }

In fact, when I test the written results in other apps, they both look the same:

... but when testing that what is being written is what I'm trying to write, the test for all of those except releaseTime ("Released Date" in that picture) passes. I'm not sure why that one is different.

The only thing I can think of that makes a difference is the fact that releaseTime has an hour/minute value while the others don't:

        tag.releaseDateTime = (year: 2015, month: 02, day: 03, hour: 11, minute: 11)
        tag.recordingDateTime = (year: 2018, month: 10, day: 11, hour: nil, minute: nil)

        XCTAssertEqual(tagWritten.recordingDateTime?.year, 2018)
        XCTAssertEqual(tagWritten.recordingDateTime?.month, 10)
        XCTAssertEqual(tagWritten.recordingDateTime?.day, 11)

        XCTAssertEqual(tagWritten.releaseDateTime?.year, 2015)
        XCTAssertEqual(tagWritten.releaseDateTime?.month, 02)
        XCTAssertEqual(tagWritten.releaseDateTime?.day, 03)
        XCTAssertEqual(tagWritten.releaseDateTime?.hour, 11)
        XCTAssertEqual(tagWritten.releaseDateTime?.minute, 11)

But I don't see why that would be causing all five values to come out as nil unless it's somehow preventing the string from being parsed in ISO8601 format?

ETA: Nope, I don;t think it's the time being on there. When I make the time values nil and change the test for them to XCTAssertNil, those tests pass, while the test for the date still fails with nil values for year, month, and day.

[NCrusher74]

Argh! I've rewritten this frame from the ground up three times. The date is there and formatted correctly when I view it with other apps, but my test isn't reading it properly, which means that my parsing isn't happening the way it should, I guess? The test keeps telling me the values are nil, which means it can't parse a valid date from the string it's reading from the file.

But it should be able to? I'm writing the string in ISO8601 format, and reading it in ISO8601 format. This is 100% a problem with me not knowing how to use the formatter correctly.

Questions in the code.

[NCrusher74]

Taking it a step at a time...

I've got this test, which writes a date to a file, then reads it and makes sure what is being read is what is being written.

    func testDateTimeV24() throws {
        let mp3Url = Bundle.v23NoMeta
        let mp3File = try Mp3File(location: mp3Url)
        var tag = try Tag(readFrom: mp3File)

        tag.releaseDateTime = (year: 2015, month: 02, day: 03, hour: nil, minute: nil)

        let outputUrl = URL(fileURLWithPath: "/Users/nolainecrusher/Desktop/test output/testV24Date.mp3")
        XCTAssertNoThrow(try mp3File.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)

        XCTAssertEqual(tagWritten.releaseDateTime?.year, 2015) // fails. says releaseDateTime.year is nil.
        XCTAssertEqual(tagWritten.releaseDateTime?.month, 02) // fails. says releaseDateTime.month is nil.
        XCTAssertEqual(tagWritten.releaseDateTime?.day, 03) // fails. says releaseDateTime.day is nil.

However, this is what I'm seeing was written to the file when looking at it from various metadata apps:

Ergo: the problem isn't with what is being written, it's in what is being read.

But just to be sure, I start with my Tag.releaseDateTime property and check out what is being written:

    public var releaseDateTime:
        (year: Int?, month: Int?, day: Int?, hour: Int?, minute: Int?)? {
        get {
            if let date = date(for: .releaseTime) {
//            print(date) // nothing prints
                let calendar = Calendar(identifier: .iso8601)
                let components = calendar.dateComponents([.year, .month, .day, .hour, .minute], from: date)
                return (components.year,
                        components.month,
                        components.day,
                        components.hour,
                        components.minute)
            } else {
                return nil
            }
        }
        set {
            let calendar = Calendar(identifier: .iso8601)
            let timeZone = TimeZone.init(secondsFromGMT: 0)
            let dateComponents = DateComponents(calendar: calendar,
                                      timeZone: timeZone,
                                      year: newValue?.year,
                                      month: newValue?.month,
                                      day: newValue?.day,
                                      hour: newValue?.hour,
                                      minute: newValue?.minute)
            if let date = calendar.date(from: dateComponents) {
//                print(date) // checks 2015-02-03 00:00:00 +0000
                set(.known(.releaseTime), .releaseTime, timeStamp: date)
            }
        }
    }

The date here is working in the set portion of things. So I check the get side and nothing prints.

Working backward from the get portion, I turn my attention to the date(for frameKey:) function.

    internal func date(for frameKey: FrameKey) -> Date? {
        if let frame = self.frames[frameKey],
            case .dateFrame(let dateFrame) = frame {
            print(dateFrame.timeStamp) // nothing prints
            return dateFrame.timeStamp
        } else {
            return nil
        }
    }

So it's not getting the timeStamp from the frame. From there, I move backward another step to where the timeStamp variable is initialized for the frame. Which, since we're reading from a file, should be in the init(decodingContents:) intializer:

    init(decodingContents contents: Data.SubSequence,
         version: Version,
         layout: FrameLayoutIdentifier,
         flags: Data) throws {
        self.flags = flags
        self.layout = layout
        self.frameKey = layout.frameKey(additionalIdentifier: nil)

        var parsing = contents
        let encoding = try DateFrame.extractEncoding(data: &parsing, version: version)
        let parsedString = parsing.extractPrefixAsStringUntilNullTermination(encoding) ?? ""

        // assuming the timestamp is in ISO-8601 format as per the ID3 spec
        let formatter = ISO8601DateFormatter()
        guard let date = formatter.date(from: parsedString) else {
            throw Mp3File.Error.InvalidDateString
        }
        self.timeStamp = date
        print(timeStamp) // checks - 2015-02-03 00:00:00 +0000
        self.timeStampString = formatter.string(from: date)
    }

So...timeStamp is being initialized properly, but is somehow losing the data between here and Tag.date(for frameKey:)

But...there's nothing in between? Like, this is where the timeStamp that date(for frameKey:) is reading comes from.

[SDGGiesbrecht]

internal func date(for frameKey: FrameKey) -> Date? {
  print(#function, frameKey) // .releaseTime
  print(#function, self.frames.keys) // [.userDefinedText("YiTG"), .year]
  // ...

I think it’s all because id3Identifier(version: Version) -> String? returns "TDRL" for several different cases, including both .releaseTime and .year. Since the mapping isn’t bijective, when it iterates them all to create the reverse mapping in stringToLayoutMapping, each one encountered overwrites the previous. Since .year is last, it’s the only entry in the mapping. So during tag loading, when it looks up what to do with "TDRL", it gets told to treat it as a .year. At that point you have a properly loaded frame, but accessible under the wrong key.

[NCrusher74]

Oh wow, that was obscure. I'd forgotten I'd done that. I was just focused on whether or not I was formatting and retrieving the date properly.

That was my attempt to redirect any effort to write one of the version 2.2/2.3 date frames (like the most intuitively-named .date) in version 2.4. Oops. Thank you for catching that.

Now, unlike yesterday's tests, the .day value is being read as day -1 which means I've forgotten to set the timezone somewhere (I don't think I need locale when using ISO8601, right?)

[NCrusher74]

All right, I've got all that stuff I was working on before working, thanks to your catching the thing with the identifier.

Now I'm working on the absurd version 2.2/2.3 date/time/year frames, and asking myself repeatedly why sticking to the (frankly idiotic) spec is so important.

The date frame is supposed to be a 4-character string, DDMM. Time is supposed to be a 4-character string, HHMM and year is supposed to be yyyy.

I pretty much have used the same code for all of them, just swapping out which component is needed. Year is working fine. Time is working fine, with the exception of the fact that my test didn't take into account the 8-hour time difference between me and GMT, so what is being written as "11" is being read as "17".

Date, however, is being written as the fall-back defaults, probably because I'm not using the formatting properly. I'm just a little confused about the right way to use it.

Here's the test:

func testDateTimeV23() throws {
    let mp3Url = Bundle.v23NoMeta
    let mp3File = try Mp3File(location: mp3Url)
    var tag = try Tag(readFrom: mp3File)

    tag.date = (month: 02, day: 03)
    tag.time = (hour: 11, minute: 11)
    tag.year = 2015

    let outputUrl = URL(fileURLWithPath: "/Users/nolainecrusher/Desktop/test output/testV23Date.mp3")
    XCTAssertNoThrow(try mp3File.write(tagVersion: .v2_3, using: tag, writingTo: outputUrl))

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

    XCTAssertEqual(tagWritten.date?.month, 02) // fails because the value is 1
    XCTAssertEqual(tagWritten.date?.day, 03) // fails because the value is 1
    XCTAssertEqual(tagWritten.time?.hour, 11) // fails, but only because the actual value is 17
    XCTAssertEqual(tagWritten.time?.minute, 11) // passes
    XCTAssertEqual(tagWritten.year, 2015) // passes
}

If the value is "1" for day and month, it's because it's receiving the fallback default in the encode function, here:

        if self.frameKey == .date {
            let day = self.timeStamp?.id3DayMonth.day ?? 01
            let month = self.timeStamp?.id3DayMonth.month ?? 01
            encodedString = "\(day)\(month)".encoded(withNullTermination: false)

Which means it's not getting the id3DayMonth from timeStamp, right? Which means my setter isn't working correctly.

Here's the setter:

        set {
            let calendar = Calendar(identifier: .iso8601)
            let timeZone = TimeZone(secondsFromGMT: 0)
            let dateComponents = DateComponents(calendar: calendar,
                                                timeZone: timeZone,
                                                month: newValue?.month,
                                                day: newValue?.day)
            if let date = calendar.date(from: dateComponents) {
                set(.known(.date), .date, timeStamp: date)
            }

I figure somewhere in here, I should probably be using the id3TimeStamp init extension for Date, but I'm not sure how or where.

    internal init?(id3TimeStamp: (year: Int?, month: Int?, day: Int?, hour: Int?, minute: Int?)) {
        if let date = DateComponents(
            calendar: Calendar(identifier: .iso8601),
            timeZone: TimeZone(secondsFromGMT: 0),
            year: id3TimeStamp.year,
            month: id3TimeStamp.month,
            day: id3TimeStamp.day,
            hour: id3TimeStamp.hour,
            minute: id3TimeStamp.minute
        ).date {
            self = date
        } else {
            return nil
        }
    }

I mean, that's probably what it's looking for when it's trying to get id3DayMonth from the components, right?

But it's working for year and time so maybe not? Why would it be working for those and not for date?

[NCrusher74]

(also, I could save myself a whole lot of headache by just going off-spec for version 2.2 and 2.3 and giving them the full timestamp that the version 2.4 frames get, and I'm trying to come up with a good reason for why not to do that.)

[NCrusher74]

Btw, this is what is stored in timeStamp for the date frame before it gets encoded in the encodeContents function:

        if self.frameKey == .date {
            print(self.timeStamp) // checks out Optional(0001-02-03 00:00:00 +0000)
            let day = self.timeStamp?.id3DayMonth.day ?? 01
            let month = self.timeStamp?.id3DayMonth.month ?? 01
            encodedString = "\(day)\(month)".encoded(withNullTermination: false)

So it looks like maybe the id3DayMonth isn't deriving the month and day properly from the string. Which I guess makes sense, id3DayMonth is looking for an DDMM format string, right? So I need to change that.

[NCrusher74]

... I'm so confused.

I tried several different ways to format that particular date, and all of them were yielding the same result. So, just to see what would happen if I removed the fallback of "01" from the equation, I removed the fallback and let the "String interpolation produces a debug description for an optional value; did you mean to make this explicit?" warning stand.

When I ran the test, I still got a failure with a message of: "XCTAssertEqual failed: ("Optional(1)") is not equal to ("Optional(2)")"

So I looked at the result in Kid3, and here's what I see in that field:

I do a search for anyplace else where that frame might be falling back to "1" in the month or day field instead of "2" and "3", respectively, I can't find anything in either DateFrame, the Tag extensions for that frame, or in the Date extension.

So, just to see if something changes, I change the fallback to "00" instead of "01". I clean my build folder, and just for good measure, I restart XCode and delete the contents of my DerivedData directory and rebuild.

When I run my test, nothing changed. It is still falling back to "1"

Here's what Kid3 is showing me:

Which, looking at that, I can maybe see why there's a problem, because it's not using the double-digit dates (I forgot to format that in because the v2.4 frames apparently don't need them) but it shouldn't be falling back to "1" when I've removed that fallback and/or replaced it with "0".

[NCrusher74]

Okay, regardless of the weirdness of my getting a value I shouldn't have been getting there, adding in the leading zeroes fixed the problem, sort of.

The only problem left is the fact that, by the time the parsing is done when reading the date from the file, "02" is coming out as "1", and I'm assuming this is a timezone or locale issue.

        if self.frameKey == .date {
//            print(parsedString) // 0302
            let formatter = DateFormatter()
            formatter.dateFormat = "DDMM"
            formatter.timeZone = TimeZone(secondsFromGMT: 0)
            formatter.locale = Locale(identifier: "en_US_POSIX")
            if let date = formatter.date(from: parsedString) {
//                print(date.id3DayMonth.month) // Optional(1)
                self.timeStamp = date
            }

As you can see, when I print out the string before formatting, it's correct. But after formatting, it's incorrect.

I've tried it with AND without the timezone and locale added to the formatter, and it doesn't make a difference. It still comes out wrong. What is written to the file is correct, but I'm not reading it properly.

[SDGGiesbrecht]

"0302"

"DDMM"

Then the 02 means February, right? No timezone difference would be enough to take February 3 and turn it into January something.

I’m not very familiar with DateComponents, but is it possible the month value is zero based? i.e. 0–11 instead of 1–12? Then February would be 1.

[NCrusher74]

I don't think so? None of the examples I've seen would indicate that (for instance, this article), and it's not happening to any of my other dates.

I'm still poking at it, but this is weird...

[NCrusher74]

... could I possibly need to strip out the leading zeroes? Could it be reading "02" as "0" and assigning it a default date of January?

Except... nope. Even when I try it with a 2-digit month, it still gives me January.

        tag.date = (month: 11, day: 03)

            formatter.dateFormat = "DDMM"
            formatter.timeZone = TimeZone(secondsFromGMT: 0)
            formatter.locale = Locale(identifier: "en_US_POSIX")
            print(parsedString)  // 0311
            print(formatter.date(from: parsedString)) // Optional(2000-01-03 00:00:00 +0000)
            if let date = formatter.date(from: parsedString) {
                self.timeStamp = date
            }

[NCrusher74]

(I've also tried setLocalizedDateFromTemplate and it didn't help.)

[NCrusher74]

Well, this was a super-convoluted way to go about it, and I suspect it will be buggy, but the test passes now.

        // assumes frame contents are spec-compliant, 4-characters, DDMM string
        if self.frameKey == .date {
            // split the four-character string into an array of 2-character strings
            let dayMonthArray = parsedString.components(withLength: 2)
            // make sure the array has at least two elements so we don't go out of bounds
            // if parsedString is not spec-compliant, there may be more than two elements
            // so we'll use the first two, rather than .first and .last
            // because those should be day and month
            // if they're not, we'll probably get nil when we try to make a date out of them
            guard dayMonthArray.count >= 2 else {
                throw Mp3File.Error.InvalidDateString
            }
            // the first array element is the day, make the string an Int
            let day = Int(dayMonthArray[0])
            // second element is the month, make it an Int
            let month = Int(dayMonthArray[1])
            // use day and month as components for a date
            let dateComponents = DateComponents(calendar: calendar,
                                                timeZone: timeZone,
                                                month: month,
                                                day: day)
            if let date = dateComponents.date {
                // initialize the timeStamp property
                self.timeStamp = date
            }

[NCrusher74]

Am I writing these properly? I'm not sure how Swift deals with negatives when using the && operator

        guard !(version == .v2_2 || version == .v2_3) && !(layout == .known(.encodingTime) || layout == .known(.taggingTime) || layout == .known(.releaseTime))  else {
            throw Mp3File.Error.DateFrameNotAvailableForVersion
        }

        guard !(layout == .known(.date) || layout == .known(.time) || layout == .known(.year)) && version != .v2_4 else {
            throw Mp3File.Error.DateFrameNotAvailableForVersion
        }

[SDGGiesbrecht]

Prefix (and suffix) operators take precedence over infix operators.

(∧ (&&) takes precedence over ∨ (||) not just in Swift, but everywhere.)

If you aren’t sure, you can always add extra parentheses.

What you have is the same as this:

guard
  (
      (
          (
              (!(version == .v2_2 || version == .v2_3))
            &&
              (!(layout == .known(.encodingTime))
          )
        ||
          layout == .known(.taggingTime)
      )
    ||
      layout == .known(.releaseTime)
  )
  else { /* ... */ }
// i.e. “Make sure that we’re neither 2.2 nor 2.3 and that we’re anything but .encodingTime,
// or else that we’re .taggingTime (at any version),
// or else that we’re .releaseTime (at any version).”

guard
  (
     (!(
          (
              layout == .known(.date)
            ||
              layout == .known(.time)
          )
        ||
          layout == .known(.year)
      ))
    &&
      version != .v2_4 
  )
  else { /* ... */ }
// i.e. “Make sure that we’re neither .date, nor .time, nor .year,
// and also that we’re not v2.4.”

[NCrusher74]

Okay, I suspected that wasn't going to work but at first glance it seemed to, but that was because I had written it wrong. This appears to work though?

    let validVersion2223DateFrames: [FrameKey] = [.date, .time, .year, .originalReleaseTime, .recordingDate]
    let validVersion24DateFrames: [FrameKey] = [.encodingTime, .taggingTime, .releaseTime, .originalReleaseTime, .recordingDate]

(and then in the encode function)

        switch version {
            case .v2_2, .v2_3:
                guard validVersion2223DateFrames.contains(self.frameKey) else {
                    throw Mp3File.Error.DateFrameNotAvailableForVersion
            }
            case .v2_4:
                guard validVersion24DateFrames.contains(self.frameKey) else {
                    throw Mp3File.Error.DateFrameNotAvailableForVersion
            }
        }

I'm waffling on whether or not I need to do something similar for the decode initializer? I keep thinking we can just ignore or pass through as "unknown" any unhandled date frames for a particular version, but then I start to wonder what might happen if someone tries to query the contents of a frame that isn't valid for a particular version.

This is a problem for another branch, but I also still need to figure out how to handle the functionality for all the other "invalid for version" frames. Date frames are special, because there are date frames that were deprecated and replaced in version 2.4, but pretty much anywhere else in KnownFrameLayoutIdentifier.id3Identifer where the return is nil for a particular version (with the exception of chapter and toc frames) needs to be written as a TXX/TXXX frame.

So, for instance, the Mood (TMOO) frame doesn't exist for version 2.2/2.3. It's a new frame added in version 2.4. So if someone tries to write a tag in version 2.2 or 2.3 that contains a .mood frame, what they should end up getting is a .userDefinedText(description: "Mood") frame. Since that will involve taking what should be a StringFrame and turning it into a LocalizedFrame I suspect it's not going to be as straightforward as it sounds.

I also will need to do something similar for static var frameKeys which point to a FrameKey case that could, conceivably, already be in use. For example:

    /// For audiobook use. Maps to the `artist` frame, which is usually used for authors of audiobooks. If an `artist` frame already exists, the frame will be created as a `UserDefinedText` frame with the description `Author`
    static var author: FrameKey { return .artist }

    /// For audiobook use. Maps to the `composer` frame, which is usually used for narrators of audiobooks. If a `composer` frame already exists, the frame will be created as a `UserDefinedText` frame with the description `Narrator`
    static var narrator: FrameKey { return .composer }

But like I said, that's for another branch. For now, since the date frame seems to be working, I'm going to wrap this one up and start working on Chapter/CTOC frames tomorrow.

[NCrusher74]

Here's an interesting hiccup.

Before I move on from the DateFrame I decided it would probably behoove me to put in some more rigorous testing, with the various frametypes encountering not only the expected input but also unexpected input, writing on a blank file versus overwriting on an already-written tag, etc.

My very first test, I decided to try to enter the date components individually rather than as a tuple, and for some reason, the year components isn't taking:

    /// Version 2.4 only. Identifier `TDEN`
    public var encodingDateTime:
        (year: Int?, month: Int?, day: Int?, hour: Int?, minute: Int?)? {
        get {
            date(for: .encodingTime)
        }
        set {
            let calendar = Calendar(identifier: .iso8601)
            let timeZone = TimeZone(secondsFromGMT: 0)
            let dateComponents = DateComponents(calendar: calendar,
                                                timeZone: timeZone,
                                                year: newValue?.year,
                                                month: newValue?.month,
                                                day: newValue?.day,
                                                hour: newValue?.hour,
                                                minute: newValue?.minute)
            if let date = calendar.date(from: dateComponents) {
//                print(date) 
// 0001-01-01 00:00:00 +0000
// 0001-11-01 00:00:00 +0000
// 0001-11-07 00:00:00 +0000
// 0001-11-07 09:00:00 +0000
// 0001-11-07 09:23:00 +0000
                set(.known(.encodingTime), .encodingTime, timeStamp: date)
            }
        }
    }

Here's how I'm writing the test:

    func testTENCExpectedInputComponentsBlankFileV24() throws {
        let mp3Url = Bundle.v23NoMeta
        let mp3File = try Mp3File(location: mp3Url)
        var tag = try Tag(readFrom: mp3File)

        tag.encodingDateTime?.year = 2002
        tag.encodingDateTime?.month = 11
        tag.encodingDateTime?.day = 7
        tag.encodingDateTime?.hour = 9
        tag.encodingDateTime?.minute = 23

        let outputUrl = URL(fileURLWithPath: "/Users/nolainecrusher/Desktop/test output/FrameTENCtest1.mp3")
        XCTAssertNoThrow(try mp3File.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)

        XCTAssertEqual(tagWritten.encodingDateTime?.year, 2002) // fails
        XCTAssertEqual(tagWritten.encodingDateTime?.month, 11)
        XCTAssertEqual(tagWritten.encodingDateTime?.day, 7)
        XCTAssertEqual(tagWritten.encodingDateTime?.hour, 9)
        XCTAssertEqual(tagWritten.encodingDateTime?.minute, 23)
    }

So, coming out of the setter, it won't set the year, but it will set all the others.

This frame works as expected when tested with all these components added as a tuple:

        tag.encodingDateTime = (year: 2016, month: 04, day: 05, hour: nil, minute: nil)

Is it the fact that when entering the values as components instead of all as a tuple, it treats the frame as optional? Why would it only do that for year instead of for all the components?

[NCrusher74]

(interestingly, while that test doesn't work when writing to a file without that frame already in place, it works fine when overwriting on a file that already has that frame)

[NCrusher74]

(also, any suggestions on tests I can use to try to break things are welcome.)

[SDGGiesbrecht]

Very weird.

tag.encodingDateTime?.year = 2002
tag.encodingDateTime?.month = 11

Is it possible tag.encodingDateTime is nil on the first line (hence there is nothing to attach 2002 to), and that it somehow it is no longer nil by the second line (so that 11 does get attached to something)?

If you switch the order, does the year register but not the month?

[NCrusher74]

Gah I just did it again. I updated your comment instead of replying to it. Sorry.

If I change the order of the test, no. And XCode won't let me change the order of the parameters in the setter.

[SDGGiesbrecht]

I only meant swap the two lines I quoted. It was just a long shot anyway.

let dateComponents = DateComponents(
  calendar: calendar,
  timeZone: timeZone,
  year: newValue?.year,
  month: newValue?.month,
  day: newValue?.day,
  hour: newValue?.hour,
  minute: newValue?.minute
)

What is newValue?.year before this line? What is dateComponents after this line?

[NCrusher74]

    public var encodingDateTime:
        (year: Int?, month: Int?, day: Int?, hour: Int?, minute: Int?)? {
        get {
            date(for: .encodingTime)
        }
        set {
            let calendar = Calendar(identifier: .iso8601)
            let timeZone = TimeZone(secondsFromGMT: 0)
//            print(newValue?.year) // Optional(1)
            let dateComponents = DateComponents(calendar: calendar,
                                                timeZone: timeZone,
                                                year: newValue?.year,
                                                month: newValue?.month,
                                                day: newValue?.day,
                                                hour: newValue?.hour,
                                                minute: newValue?.minute)
//            print(dateComponents) // calendar: iso8601 (fixed) timeZone: GMT (fixed) year: 1 month: 11 day: 7 hour: 9 minute: 23 isLeapMonth: false
            if let date = calendar.date(from: dateComponents) {
                set(.known(.encodingTime), .encodingTime, timeStamp: date)
            }
        }
    }

It has to have something to do with the fact that there's not a value there already for the blank file, right? Because it works when overwriting a value on a file that already has this frame.

[SDGGiesbrecht]

It has to have something to do with the fact that there's not a value there already for the blank file, right? Because it works when overwriting a value on a file that already has this frame.

That’s what I thought at first, but then I expected the month to break instead when you swapped the lines.

 //            print(newValue?.year) // Optional(1)

Is that the first time? If you add a break point there, that print statement is happening inside tag.encodingDateTime?.year = 2002? Because if 2002 is really turning into 1 while it is being passed to the setter, then the compiler is doing something very strange.

[SDGGiesbrecht]

Actually, try just print(newValue). That will tell us both the year and which of the other things have been set so far.

[NCrusher74]

Hmmm... interesting. It's nil on the first pass where the year would be set.

nil
Optional((year: Optional(1), month: Optional(11), day: Optional(1), hour: Optional(0), minute: Optional(0)))
Optional((year: Optional(1), month: Optional(11), day: Optional(7), hour: Optional(0), minute: Optional(0)))
Optional((year: Optional(1), month: Optional(11), day: Optional(7), hour: Optional(9), minute: Optional(0)))
Optional((year: Optional(1), month: Optional(11), day: Optional(7), hour: Optional(9), minute: Optional(23)))

So the "1" is a fallback?

[NCrusher74]

(actually this is the only strange behavior relating to the destination file, but I'm not sure the other issue is related.)

[SDGGiesbrecht]

Then I’m confused about the month having worked when you reversed it, but it does sound again like what I thought at first:

tag.encodingDateTime?.year = 2002

This calls the getter for tag.encodingDateTime and presumably receives nil. The nil causes ? and everything after it to be skipped, and to just save the nil back into tag.encodingDateTime. However that setter is probably actually creating a frame even for nil. (if let date = calendar.date(from: dateComponents) might not be returning nil when we think it should. It could be giving us some sort of default.)

Then the next time around, there is a frame, so the getter for tag.encodingDateTime receives an actual tuple with some defaults in it, which it can then adjust by whatever is after the ?. So from the second use onward, it works as expected. Or if it had a frame to begin with, then the first use works too.

[NCrusher74]

Hm. Okay. So, I should probably make the timeStamp property non-optional, right? Or give it fallback so that it doesn't return nil?

[NCrusher74]

Okay, getting rid of the nil return in the getter solved the issue. I just used Date.distantPast.

But I'm not sure that's a very good idea?

[NCrusher74]

Actually, it also works if the getter (or rather, the date(for frameKey:) function) returns (nil, nil, nil, nil, nil) instead of just nil.

[SDGGiesbrecht]

Actually, it also works if the getter (or rather, the date(for frameKey:) function) returns (nil, nil, nil, nil, nil) instead of just nil.

I like that option better.

[NCrusher74]

While I'm working on tests, I should probably ask:

What is the best way to handle writing test files? My project is a framework for now, but eventually we'll be making a package out of it, and it looked to me from what I saw with ID3TagEditor that handling of the test bundle and paths are different in packages? But I don't understand how.

Should I have a PathLoader class the way ID3TagEditor did? Or is there a better way?

[SDGGiesbrecht]

Writing should be done to a temporary directory. Permanent fixtures that you want to load from the repository can be located using some variation of this.

[NCrusher74]

Will that replace my Bundle.swift file where I have all my shortcut stuff like:

    static let writtenV22: URL = {
        guard let location = Bundle.testBundle.url(forResource: "mp3-v22-with-meta", withExtension: "mp3") else {
            fatalError("The audio file cannot be found")
        }
        return location
    }()

    static func mp3V22() throws -> Mp3File {
        return try Mp3File(location: writtenV22)
    }

    static func tagV22() throws -> Tag {
        return try Tag(readFrom: mp3V22())
    }

[SDGGiesbrecht]

Well, it will replace this piece of it:

Bundle.testBundle.url(forResource: "mp3-v22-with-meta", withExtension: "mp3")

With SwiftPM, there are no bundles.

[NCrusher74]

Would it be better for me to make this a SwiftPM now, or wait until I'm done developing? I think the reason I started it this way is because I noticed when working in SwiftPM, things like the comments at the head of a file didn't get auto-populated with the file name and other such information. I'm not sure what all I'm going to need to change once I make it a package instead. I don't want to get too entrenched in the current structure if I'm going to need to move everything around later.

[SDGGiesbrecht]

Would it be better for me to make this a SwiftPM now, or wait until I'm done developing?

It is probably easiest to put it in the intended format immediately. That way you know before you add something problematic, instead of after.

when working in SwiftPM, things like the comments at the head of a file didn't get auto-populated with the file name and other such information.

There are various tools to handle that. I use this. (Disclaimer: I wrote it.) I can help you set it up if you’d like, but your project will need to be usable as a package (though that doesn’t preclude you from also supporting other formats).

But I would finish off this PR before any of that.

[NCrusher74]

This project will definitely be usable as a package.

My goal is this: to create this ID3 tagging framework (SwiftTaggerID3) as a package.

Then, create a similar package for MP4 files, which should be very easy, since almost all of it can be done in AVFoundation, and in fact I've got most of it done already. (I put it together during downtime when I needed to step away from this project to await an answer from you or let something rattle around in my brain for a while before I continue.)

Why? Because MP42Foundation is just too overbuilt for my purposes; I don't need something that complicated. And since almost all of what I do need can come from AVFoundation, there's no reason not to do that.

Mostly what SwiftTaggerMP4 will do is bridge the gap between the way SwiftTaggerID3 works, and the way AVFoundation works with mp4 files, so that I can use them together with minimal effort (basically what I was doing in AudiobookTaggerwith Id3TagEditor and Mp42Foundation.)

That way, instead of having to code things one way if it's an Mp4 and another way if it's an Mp3, they'll use identical methods and properties (such as wrapping up the more complex code to set a particular value in a tag.getterSetter). And a lot of metadata frames/atoms that exist in one but not in the other have a "pseudo-equivalent". For instance, if there's an MP4 atom that doesn't already have an ID3 equivalent, SwiftTaggerID3 will have a userDefinedText(description: whateverTheAtomIsCalled) frame, and vice versa.

So, when I go back to AudiobookTagger, I'm going to be using SwiftTaggerID3 and SwiftTaggerMP4 together. (or maybe just SwiftTagger, which would combine the two into a single package, the way your SDGCornerstone is a package of multiple packages? I'm not sure yet which would be best.)

Anyway, anyone else can use SwiftTaggerID3 or SwiftTaggerMP4 individually, depending on their needs, or combined, if they plan to support more than one kind of file. It won't be as all-encompassing as, say, TagLibKit, but it will be pure Swift.

So, since right now everything is building, and all tests are passing, I'll wrap this PR up and merge, migrate everything over to a package in another branch/PR, and then I'll resume working on making sure the tests cover all the bases.