Relax record terminate char spec

[at-wat]

Allow single '\n' as a record termination since RFC 4566 says:

parsers SHOULD be tolerant and also accept records terminated with
a single newline character.

Allow to have empty lines.
Other SDP parsers (e.g. one in VLC media player) allow empty lines.

Reference issue

Fixes #44

[codecov]

Codecov Report

Merging #46 into master will increase coverage by 0.10%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##           master      #46      +/-   ##
==========================================
+ Coverage   63.98%   64.08%   +0.10%     
==========================================
  Files          11       11              
  Lines        1080     1086       +6     
==========================================
+ Hits          691      696       +5     
- Misses        297      298       +1     
  Partials       92       92              

Flag	Coverage Δ
#go	64.08% <66.66%> (+0.10%)	⬆️
#wasm	64.08% <66.66%> (+0.10%)	⬆️

Impacted Files	Coverage Δ
unmarshal.go	60.98% <ø> (+0.54%)	⬆️
util.go	65.89% <66.66%> (-1.17%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2fd3281...0bffc66. Read the comment docs.

[at-wat]

I'm not 100% sure, but I think TimeZone should be z=.

[trivigy]

Can you find the reference for this in the RFC? I am pretty sure it was r. Other than that the PR seems fine and the unittests are passing.

[at-wat]

https://tools.ietf.org/html/rfc4566#section-5.11
TimeZones field is z=, but in

sdp/unmarshal_test.go

Lines 240 to 253 in 2fd3281

	func TestUnmarshalTimeZones(t *testing.T) {
	sd := &SessionDescription{}
	if err := sd.Unmarshal([]byte(TimeZonesSDP)); err != nil {
	t.Errorf("error: %v", err)
	}
	actual, err := sd.Marshal()
	if got, want := err, error(nil); got != want {
	t.Fatalf("Marshal(): err=%v, want %v", got, want)
	}
	if string(actual) != TimeZonesSDPExpected {
	t.Errorf("error:\n\nEXPECTED:\n%v\nACTUAL:\n%v", TimeZonesSDPExpected, actual)
	}
	}

, z= is never appeared. I'm not very sure what was intended in this test originally.

[trivigy]

So looking quickly through the rfc it looks liker= in section 5.10 has to do with time repeating. I inplemented this stuff almost a year ago so I really don't remember. My vote is to not touch this in this PR if it is not durectly broken or breaks something specific for your usecase. It looks like this PR is related only to CRLF termination so I would keep it this way. Maybe separate this fix into a different PR.

[at-wat]

👍

[at-wat]

Oh, I touched this because I wanted to have a test case that ends with TimeZones and extra CRLF to cover s13's EOF handling.

[at-wat]

I'll add another const for now.

[at-wat]

Reverted TimeZonesSDP and added TimeZonesSDP2 for now.

[at-wat]

@trivigy reverted const TimeZonesSDP. Please take another look.

[trivigy]

LGTM