sp_album_cover implemented

[swissmanu]

hi florent
following pull request contains the implementation of sp_album_cover for the album object.

it adds the following methods to the album object:

• coverImage(cb)
• smallCoverImage(cb)
• largeCoverImage(cb)

each of them expects a function callback which will be called as soon as the cover image is loaded. the images raw data will be contained in the first function argument, represented inside a Buffer:

album.coverImage(function(buffer) {
    if(buffer.length > 0) {
        fs.writeFileSync('cover.jpg', buffer);
    } else {
        console.log('image was not available');
    }
});

a unit test is included and should run successfully. further i allowed myself to update your package.json with the minimum requirement of node.js 0.10.0 since node-libspotify needs the new stream implementations.

please let me know if you want some modifications/improvements on the code. otherwise i'd be happy if you can use my addition to bring node-libspotify one step closer to completion.

cheers,
manu

[Floby]

Thank you very much for contributing. I'll run this on my installation when
I'm on computer. Hopefully this will find its way to master :)

Florent
Le 3 sept. 2013 21:47, "Manuel Alabor" notifications@github.com a écrit :

hi florent
following pull request contains the implementation of sp_album_cover for
the album object.

it adds the following methods to the album object:

• coverImage(cb)
• smallCoverImage(cb)
• largeCoverImage(cb)

each of them expects a function callback which will be called as soon as
the cover image is loaded. the images raw data will be contained in the
first function argument, represented inside a Buffer:

album.coverImage(function(buffer) {
if(buffer.length > 0) {
fs.writeFileSync('cover.jpg', buffer);
} else {
console.log('image was not available');
}
});

a unit test is included and should run successfully. further i allowed
myself to update your package.json with the minimum requirement of node.js
0.10.0 since node-libspotify needs the new stream implementations.

please let me know if you want some modifications/improvements on the
code. otherwise i'd be happy if you can use my addition to bring
node-libspotify one step closer to completion.

cheers,

manu

You can merge this Pull Request by running

git pull https://github.com/swissmanu/node-libspotify album-cover

Or view, comment on, or merge it at:

#24
Commit Summary

• sp_album_cover implemented

File Changes

• M lib/Album.jshttps://github.com/sp_album_cover implemented #24/files#diff-0(28)
• M package.jsonhttps://github.com/sp_album_cover implemented #24/files#diff-1(4)
• M src/album.cchttps://github.com/sp_album_cover implemented #24/files#diff-2(70)
• M test/test-031-album.jshttps://github.com/sp_album_cover implemented #24/files#diff-3(39)

Patch Links:

• https://github.com/Floby/node-libspotify/pull/24.patch
• https://github.com/Floby/node-libspotify/pull/24.diff

[Floby]

So ive been reading in more detail.

the doc says about images "If an image is loaded, and loading fails, the image will behave like an empty image."
I wonder what this implies for us. Does it return a zero-length buffer ?

[swissmanu]

my first intention was that the buffer is empty, yes. (implemented it this way in my project https://github.com/swissmanu/kaffeeundkuchen/blob/master/src/api/coverimage.js#L42)
but rethinking it now, it could absolutly be possible that jpeg raw data without any pixel information could be contained in the buffer.

in that case we wouldn't have any proper chance to check if the image was actually loaded or not i guess.

[Floby]

I don't know. Should we consider an unloaded image as an error and treat it as such ?
libspotify seems to consider that empty images are valid cover images.

[swissmanu]

if we'd reflect libspotifys own behaviour, i guess we should not.
but if we get the chance to improve an existing api, i'm always in ;)

what's your opinion on extending coverImage()s callback to something like the following:

album.coverImage(function(buffer, success) {
    if(success) {
        // process buffer
    } else {
        console.log('image is not available or an error occured during loading it');
    }
});

success represents (buffer.length > 0) directly to make the failure situation more understandable (and transparent) to the developer.

-- UPDATE --
another, more "nodejs" way would be:

    album.coverImage(function(error, buffer) { /* ... */ });

error would contain a javascript error object if empty data was returned by libspotify. if everything worked fine, error is undefined and buffer would contain the actual data.

[LinusU]

Please, please, please use the "more nodejs way", e.g. function (err, buffer) { /* ... */ }.