Skip to content

Go test for jfreymuth/oggvorbis fails #3454

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
sago35 opened this issue Feb 16, 2023 · 6 comments
Closed

Go test for jfreymuth/oggvorbis fails #3454

sago35 opened this issue Feb 16, 2023 · 6 comments

Comments

@sago35
Copy link
Member

sago35 commented Feb 16, 2023

The test was successful for tinygo 0.25.
However, TinyGo 0.26 or later fails.

This is caused by oggvorbis.NewReader() returning an error.
This prevents normal use as well as testing.
It was working in TinyGo 0.25.

https://github.com/jfreymuth/oggvorbis

$ /c/tinygo/tinygo0.25.0.windows-amd64/tinygo/bin/tinygo test .
ld.lld: warning: duplicate /export option: hypot
ld.lld: warning: duplicate /export option: nextafter
ok      github.com/jfreymuth/oggvorbis  0.121s
$ /c/tinygo/tinygo0.26.0.windows-amd64/tinygo/bin/tinygo test .
ld.lld: warning: duplicate /export option: hypot
ld.lld: warning: duplicate /export option: nextafter
panic: runtime error: nil pointer dereference
FAIL    github.com/jfreymuth/oggvorbis  0.186s
FAIL
$ /c/tinygo/tinygo0.27.0.windows-amd64/tinygo/bin/tinygo test .
FAIL    github.com/jfreymuth/oggvorbis  0.209s
$ go test .
ok      github.com/jfreymuth/oggvorbis  (cached)
@dgryski
Copy link
Member

dgryski commented Feb 16, 2023

Weird that it started failing with newer versions. I'll take a look and investigate.

@dgryski
Copy link
Member

dgryski commented Feb 16, 2023

First pass investigation might blame the Seek() system calls. Leaving for now..

@aykevl
Copy link
Member

aykevl commented Feb 16, 2023

Ok, so the nil pointer panic is caused because oggvorbis.NewReader (in TestLengthForUnexpectedEof) returns an error, this is logged using t.Fatal, but we don't actually stop at t.Fatal. I didn't investigate further.

@dgryski
Copy link
Member

dgryski commented Feb 17, 2023

Yeah, the issue though is that NewReader shouldn't be returning an error in the middle of the test, and my thought is because the Seek() behaviour to rewind the ogg stream to the beginning isn't working, so the packet decoding step fails.

@dgryski
Copy link
Member

dgryski commented Mar 1, 2023

This is a bug in the upstream floating point code: #3482 (comment)

@dgryski
Copy link
Member

dgryski commented Mar 1, 2023

Filed jfreymuth/vorbis#11

@dgryski dgryski closed this as completed Mar 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants