Conversation
tillt
left a comment
tillt
left a comment
There was a problem hiding this comment.
Great idea adding support for these. Am of no authority but I would definitely try to add some tests and maybe even more validation of this doing its job nicely.
| @@ -1,4 +1,4 @@ | |||
| module github.com/bogem/id3v2 | |||
| module github.com/tramhao/id3v2 | |||
| import ( | ||
| // "bytes" | ||
| "encoding/binary" | ||
| // "fmt" |
There was a problem hiding this comment.
These shouldn't be left in here.
n10v
left a comment
n10v
left a comment
There was a problem hiding this comment.
Thanks a lot for adding this PR and super sorry for taking so much time to submit my review. I added comments a year ago, but forgot to submit the review 🤦🤦🤦🤦🤦🤦
| package id3v2 | ||
|
|
||
| import ( | ||
| // "bytes" |
| import ( | ||
| // "bytes" | ||
| "encoding/binary" | ||
| // "fmt" |
| @@ -1,4 +1,4 @@ | |||
| module github.com/bogem/id3v2 | |||
| module github.com/tramhao/id3v2 | |||
| type SynchronisedLyricsFrame struct { | ||
| Encoding Encoding | ||
| Language string | ||
| TimestampFormat byte |
There was a problem hiding this comment.
| TimestampFormat byte | |
| TimestampFormat TimestampFormat |
| type TimestampFormat int | ||
|
|
||
| const ( | ||
| Unknown TimestampFormat = iota |
There was a problem hiding this comment.
IMO TimestampFormat is redundant here. Just use byte
| Unknown TimestampFormat = iota | |
| Unknown byte = iota |
There was a problem hiding this comment.
Please also prefix timestamp formats. Please also get rid of Unknown, I can't find such TimestampFormat in spec
const (
SYLTAbsoluteMpegFramesTimestampFormat = iota + 1
SYLTAbsoluteMillisecondsTimestampFormat
)| 7: "WebpageUrls", | ||
| 8: "ImageUrls", | ||
| } | ||
| ) |
There was a problem hiding this comment.
Please also use iota here:
const (
SYLTOtherContentType = iota
SYLTLyricsContentType
SYLTTextTranscriptionContentType
SYLTMovementContentType
SYLTEventsContentType
SYLTChordContentType
SYLTTriviaContentType
SYLTWebpageURLsContentType
SYLTImageURLsContentType
)
3 participants
According to id3v2 sylt specification, add the sylt support.