Skip to content

iPod support restored using ctypes and libgpod #1289

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.

Sign up for GitHub

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

Jump to bottom
Merged
auouymous merged 4 commits into from
Jun 3, 2022
Merged

iPod support restored using ctypes and libgpod #1289

auouymous merged 4 commits into from
Jun 3, 2022

Conversation

@thp
Copy link
Member

@thp thp commented May 3, 2022

Debian/Ubuntu haven't built the Python bindings for libgpod for a while now (presumably as part of the Python 2 -> Python 3 transition, but I haven't investigated more than this):


libgpod (0.8.3-14) unstable; urgency=medium

...

  [ Andrey Rahmatullin ]
  * Drop the Python subpackage.

 -- Andrey Rahmatullin <wrar@debian.org>  Wed, 21 Aug 2019 22:31:15 +0500

This leaves users without an "easy" way to use iPod sync support, even on Linux.

Apart from that, I noticed that even with python-gpod installed, the iPod sync support is probably just broken at the moment.

This patch re-introduces "classic" iPod support using libgpod (which is still available in distros) directly using Python's ctypes FFI module to interact with the C library. It also cleans up some things in src/gpodder/sync.py that were unused/dead code.

This actually does more than previous incarnations of iPod support in gPodder ever did:

  • "New" status and bookmark position (resume point) on iPod is restored from current_position of the episode
  • On the next sync, updated bookmark values from the iPod are synchronized to the episode's current_position

This means that when listening to an episode on an iPod, and later syncing, the playback position (bookmark) is reflected in gPodder's UI. Deleting episodes on the iPod that have been deleted in gPodder has also been tested and works.

The other great thing about this change is, if we ever get to build libgpod and its dependencies for Windows and/or macOS, iPod syncing would work for those devices as well (libgpod is basically just writing files on the filesystem for "classic" iPods, nothing magical there).

I tried to be careful not to introduce use-after-free and memory leak bugs, and tried to document the pitfalls I ran into, but there might still be some issues lurking (valgrind is mostly happy, though). A "proper" C extension module for Python might be the way to go, but this ctypes-based solution doesn't require development headers or even a compiler (and trying to get the SWIG-based Python bindings re-enabled in Linux distros might be more effort than what it's worth).

@auouymous
Copy link
Member

auouymous commented May 4, 2022

I'm still reviewing this but you used at least one f-string which isn't supported in 3.5.

auouymous
auouymous requested changes May 8, 2022
View reviewed changes
Copy link
Member

@auouymous auouymous left a comment

Choose a reason for hiding this comment

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

Debian/Ubuntu haven't built the Python bindings for libgpod for a while now (presumably as part of the Python 2 -> Python 3 transition, but I haven't investigated more than this):

This comment from the Gentoo ebuild for gpodder suggests the transition is the reason.

# As in Fedora: re-enable dev-python/eyeD3 and
# ipod? ( media-libs/libgpod ) once they support python3

@thp thp mentioned this pull request May 17, 2022
Closed
@thp
Copy link
Member Author

thp commented May 20, 2022

This PR in CPython: python/cpython#92870 should take care of exposing time_t properly, as it depends on OS, compiler used and other factors whether time_t is 64-bit or 32-bit on 32-bit systems (see python/cpython#92869 for detailed information what I could have gathered -- e.g. OpenBSD and NetBSD use 64-bit time_t also on 32-bit, but FreeBSD on 32-bit i386 uses 32-bit time_t). For now, the current check is probably "good enough" to get it working on 64-bit and 32-bit Linux systems, and if c_time_t becomes part of Python's ctypes, we can use that eventually.

@auouymous auouymous merged commit a972615 into master Jun 3, 2022
@auouymous auouymous deleted the ipod-support branch June 3, 2022 00:41
@auouymous
Copy link
Member

auouymous commented Jun 3, 2022

Merged, thanks for the patch.

@thp thp mentioned this pull request Jul 12, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@auouymous auouymous auouymous requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@thp @auouymous