Skip to content
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

Displaying filename while music metadata is being downloaded. #1361

Merged
merged 1 commit into from Apr 20, 2018

Conversation

3 participants
@codealchemist
Copy link
Contributor

commented Apr 19, 2018

Related to #1240.
Thanks @Borewit for all the hard work there!
Can you help testing this one?

@codealchemist

This comment has been minimized.

Copy link
Contributor Author

commented Apr 19, 2018

Player page while downloading metadata:
image

Player page when metadata is available:
image

@Borewit
Copy link
Member

left a comment

Great work!

I used this test torrent.

FLAC and MP3 are really working great.
It looks like Ogg less good, probably because it is less good in retrieving metadata from partial downloaded files. Once the incomplete metadata is cached, it does not get a change to be updated.
Nevertheless, this is a step forward.

If you use this test file, you will also understand why I wrote pull request #1334.

@Borewit

This comment has been minimized.

Copy link
Member

commented Apr 19, 2018

Off topic:, I wrote a spectrum analyzer component which looks like this:
spectrum
Possibly we could add that to the bottom of the music play page, would that fit, would that be of any added value you think?
Please be critical.

@codealchemist

This comment has been minimized.

Copy link
Contributor Author

commented Apr 19, 2018

Yeah, I just looked at the source on #1334, neat stuff getting the bestCover ;)
If we can merge this fix I can later merge master onto that branch and retest it.
That would be a good enhancement.

On the other hand, about the spectrum analyzer, while I think it's something pretty cool it might not make too much sense to have it integrated into WebTorrent.
I don't think it adds value to the app.
If you download an album and start listening to a song you might not stare at the spectrum while you listen, at least not for too long :D

I'd love to add a plugin system into WebTorrent, so all this features would be possible without affecting the core.

@DiegoRBaquero
Copy link
Member

left a comment

I like this

@Borewit

This comment has been minimized.

Copy link
Member

commented Apr 20, 2018

Thanks for your feedback!

If we can merge this fix I can later merge master onto that branch and retest it.

I merged all (master, this one (#1361) into the #1334 branch.

@codealchemist

This comment has been minimized.

Copy link
Contributor Author

commented Apr 20, 2018

Direct reference to the test magnet shared by @Borewit:

magnet:?xt=urn:btih:73f8847f7a2a6e283168b05f867aa40622adceae&dn=ParovPrincess&tr=http%3A%2F%2Fbt1.archive.org%3A6969%2Fannounce&tr=http%3A%2F%2Fbt2.archive.org%3A6969%2Fannounce&tr=wss%3A%2F%2Ftracker.btorrent.xyz&tr=wss%3A%2F%2Ftracker.fastcast.nz&tr=wss%3A%2F%2Ftracker.openwebtorrent.com&ws=http%3A%2F%2Fia600203.us.archive.org%2F29%2Fitems%2F&ws=http%3A%2F%2Fia800203.us.archive.org%2F29%2Fitems%2F&ws=https%3A%2F%2Farchive.org%2Fdownload%2F)

@codealchemist codealchemist merged commit 9906217 into master Apr 20, 2018

3 checks passed

Node Security No known vulnerabilities found
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@DiegoRBaquero DiegoRBaquero deleted the music-metadata-fix branch Apr 23, 2018

@Borewit Borewit removed the request for review from feross May 2, 2018

@lock lock bot locked as resolved and limited conversation to collaborators Jul 31, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.