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

Integration with music-metadata-browser #673

Closed
wants to merge 47 commits into from

Conversation

Borewit
Copy link
Contributor

@Borewit Borewit commented Oct 6, 2018

Using Web Fetch API, thanks for the idea @durasj.#606

This is a continuation of the branch of close PR #606. I wanted to know how music-metadata-browser behaved in WebAmp.

After PR #606, I opened #634 (making use of the tag event updates while parsing). Although I believe that is a nicer solution on the long run, I think music-metadata has to be merged in first with the least amount of change to avoid PR's have to be maintained forever.

Another difference is that this is build music-metadata-browser and no longer directly on music-metadata. music-metadata-browser is tailered to the needs of JavaScript module based browser applications and driven by the requirements of WebAmp.

So this PR is back to basics.

  1. This change should improve media formation information as displayed by WebAmp:
    1. Set the sample-rate to actual sample-rate (currently faked)
    2. Set the bitrate to actual bit-rate (currently faked)
    3. Duration retrieved from audio metadata, and not the Audio-element
  2. Improve / fix the HTTP-retrieval (jsmediatags has issues retrieving metadata caused by XHR pre-flight requests).

Preview: 🚀 Music-metadata powered WebAmp

…/*`.

The relative URL used for 'Llama DJ Mike Llama' results in a MIME-type: `audio/mpeg; charset=UTF-8`.
Pass MIME-type parameters to music-metadata.
# Conflicts:
#	js/actionCreators/files.js
@durasj
Copy link
Collaborator

durasj commented Oct 6, 2018

Cool stuff @Borewit! Seems like the duration is a bit off on the songs though. Also, unfortunately, Firefox will download the whole song.

@Borewit
Copy link
Contributor Author

Borewit commented Oct 7, 2018

Seems like the duration is a bit off on the songs though.

Can you give me an example @durasj? Feel free to open an issue.

Firefox will download the whole song.

Indeed, apperently the response.body, which allows to the partial stream, is not implemented by Firefox.

@durasj
Copy link
Collaborator

durasj commented Oct 8, 2018

So I've checked it again and it seems to be the duration of the current song that's wrong. See gif:
winamp current song duration issue
The LLama Intro has duration of 5m in the main window. It seems to be the one from the second song. Then you can see also other songs have that duration of 5m

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

Successfully merging this pull request may close these issues.

3 participants