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

Already on GitHub? Sign in to your account

session.image_create(track.album().cover()) gives SpotifyError: Image id length != 20 #102

Closed
mh-be opened this Issue Jun 9, 2013 · 10 comments

Comments

Projects
None yet
4 participants

mh-be commented Jun 9, 2013

For any album, any track.album()

As far as I tested, Album_cover in album.c (album().cover()) gives back the same value as sp_album_cover(self->_album, SP_IMAGE_SIZE_NORMAL)

mh-be commented Jun 9, 2013

session.image_create() works if I comment

// if (len != 20) {
// PyErr_SetString(SpotifyError, "Image id length != 20");
// return NULL;
// }

in session.c

Member

kingosticks commented Jun 9, 2013

And what value does sp_album_cover return for your album in question? Does it look valid (i.e. 20 bytes long)? Some albums don't have an image and return NULL, are you sure the albums you tested have images?

mh-be commented Jun 9, 2013

As stated, I have tested with multiple albums that have a cover. If I disable the check, the function is working. sp_album_cover returns the correct value (as does in turn the python function), but it seems that size_t len; doesn't give back 20. If you do

printf("len is : [%zu] [%d]",len,len);

you get

len is : [140591459467284] [20]

I've changed the code to

if ((int)len !=20) {

as workaround.

The full code beeing:

static PyObject *
Session_image_create(Session * self, PyObject *args)
{
    byte *image_id;
    size_t len;
    sp_image *image;

    if (!PyArg_ParseTuple(args, "s#", &image_id, &len))
        return NULL;

//    printf("len is : [%zu] [%d]\n",(int)len,(int)len);

    if (((int)len) != 20) {
        PyErr_SetString(SpotifyError, "Image id length != 20");
        return NULL;
    }
    image = sp_image_create(self->_session, image_id);
    return Image_FromSpotify(image);
}
Member

kingosticks commented Jun 9, 2013

Ah right, size_t is unsigned and "s#" refers to a signed integer, Good spot, this bug is 3 years old!

Owner

adamcik commented Jun 15, 2013

#104 sneaks in a fix for this casting to int.

Member

kingosticks commented Jun 16, 2013

Strictly speaking the variable len should probably just be of type int or we define the macro PY_SSIZE_T_CLEAN as documented here.

Owner

adamcik commented Jun 16, 2013

True, that's probably the more correct and long term fix. We can look into switching to that once #104 is in :-)

Member

kingosticks commented Jun 21, 2013

It also needs a test!

Owner

jodal commented Jul 30, 2013

#104 was merged more than a month ago, so the fields are green and open for a proper test and fix of this issue. nudge, nudge :-)

Owner

jodal commented Jan 15, 2015

Due to the imminent release of the 2.0 rewrite of pyspotify, this will probably never be done. Closing.

@jodal jodal closed this Jan 15, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment