Podcast support #54

fracai · 2021-08-28T02:58:23Z

Adds podcast support. This consists of treating tracks specially if they're found in the iPod_Control/Podcasts directory or if metadata contains "Podcast" in the genre list. Identified podcast tracks are excluded from the "All Songs" playlist and skipped when shuffling. In support of this, a Podcasts directory will be created when the script is run and symlinks will be followed to support creating playlists (this will aid regular music tracks as well).

NicoHood · 2021-08-28T09:55:01Z

ipod-shuffle-4g.py

@@ -408,7 +423,7 @@ def __init__(self, parent):
                          ("header_id", ("4s", b"hphs")), #shph
                          ("total_length", ("I", 0)),
                          ("number_of_playlists", ("I", 0)),
-                          ("number_of_non_podcast_lists", ("2s", b"\xFF\xFF")),
+                          ("number_of_non_podcast_lists", ("H", 65535)),


Why did you change this?

At the time, because I assumed I needed to specify that it was an unsigned short instead of two bytes, but I suppose they're equivalent. I do think it's useful to indicate the actual type now that it'll change values.

To reduce the noise I would revert this change

I just tested and actually can't revert this. Packing the structure to build the database throws an exception because the struct has a short but is expecting bytes.

Traceback (most recent call last): File ".../IPod-Shuffle-4g/./ipod-shuffle-4g.py", line 810, in <module> shuffle.write_database() File ".../IPod-Shuffle-4g/./ipod-shuffle-4g.py", line 675, in write_database f.write(self.tunessd.construct()) File ".../IPod-Shuffle-4g/./ipod-shuffle-4g.py", line 293, in construct play_header = self.play_header.construct(self.track_header.tracks) File ".../IPod-Shuffle-4g/./ipod-shuffle-4g.py", line 463, in construct output = Record.construct(self) File ".../IPod-Shuffle-4g/./ipod-shuffle-4g.py", line 215, in construct output += struct.pack("<" + fmt, self._fields.get(i, default)) struct.error: argument for 's' must be a bytes object

Indeed H seems correct here. Can you give it the value 0xFFFF?

I am also wondering if a non-podcasts count makes sense. I guess a podcast count makes more sense, but that is what the docs from reverse engineering say. But maybe those are wrong!?

Technically, ("H", b"\xFF\xFF") would be accepted, but I think that just looks confusing, given that it's representing a number.

I also don't think I'm going to question the docs. If it was actually a "podcast count", 0 would make more sense for the case when there aren't any podcast playlists.

0xFFFF is more clear than the number. Maybe it is also a signed short, which would be -1.

fracai · 2021-08-29T14:22:38Z

I added readme mentions regarding podcast and symlink support.
And also marked podcast tracks as remembering their playback position.

README.md

aoss-doeice · 2021-08-31T13:02:34Z

Could you please remove me from this project? Thanks, lvN

…

On Tue, Aug 31, 2021 at 2:10 AM NicoHood ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In README.md <#54 (comment)>: > +#### Use symlinks to create playlists +Symlinks will be followed to their original location on the iPod Shuffle. This allows creating additional playlists without duplicating physical files or duplicate entries in the iTunesSD database. In your case I'd create a dedicated m3u playlist and I would not frickel with symlinks. That does not make sense to me. It would also have all other benefits you mentioned. Apart from the symlink discussion I dont see why this is related to podcast. I understand that a song might be in multiple playlists, but a podcast? I am not sure if your solution accepts m3u files for podcasts or the autogenerated playlists using the folders, but if it does not, you should fix that in my opinion. So an easy solution is always better than something complex. I'd be happy if you can give it another cleanup round :-) — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#54 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABNDMDJDPJYL7JRZZE5E6F3T7RW5RANCNFSM5C6UIHSA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

NicoHood · 2021-08-31T13:54:55Z

@aoss-doeice You have subscribed for the notifications yourself. Only you can unsubscribe - we cannot unsubscribe you.

ipod-shuffle-4g.py

NicoHood · 2021-09-01T08:32:56Z

ipod-shuffle-4g.py

+            self["dontskip"] = 0
+            self["remember"] = 1


Could you please add more explanation what this does?

update incoming

NicoHood · 2021-09-01T08:36:59Z

README.md

@@ -93,6 +93,9 @@ The file can be found in the [extras](extras) folder.
 #### Compress/Convert your music files
 ([#11](https://github.com/nims11/IPod-Shuffle-4g/issues/11)) Shuffle is short on storage, and you might want to squeeze in more of your collection by sacrificing some bitrate off your files. In rarer cases, you might also possess music in formats not supported by your ipod. Although `ffmpeg` can handle almost all your needs, if you are looking for a friendly alternative, try [Soundconverter](http://soundconverter.org/).

+#### Podcast support
+Place podcast tracks in `iPod_Control/Podcasts` to generate playlists. These tracks will be skipped when shuffling, will be marked to remember their last playback position, and won't be included in the "All Songs" playlist.


Or mark then as genre id3 tag "Podcast"?

right, thanks

ipod-shuffle-4g.py

NicoHood · 2021-09-01T08:47:57Z

ipod-shuffle-4g.py

@@ -562,6 +589,9 @@ def construct(self, tracks):
        for i in self.listtracks:
            path = self.ipod_to_path(i)
            position = -1
+            if self["listtype"] == 1 and "/iPod_Control/Podcasts/" in path:
+                print ('not including podcast in master playlist: {}'.format(path))


Do we really need to print that to the user? Also shouldnt the listtype be 3 instead of 1?

I can remove that printout. The intention was to match the print level of other messages.
Also, this section is excluding podcasts from the All Songs playlist. That's a bit more clear with the enum, but I added a comment.

I have not run the program for ages. If you say the print fits, we can keep it. But please use a capital Not at the beginning :-D

I currently try to understand the code again. Isnt there a track object inside that playlist of which we can check if it is a podcast. But at another point we also accept id3 tags with Genre set to Podcast, but this code here does not support this.

It's not a track object, just a path to the track. In order to detect by id3 at this point, the code would have to reference the track header and look up each track object, or reload the metadata with mutagen. I think that would be better handled by a bigger re-architecture that loads all data, building the database objects and then writes everything at once, rather than building up some data as the database is built.

ipod-shuffle-4g.py

NicoHood · 2021-09-02T07:01:26Z

ipod-shuffle-4g.py

@@ -469,7 +469,7 @@ def construct(self, tracks):
            playlist.populate(i)
            construction = playlist.construct(tracks)
            if playlist["number_of_songs"] > 0:
-                if playlist["listtype"] == 3:
+                if playlist["listtype"] == PlaylistType.PODCAST.value:


Why do you use .value here?

"listtype" is a number and can't be compared directly with the enum. I could make PlaylistType extend IntEnum and then I could drop the .value in that location. But I can't make the FileType enum extend IntEnum so they'd behave differently and I don't think that's worth the minor convenience in this one spot.

I would use IntEnum here and maybe rethink about the filetype enum. Is that filetype enum even required? It looked to me that it makes things just more complicated, but I might be wrong

Sounds good. Actually, in researching how to make enums comparable to ints I came across reasons not to do so (enums aren't ints), but I can instead convert the listtype value to an enum.
if PlaylistType(playlist["listtype"]) == PlaylistType.PODCAST
I might like that better actually.

I like the FileType enum because it more tightly groups the file extensions with the FileType they go with and it adds consistency that the database values that are a limited range of values should be handled as those values.

Using IntEnum does not work?

It can work, but while researching how to do so I came across reasons not to do so and I think it's clearer to directly compare the enum rather than mixing enums and ints.

NicoHood · 2021-09-02T07:05:26Z

ipod-shuffle-4g.py

@@ -467,7 +520,7 @@ def set_master(self, tracks):
        if self.playlist_voiceover and (Text2Speech.valid_tts['pico2wave'] or Text2Speech.valid_tts['espeak']):
            self["dbid"] = hashlib.md5(b"masterlist").digest()[:8]
            self.text_to_speech("All songs", self["dbid"], True)
-        self["listtype"] = 1
+        self.listtype = PlaylistType.ALL_SONGS


Why did you change from struct to class property?

I created the class property self.listtype so I could set it to the enum value and then compare to the enum instead of converting to the .value. I'm not sure why I didn't do the same for the PlaylistHeader. I'd be inclined to make that one match with a new class property, rather than making this match by dropping it.

NicoHood · 2021-09-12T18:58:59Z

ipod-shuffle-4g.py

+        self.extensions = extensions
+
+# collect all the supported audio extensions
+audio_ext = functools.reduce(lambda j,k: j.union(k), map(lambda i: i.extensions, FileType))


Doesnt it make sense to include that inside the FileType Class?

Maybe I'm missing how to do this, but my attempts haven't succeeded and the examples I have seen for defining this as a static attribute involve extra lines. Also, given that it's right above two other lists of extensions, I don't think it's objectionable to have it outside the class.

NicoHood · 2021-09-12T19:02:32Z

ipod-shuffle-4g.py

@@ -666,7 +725,7 @@ def check_unicode(path):
    ret_flag = False # True if there is a recognizable file within this level
    for item in os.listdir(path):
        if os.path.isfile(os.path.join(path, item)):
-            if os.path.splitext(item)[1].lower() in audio_ext+list_ext:
+            if os.path.splitext(item)[1].lower() in all_ext:


I think all_ext is really bad to understand. Since it is only used once (i think) we do not need it and can use the better understandable name.

Why does it even check the extension here? The name of the function only checks unicode, so is it even placed correct here (in the first place, was not your initial code).

I figured it made sense to give that union a name and define it with the other lists.

I assume it's doing the extension check to avoid renaming any non-audio files. I'll note that while the code renames non-unicode files to avoid characters that can't be handled by the iPod Shuffle file system, there are some ascii characters that can't be handled as well. Specifically, any file with these characters can't be played: "*/:<>?|\. That might be useful to check for as well, but I figure those make sense in a different PR.

$fracai$

fracai added 4 commits August 27, 2021 22:33

$@fracai$

follow symlinks, avoid duplicates from following links

fd6150c

$@fracai$

mark tracks and playlists as podcasts

bab8af0

$@fracai$

create podcast directory

5714079

$@fracai$

exclude podcasts from master playlist

7dc7d4c

NicoHood reviewed Aug 28, 2021

View reviewed changes

fracai added 2 commits August 28, 2021 14:10

$@fracai$

podcasts should remember their last playback position

368a9b2

$@fracai$

readme entries for podcast and symlink support

dd0ccd9

NicoHood reviewed Aug 29, 2021

View reviewed changes

README.md Outdated Show resolved Hide resolved

$@fracai$

remove symlink support

78e1f59

NicoHood reviewed Sep 1, 2021

View reviewed changes

ipod-shuffle-4g.py Show resolved Hide resolved

NicoHood reviewed Sep 1, 2021

View reviewed changes

ipod-shuffle-4g.py Outdated Show resolved Hide resolved

NicoHood reviewed Sep 1, 2021

View reviewed changes

ipod-shuffle-4g.py Show resolved Hide resolved

NicoHood reviewed Sep 1, 2021

View reviewed changes

ipod-shuffle-4g.py Outdated Show resolved Hide resolved

NicoHood reviewed Sep 1, 2021

View reviewed changes

fracai added 8 commits September 1, 2021 12:30

$@fracai$

documentation and comments

020433a

$@fracai$

documentation and comments

e0a2f6c

$@fracai$

use Enums for (Playlist,File)Types

3ca83b6

$@fracai$

use set_podcast method to dedup podcast feature setting

0b277a9

$@fracai$

comments

dde2e3b

$@fracai$

comments

d4a9a14

$@fracai$

more enum use

767f371

$@fracai$

restore extension/filetype detection

a61317d

NicoHood reviewed Sep 1, 2021

View reviewed changes

ipod-shuffle-4g.py Outdated Show resolved Hide resolved

NicoHood reviewed Sep 1, 2021

View reviewed changes

ipod-shuffle-4g.py Outdated Show resolved Hide resolved

$@fracai$

set total_podcasts = 0 before processing tracks in construct()

3d7189c

NicoHood reviewed Sep 2, 2021

View reviewed changes

fracai added 4 commits September 2, 2021 12:30

$@fracai$

move podcast playlist detection to cover pls and m3u handling

aa740c2

$@fracai$

use a default of 0xFFFF instead of 65535

299813b

$@fracai$

better enum comparison

622f55f

$@fracai$

use the existing audio_ext set for checking file extensions

c68063e

NicoHood reviewed Sep 12, 2021

View reviewed changes

$@fracai$

fix setting filetype

ce2b20e

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Podcast support #54

Podcast support #54

$@fracai$ fracai commented Aug 28, 2021

NicoHood Aug 28, 2021

$@fracai$ fracai Aug 28, 2021

NicoHood Sep 1, 2021

$@fracai$ fracai Sep 1, 2021

NicoHood Sep 1, 2021

$@fracai$ fracai Sep 1, 2021

NicoHood Sep 2, 2021

fracai commented Aug 29, 2021

aoss-doeice commented Aug 31, 2021 via email

NicoHood commented Aug 31, 2021

NicoHood Sep 1, 2021

$@fracai$ fracai Sep 1, 2021

NicoHood Sep 1, 2021

$@fracai$ fracai Sep 1, 2021

NicoHood Sep 1, 2021

$@fracai$ fracai Sep 1, 2021

NicoHood Sep 1, 2021

$@fracai$ fracai Sep 1, 2021

NicoHood Sep 2, 2021

$@fracai$ fracai Sep 2, 2021

NicoHood Sep 2, 2021

$@fracai$ fracai Sep 2, 2021

NicoHood Sep 12, 2021

$@fracai$ fracai Sep 13, 2021

NicoHood Sep 2, 2021

$@fracai$ fracai Sep 2, 2021

NicoHood Sep 12, 2021

$@fracai$ fracai Sep 13, 2021

NicoHood Sep 12, 2021

$@fracai$ fracai Sep 13, 2021

Podcast support #54

Are you sure you want to change the base?

Podcast support #54

Conversation

fracai commented Aug 28, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fracai commented Aug 29, 2021

aoss-doeice commented Aug 31, 2021 via email

NicoHood commented Aug 31, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

$@fracai$ fracai commented Aug 28, 2021