Fix UnsynchTextFrame byte format

- change encodedBytes.Reader's ReadNullTermString behavior to strip off
  null byte(s)
- add WriteNullTermString function to encodedBytes.Writer
- include NullLength information in encodingbytes.EncodingMap
- increase size of UnsynchTextFrame by the null length of the encoding
- automatically null-terminate description string in
  UnsynchTextFrame.Bytes()

encodedbytes/reader.go

@@ -71,12 +71,13 @@ func (r *Reader) ReadRestString(encoding byte) (string, error) {
 
 // Read a null terminated string of specified encoding
 func (r *Reader) ReadNullTermString(encoding byte) (string, error) {
-	b, err := r.ReadNumBytes(afterNullIndex(r.data[r.index:], encoding))
+	atIndex, afterIndex := nullIndex(r.data[r.index:], encoding)
+	b, err := r.ReadNumBytes(afterIndex)
 	if err != nil {
 		return "", err
 	}
 
-	return Decoders[encoding].ConvertString(string(b))
+	return Decoders[encoding].ConvertString(string(b[:atIndex]))
 }
 
 func NewReader(b []byte) *Reader { return &Reader{b, 0} }

encodedbytes/util.go

@@ -14,15 +14,19 @@ const (
 	SynchByteLength = 7
 	NormByteLength  = 8
 	NativeEncoding  = 3
-	UTF16NullLength = 2
 )
 
+type Encoding struct {
+	Name       string
+	NullLength int
+}
+
 var (
-	EncodingMap = [...]string{
-		"ISO-8859-1",
-		"UTF-16",
-		"UTF-16BE",
-		"UTF-8",
+	EncodingMap = [...]Encoding{
+		{Name: "ISO-8859-1", NullLength: 1},
+		{Name: "UTF-16", NullLength: 2},
+		{Name: "UTF-16BE", NullLength: 2},
+		{Name: "UTF-8", NullLength: 1},
 	}
 	Decoders = make([]*iconv.Converter, len(EncodingMap))
 	Encoders = make([]*iconv.Converter, len(EncodingMap))
@@ -31,8 +35,8 @@ var (
 func init() {
 	n := EncodingForIndex(NativeEncoding)
 	for i, e := range EncodingMap {
-		Decoders[i], _ = iconv.NewConverter(e, n)
-		Encoders[i], _ = iconv.NewConverter(n, e)
+		Decoders[i], _ = iconv.NewConverter(e.Name, n)
+		Encoders[i], _ = iconv.NewConverter(n, e.Name)
 	}
 }
 
@@ -92,12 +96,21 @@ func EncodingForIndex(b byte) string {
 		encodingIndex = 0
 	}
 
-	return EncodingMap[encodingIndex]
+	return EncodingMap[encodingIndex].Name
+}
+
+func EncodingNullLengthForIndex(b byte) int {
+	encodingIndex := int(b)
+	if encodingIndex < 0 || encodingIndex > len(EncodingMap) {
+		encodingIndex = 0
+	}
+
+	return EncodingMap[encodingIndex].NullLength
 }
 
 func IndexForEncoding(e string) byte {
 	for i, v := range EncodingMap {
-		if v == e {
+		if v.Name == e {
 			return byte(i)
 		}
 	}
@@ -105,28 +118,23 @@ func IndexForEncoding(e string) byte {
 	return 0
 }
 
-func afterNullIndex(data []byte, encoding byte) int {
-	encodingString := EncodingForIndex(encoding)
+func nullIndex(data []byte, encoding byte) (atIndex, afterIndex int) {
+	byteCount := EncodingNullLengthForIndex(encoding)
+	limit := len(data)
+	null := bytes.Repeat([]byte{0x0}, byteCount)
 
-	if encodingString == "UTF-16" || encodingString == "UTF-16BE" {
-		limit, byteCount := len(data), UTF16NullLength
-		null := bytes.Repeat([]byte{0x0}, byteCount)
+	for i, _ := range data[:limit/byteCount] {
+		atIndex = byteCount * i
+		afterIndex = atIndex + byteCount
 
-		for i, _ := range data[:limit/byteCount] {
-			atIndex := byteCount * i
-			afterIndex := atIndex + byteCount
-
-			if bytes.Equal(data[atIndex:afterIndex], null) {
-				return afterIndex
-			}
-		}
-	} else {
-		if index := bytes.IndexByte(data, 0x00); index >= 0 {
-			return index + 1
+		if bytes.Equal(data[atIndex:afterIndex], null) {
+			return
 		}
 	}
 
-	return -1
+	atIndex = -1
+	afterIndex = -1
+	return
 }
 
 func EncodedDiff(ea byte, a string, eb byte, b string) (int, error) {

encodedbytes/writer.go

@@ -48,4 +48,12 @@ func (w *Writer) WriteString(s string, encoding byte) (err error) {
 	return
 }
 
+func (w *Writer) WriteNullTermString(s string, encoding byte) (err error) {
+	err = w.WriteString(s, encoding)
+	if err == nil {
+		err = w.WriteByte(0)
+	}
+	return
+}
+
 func NewWriter(b []byte) *Writer { return &Writer{b, 0} }

id3_test.go

@@ -7,6 +7,7 @@ import (
 	"bytes"
 	v2 "github.com/mikkyang/id3-go/v2"
 	"io/ioutil"
+	"os"
 	"testing"
 )
 
@@ -72,3 +73,50 @@ func TestClose(t *testing.T) {
 		t.Errorf("Close: unable to write original contents to test file")
 	}
 }
+
+func TestUnsynchTextFrame_RoundTrip(t *testing.T) {
+	var (
+		err              error
+		tempfile         *os.File
+		f                *File
+		tagger           *v2.Tag
+		ft               v2.FrameType
+		utextFrame       *v2.UnsynchTextFrame
+		parsedFrame      v2.Framer
+		resultFrame      *v2.UnsynchTextFrame
+		ok               bool
+		expected, actual string
+	)
+
+	tempfile, err = ioutil.TempFile("", "id3v2")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	tagger = v2.NewTag(3)
+	ft = v2.V23FrameTypeMap["COMM"]
+	utextFrame = v2.NewUnsynchTextFrame(ft, "Comment", "Foo")
+	tagger.AddFrames(utextFrame)
+
+	_, err = tempfile.Write(tagger.Bytes())
+	tempfile.Close()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	f, err = Open(tempfile.Name())
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	parsedFrame = f.Frame("COMM")
+	if resultFrame, ok = parsedFrame.(*v2.UnsynchTextFrame); !ok {
+		t.Error("Couldn't cast frame")
+	} else {
+		expected = utextFrame.Description()
+		actual = resultFrame.Description()
+		if expected != actual {
+			t.Errorf("Expected %q, got %q", expected, actual)
+		}
+	}
+}

v2/frame.go

@@ -399,6 +399,9 @@ func NewUnsynchTextFrame(ft FrameType, desc, text string) *UnsynchTextFrame {
 	f := NewDescTextFrame(ft, desc, text)
 	f.size += uint32(3)
 
+	// add null length for this encoding
+	f.size += uint32(encodedbytes.EncodingNullLengthForIndex(f.encoding))
+
 	return &UnsynchTextFrame{
 		DescTextFrame: *f,
 		language:      "eng",
@@ -443,6 +446,19 @@ func (f *UnsynchTextFrame) SetLanguage(language string) error {
 	return nil
 }
 
+func (f *UnsynchTextFrame) SetEncoding(encoding string) error {
+	prevIndex := f.encoding
+	err := f.DescTextFrame.SetEncoding(encoding)
+	if err != nil {
+		return err
+	}
+
+	n1 := encodedbytes.EncodingNullLengthForIndex(prevIndex)
+	n2 := encodedbytes.EncodingNullLengthForIndex(f.encoding)
+	f.changeSize(n2 - n1)
+	return nil
+}
+
 func (f UnsynchTextFrame) String() string {
 	return fmt.Sprintf("%s\t%s:\n%s", f.language, f.description, f.text)
 }
@@ -460,7 +476,7 @@ func (f UnsynchTextFrame) Bytes() []byte {
 		return bytes
 	}
 
-	if err = wr.WriteString(f.description, f.encoding); err != nil {
+	if err = wr.WriteNullTermString(f.description, f.encoding); err != nil {
 		return bytes
 	}
 

v2/frame_test.go

@@ -0,0 +1,29 @@
+package v2
+
+import (
+	"testing"
+)
+
+func TestUnsynchTextFrame_SetEncoding(t *testing.T) {
+	f := NewUnsychTextFrame(V23CommonFrame["Comments"], "Foo", "Bar")
+	size := f.Size()
+
+	err := f.SetEncoding("UTF-16")
+	if err != nil {
+		t.Fatal(err)
+	}
+	newSize := f.Size()
+	if newSize-size != 1 {
+		t.Errorf("expected size to increase to %d, but it was %d", size+1, newSize)
+	}
+
+	size = newSize
+	err := f.SetEncoding("UTF-16")
+	if err != nil {
+		t.Fatal(err)
+	}
+	newSize := f.Size()
+	if newSize-size != -1 {
+		t.Errorf("expected size to decrease to %d, but it was %d", size-1, newSize)
+	}
+}