ytdl-sub
ytdl-sub copied to clipboard
Add list support for music_tags plugin
id3v2.4 tags allow lists. We should be able to write those.
The music_tags plugin uses beets' mediafile package: https://github.com/beetbox/mediafile/blob/master/mediafile.py You can see some fields are ListMediaField. Those are the ones that should be able to support list values in ytdl-sub
Task: Support id3v2.4 lists by specifying a list in music_tags.tags.<some_field> https://github.com/jmbannon/ytdl-sub/blob/master/src/ytdl_sub/plugins/music_tags.py#L76
subtask: These fields should support both strings and list of strings. https://github.com/jmbannon/ytdl-sub/blob/master/src/ytdl_sub/plugins/music_tags.py#L42
subtask: We should verify the specified field supports lists prior to any downloading. This would need to happen in the
Can I take this issue?
Can I take this issue?
Lets get your other PR in first :-)
Do you still want to take this @MariaMozgunova ?
@jmbannon Could you answer a few questions?
There is an example https://github.com/jmbannon/ytdl-sub/blob/master/src/ytdl_sub/plugins/music_tags.py#L30-L33 for non-list tags. What will it look like for the list tags?
For the first subtask, the new FormatterValidator
needs to be declared?
The second subtask seems to be incomplete.
@jmbannon Could you answer a few questions?
There is an example https://github.com/jmbannon/ytdl-sub/blob/master/src/ytdl_sub/plugins/music_tags.py#L30-L33 for non-list tags. What will it look like for the list tags?
It should be either a single value per tag like it is now OR a list of values. In the backend, treat both cases as a list of StringFormatValidators.
I added this change so yaml input can support single value (casts to list of 1) or lists for any ListValidator: https://github.com/jmbannon/ytdl-sub/pull/126
For the first subtask, the new
FormatterValidator
needs to be declared?
I think two validators should be made: a StringFormatListValidator, and a MusicTagValidator that inherits from it. Each music tag will technically be a list, but treat it as a value if the list length is only one.
The second subtask seems to be incomplete.
My bad. Was going to say the MusicTagValidator should ensure the tag supports lists (i.e. artists does, not artist). See the MediaFile dep source code to see which ones are lists or not. This validation should be in the MusicTagValidator init.
Hopefully that makes sense 😅