[PATCH] [RFC] Gigabeat S support / mtpmediadevice tweaks

[Seb Ruiz]

Hi Andrew,
I'll let the proper analysis be done by somebody with experience with
MTP devices. Just a few things about your patch.

1 - please don't precede all your code with " // hobbs". It's quite meaningless.

2 - please add some comments to the patch explaining what the code
does and why (such as all those replace() calls

3 - don't use sprintf(). Use i18n( "Disc %1" ).arg( discNumber ). This
allows translations.

Thanks
Seb

On 07/11/2007, Andrew Rodland <arodland at comcast.net> wrote:
> Hello folks.
>
> I've got a Toshiba Gigabeat S30. It speaks MTP, and as far as I've been able
> to find out, nothing has really good support for it -- so I decided to add
> the support to my favorite app, Amarok.
>
> Since I'm not deeply familiar with the codebase, and since C++ is far from my
> most comfortable language, this is pretty dirty code. There's stuff in here
> that doesn't have help, stuff that should be factored out and made less
> redundant, and stuff that someone will probably just flatly disagree with. I
> include a patch because this code _works_ for me, but it's definitely "patch
> for discussion", not "patch submission" :)
>
> Anyway, this is a rolled up patch against amarok 1.4 svn head that contains
> fixes for several issues I discovered while trying to make my device work. On
> request I'll split it out into separate feature components, though the patch
> is simple enough that I think anyone should be able to do that by hand.
>
> Anyway, here's what I found
>
> =1=
> Problem: The Gigabeat S considers every track to be Unknown Artist / Unknown
> Album unless it's associated with an MTP album object (i.e. it ignores tags
> in the file)
> Fix: Call getOrCreateAlbum regardless of whether there's album art available;
> have getOrCreateAlbum fill in the name and artist fields of the
> LIBMTP_album_t struct.
> Considerations: There's code yet in updateAlbumArt that maybe shouldn't be.
>
> =2=
> Problem: The player doesn't recognize the disc-number tag and the MTP data
> structure has no way to represent it. I have lots of multi-disc albums in my
> collection.
> Fix: append "(Disc #)" to the album title sent to the device if there's a
> discNumber and make sure that we get separate MTP album objects as well.
> Consideration: Now the album title on the device doesn't equal the local album
> number... so we need to replicate this transform anywhere we compare against
> the albums on the device (like checking whether a track is already sent). Not
> so cool.
>
> =3=
> Problem: The device doesn't support user-created folders. Folder creation
> returns success but if you try to find the returned folder ID, it doesn't
> exist. Disabling the use of folders works okay, however filename conflicts if
> two tracks have the same filename on disk can cause transfers to fail
> mysteriously.
> Solution: Create filenames when sending to the device, in a manner very
> similar to the "Organize Files' feature.
> Consideration: Okay, I duplicated a lot of code here, and the whole thing
> smells bad to me. Probably the best plan would be to make this compatible
> with the existing Organize Files code somehow... but once again, example of
> working code.
>
> Thanks for the great app and let me know what you think. (Please CC.)
>
> Andrew
>
> _______________________________________________
> Amarok-devel mailing list
> Amarok-devel at kde.org
> https://mail.kde.org/mailman/listinfo/amarok-devel
>
>
>


-- 
Seb Ruiz

http://www.sebruiz.net/