Skip to content

populate library with real metadata#13

Merged
ungoldman merged 10 commits intomasterfrom
walkman
Aug 11, 2016
Merged

populate library with real metadata#13
ungoldman merged 10 commits intomasterfrom
walkman

Conversation

@ungoldman
Copy link
Copy Markdown
Member

resolves #11

First pass at getting real metadata in there. It's scanning whatever directory is set in config every time a new window gets opened for now, but it's real data!

Next step is getting it to rescan when state.config.music changes. Then maybe caching metadata more intelligently.

Comment thread renderer/lib/library.js Outdated

const extensions = ['.m4a', '.mp3']

module.exports = (libPath, send, done) => {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe i should replace send, done with a simple callback so this file isn't so tightly coupled with the model.

@bcomnes
Copy link
Copy Markdown
Contributor

bcomnes commented Aug 9, 2016

LGTM!

@bcomnes
Copy link
Copy Markdown
Contributor

bcomnes commented Aug 9, 2016

screenshot 2016-08-09 10 14 26

@ungoldman
Copy link
Copy Markdown
Member Author

@bcomnes I'm only using about 10 files to test now, guessing streaming every metadata update to state is too much.

@ungoldman
Copy link
Copy Markdown
Member Author

I'm also running into a really weird bug where one file in particular causes electron to completely freeze without any console error or anything.

@bcomnes
Copy link
Copy Markdown
Contributor

bcomnes commented Aug 9, 2016

I imagine there are going to be worlds of edge cases to deal with in this problem. Wrap in a setTimeout?

@ungoldman
Copy link
Copy Markdown
Member Author

@bcomnes sounds good for now, I'll try that.

@ungoldman
Copy link
Copy Markdown
Member Author

@bcomnes I tried putting setTimeout in a couple different places and didn't get great results. Gotta run for now, feel free to jump in.

@ungoldman
Copy link
Copy Markdown
Member Author

Another option is to try throttling the stream -- either that or batching metadata updates.

@bcomnes
Copy link
Copy Markdown
Contributor

bcomnes commented Aug 9, 2016

I have to work all day :(

@ungoldman
Copy link
Copy Markdown
Member Author

ungoldman commented Aug 9, 2016

I may have figured out what's causing a crash -- I think musicmetadata shoves the entire album art image in a buffer in the object it returns. I've got two files that are both causing a crash and they're the only two that have album art.

I bet choo chokes on diffing state because a giant image buffer has been shoved in there.

@ungoldman
Copy link
Copy Markdown
Member Author

Okay gotta go for real, I might get to this today or might not. Need to get ready for a visit from my mom tomorrow followed by road trip to Seattle on Friday and flight to Paris Saturday! Busy week!

@bcomnes
Copy link
Copy Markdown
Contributor

bcomnes commented Aug 9, 2016

Np. Nice work

@ungoldman
Copy link
Copy Markdown
Member Author

Fixed the buffer issue and tweaked a couple minor things. Haven't gotten to try batching or throttling to slow down state updates from the metadata stream yet. Going to merge this for now as it's a good stopping point.

@ungoldman ungoldman merged commit 33f8f4b into master Aug 11, 2016
@ungoldman ungoldman deleted the walkman branch August 11, 2016 23:01
@bcomnes
Copy link
Copy Markdown
Contributor

bcomnes commented Aug 11, 2016

Read through the whole Choo README and started looking through your work. Looks great! Go nate go!

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.

Populate music library with real metadata

2 participants