After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 650011 - [PATCH] Fix items disappearing in playlists on re-connect (64bits?)
[PATCH] Fix items disappearing in playlists on re-connect (64bits?)
Status: RESOLVED FIXED
Product: banshee
Classification: Other
Component: Device - iPod
git master
Other Linux
: Normal normal
: 1.x
Assigned To: Banshee Maintainers
Banshee Maintainers
: 650617 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2011-05-12 07:03 UTC by Chase
Modified: 2011-08-23 18:31 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch v1 (2.20 KB, patch)
2011-05-12 07:03 UTC, Chase
none Details | Review
Patch v2 (1.31 KB, patch)
2011-05-12 17:10 UTC, Chase
committed Details | Review

Description Chase 2011-05-12 07:03:24 UTC
Created attachment 187678 [details] [review]
Patch v1

Replication: Create a playlist on the device. Disconnect. On reconnect, the playlists won't fully populate in some cases.

Attached is a (hacky) patch to fix this. The issue arises in the fact that the AppleDeviceSource is using ExternalId for identifying tracks on the iPod. Unfortunately, gpod gives us ulongs, and the database is storing signed ints, so when we go to look for information given the ulong, we never find it if the id wraps to the negative numbers when interpreted as a signed number.

The patch simply stores and retrieves numbers based on Int64 values instead of longs. This might not be portable to 32-bit systems? Not sure how the INTEGER type in sqlite3 maps to 32-bit systems---is it still 64-bit or does it go to 32-bit?

There's probably a more elegant way of doing this---if you know of a particular way you would want it done, let me know and I'll code it up. Maybe we should be mapping track.DBID (gpod track id) to smallish integers we can use for storing instead of this casting business?
Comment 1 Andrés G. Aragoneses (IRC: knocte) 2011-05-12 08:31:08 UTC
Thanks for looking into this!

I seem to recall that there were some issues with the binding that Alan fixed recently or was working on fixing. Did you test first with the master branch of libgpod-sharp? Maybe it has been fixed there already.
Comment 2 Chase 2011-05-12 17:10:52 UTC
Created attachment 187725 [details] [review]
Patch v2

I looked at the tree for libgpod-sharp and the DBIDs are still ulongs, so we'd still have this issue with the latest master from there.

But after looking into this a bit more, I think it's much simpler than I initially thought---I think there was just a missing cast. (The plugin is casting the track.DBID (a ulong) to a long for storage in the database. When querying for playlist generation, it wasn't casting the ulong in the query generation.)

Here's a new patch that adds in the missing cast (simple one-liner).
Comment 3 Alan McGovern 2011-05-19 12:25:02 UTC
Good catch on this!

By any chance would it be possible to change DBID itself to be of type 'long'. If not, is there another property we could use which would be safer? Basically, anywhere we use this property we can potentially forget to cast it and if we don't cast it we run the risk of hitting the same bug again. If we can change the property itself to be an int64 (long) then we can prevent it from ever recurring.

If that's not possible, could you stick a comment in the code explaining why the cast is needed so future developers know it's really important :) Once that's done we can commit it.

Thanks for tracking this down!
Comment 4 Andrés G. Aragoneses (IRC: knocte) 2011-05-19 19:26:24 UTC
*** Bug 650617 has been marked as a duplicate of this bug. ***
Comment 5 Doug Rintoul 2011-05-20 19:31:55 UTC
Changing track.DBID to a long would have to be done in libgpod-sharp. This could have ramifications to all the projects using libgpod including gtkpod, banshee, amarok, exaile, rhythmbox and others. I don't think it is practical nor warranted. SQLite does not have an unsigned integer data type, so I think the only option is to cast the ulong to a long.
Comment 6 Andrés G. Aragoneses (IRC: knocte) 2011-05-25 17:20:45 UTC
(In reply to comment #5)
> Changing track.DBID to a long would have to be done in libgpod-sharp. This

Ok.

> could have ramifications to all the projects using libgpod including gtkpod,

libgpod and libgpod-sharp are two different things. The latter is the C# binding to the libgpod library.

> banshee, amarok, exaile, rhythmbox and others. I don't think it is practical

AFAIK, banshee is the only piece of software that uses libgpod-sharp. So, could the type be changed in the binding? Alan?

> nor warranted. SQLite does not have an unsigned integer data type, so I think
> the only option is to cast the ulong to a long.

SQLite? I don't think libgpod or libgpod-sharp uses SQLite at all. Please correct me if I'm wrong.
Comment 7 Doug Rintoul 2011-05-25 22:04:04 UTC
>> Changing track.DBID to a long would have to be done in libgpod-sharp. This
> Ok.
>
>> could have ramifications to all the projects using libgpod including gtkpod,
> libgpod and libgpod-sharp are two different things. The latter is the C#
> binding to the libgpod library.
I understand that. The bindings are part of the libgpod source. Do not the bindings reflect the libgpod structure and data types? Why change the binding because a downstream application doesn't want to do a cast? A cast is going to have to be done somewhere, unless the original libgpod DBID is signed, in which case the binding does not reflect the original library.
>
>> banshee, amarok, exaile, rhythmbox and others. I don't think it is practical
> AFAIK, banshee is the only piece of software that uses libgpod-sharp. So, could
> the type be changed in the binding? Alan?
This would be okay as long as changing the binding does not affect libgpod proper.
>
>> nor warranted. SQLite does not have an unsigned integer data type, so I think
>> the only option is to cast the ulong to a long.
> SQLite? I don't think libgpod or libgpod-sharp uses SQLite at all. Please
> correct me if I'm wrong.
>
libgpod-sharp does not use SQLite; banshee does. The track.DBID retrieved via libgpod-sharp is stored in the banshee database. Since the track.DBID is ulong and sqlite does not have an unsigned int datatype, the track.DBID has to be cast to a long.

Doug.
Comment 8 Andrés G. Aragoneses (IRC: knocte) 2011-05-26 13:51:59 UTC
(In reply to comment #7)
> >> Changing track.DBID to a long would have to be done in libgpod-sharp. This
> > Ok.
> >
> >> could have ramifications to all the projects using libgpod including gtkpod,
> > libgpod and libgpod-sharp are two different things. The latter is the C#
> > binding to the libgpod library.
> I understand that. The bindings are part of the libgpod source. Do not the

I don't know if the sources of the binding are on the same source repository, but AFAIK they are packaged separatedly.

> bindings reflect the libgpod structure and data types? Why change the binding
> because a downstream application doesn't want to do a cast? 

I was thinking that maybe libgpod was using "long", while the binding was exposing "ulong" wrongly. If this was the case, a fix just in the binding would be the most desirable, right?

> A cast is going to have to be done somewhere, unless the original libgpod DBID is signed, in which
> case the binding does not reflect the original library.
> >
> >> banshee, amarok, exaile, rhythmbox and others. I don't think it is practical
> > AFAIK, banshee is the only piece of software that uses libgpod-sharp. So, could
> > the type be changed in the binding? Alan?
> This would be okay as long as changing the binding does not affect libgpod
> proper.
> >
> >> nor warranted. SQLite does not have an unsigned integer data type, so I think
> >> the only option is to cast the ulong to a long.
> > SQLite? I don't think libgpod or libgpod-sharp uses SQLite at all. Please
> > correct me if I'm wrong.
> >
> libgpod-sharp does not use SQLite; banshee does. The track.DBID retrieved via
> libgpod-sharp is stored in the banshee database. Since the track.DBID is ulong
> and sqlite does not have an unsigned int datatype, the track.DBID has to be
> cast to a long.

I see. I wonder then if we should improve the ServiceManager.DbConnection.Execute() method to error out whenever it is called a param whose type is ulong. This way we would catch these bugs faster.
Comment 9 Bertrand Lorentz 2011-08-23 18:30:46 UTC
Review of attachment 187725 [details] [review]:

Although I don't have an iPod to test this, it looks harmless enough, so I've committed it to git master, along with an additional comment.

Thanks for the patch !
Comment 10 Bertrand Lorentz 2011-08-23 18:31:20 UTC
This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.