I'm attaching a patch against the current CVS version that adds support for transcoding to supported formats as specified by HAL. So this is also for bug #344820.

It mostly works for me; I'm having some trouble with Banshee occasionally hanging when dealing with the DAP or quitting, but I don't think/I hope it's not caused by my patch.

Created attachment 72451 [details] [review]
Patch for MassStorageDap transcoding support

Juri, this patch looks great.  I notice that you implement Synchronizing, which overall is probably the right thing to do at the moment, since that's how Banshee works for other DAPs.  I had written it to not use synchronize, but to just to transfers immediately, because then by the time the user is ready to eject the device, the files are already on it, they don't have to realize they need to press Synchronize then wait.  Anyway, we can change it back if there is consensus to change all the DAP behavior to be like that, I suppose.

I have only looked through the code so far.  I will test this later and hopefully commit it soon.

Yeah. I was thinking of doing it directly, but basically gave up in the face of the DAP interface and just went with the synchronize approach. I'm not complaining if it can be made to work without synchronize.

Ok, this works pretty well.  The only bug I've found so far is that if you add a track, it ends up appearing twice in the DAP's source view (though it's only on the device once of course - if you restart Banshee it's not duplicated).  That was for a track that didn't need to be transcoded - don't know if that makes a difference.

I don't see entries twice in the track listing, but I do see them increasing the track count by two.

Another thing I noticed is that it gets the artist/title/album wrong for items that are already there: The Herbaliser/Another Mother/Blow Your Headphones becomes  Blow Your Headphones/Another Mother.mp3/03 - Another Mother.mp3. I'll try to fix these today if I have some time to hack.

Turned out the "another thing" was a separate bug, not related to this. I filed bug #355256 and attached a patch.

Created attachment 72493 [details] [review]
A new version of the MassStorageDap transcoding support patch

A new version of the previous patch that fixes for me the problem of adding a track to a DAP increasing the number of tracks by two.

The only difference to the previous patch is adding a tracks.Remove(track) line to MassStorageDap.HandleCopyRequested to remove the old TrackInfo once we've added the new, copied one.

Gabriel: can you set a target milestone for this too?

Created attachment 72661 [details] [review]
Updated patch

This patch is almost the same, it is just a little more careful when getting the extensions for a DAP.  Another bug I noticed is that if you remove a Track it is removed immediately instead of waiting until you press Synch.  That at least seems like a bug to me - I can't verify how the iPod support works atm.

That's how it is for the iPod as well.

I really want to get this in, but it's hard to test at the moment since some things are broken b/c the managed dbus.

I haven't tested it in a while (I'm blaming hardware troubles), but I'm pretty sure it's not quite ready to go in yet. Last time I used it, there was some weird behaviour with duplicated songs in silly folders etc. I'll have very little time to hack during the next two or three weeks, but I'll see if I can find some time. Although if there's dbus trouble, I guess I won't be able to do much...

I tried this patch with my Motorola V360 phone. It's not supported by upstream HAL, I have patched 10-usb-music-players.fdi to support the phone, by adding the following lines:

          <!-- V360 mass storage mode -->
	  <match key="@storage.physical_device:usb.product_id" int="0x4810">
	    <append key="info.capabilities" type="strlist">portable_audio_player</append>
	    <merge key="info.category" type="string">portable_audio_player</merge>
	    <merge key="portable_audio_player.type" type="string">generic</merge>
	    <merge key="portable_audio_player.access_method" type="string">storage</merge>
	    <merge key="portable_audio_player.storage_device" type="copy_property">info.udi</merge>
	    <append key="portable_audio_player.output_formats" type="strlist">audio/mpeg</append>
	    <append key="portable_audio_player.input_formats" type="strlist">audio/x-wav</append>
	    <append key="portable_audio_player.audio_folders" type="strlist">mobile/audio/</append>
	  </match>
	</match>

I applied the patch to 0.11.0 (since current HEAD seems flaky), built and installed it. I ran banshee, and dragged a file that is originally .ogg from my library to the phone. Then I went to the phone, hit synchronise, and told it to apply the manual changes. It all looked to be working perfectly; I saw it transcoding then transferring the file to the phone. I then unmounted the phone from the desktop, unplugged it, and looked for the song - it wasn't there. I plugged the phone back in and browsed the tree manually, whereupon I saw this. The song I tried to transfer was called 'Pray', by the artist 'E'.

The phone doesn't really _respect_ directories under /mobile/audio - it just displays any file or playlist in that directory or any subdirectory of it, all in one big list - but it does handle them.

When I use Banshee without the patch, it just dumps the .ogg file onto the phone, but it does so correctly, it doesn't create junk directories like this.

Any more info needed can be provided...

[root@zen temp]# cd /mnt/V360/mobile/audio/E/
[root@zen E]# ls
ls: /ò??é??.?ò|: No such file or directory
ls: Z??/hI?.ZYC: No such file or directory
ls: ÿ??/ ??t."@?: No such file or directory
ls: ?úg?6?ü.?i/: No such file or directory
ls: ?D$?ô.-/L: No such file or directory
ls: ??/F\?g.å +: No such file or directory
ls: ?ûæqX/ì.`c: No such file or directory
ls: ??5W?/.hxx: No such file or directory
??_??.??<*     àrúål$h`.ñ,?/  h??:(?&?.ln?/  ñ«?w!ïë?.vê:*  {·±÷????.ú?·*
????????.???*  ?å?s?~:?.??å*  ??h?ó?bª.z?Ü/  ?????ñw?.?mq/  ù???>??@*
??????*?.<??*  å???¿ün?.T?á/  ?hºü?@å¡/      ñ¡za4£?q.J?ê/  'ù½?C ?F.=Dâ*
?/             ^*à·V¥?r. ]P/  ?h?y??¼>.sò)*  ?ò???^$¥.,^%*  ?uå ??â?..?K/
?.???*         ????ªwp?.?d¿*  ??h?ÿ??å.V¼?*  @.??ò*         ü?ÆHP?ó4.=?b/
,.??0/         ?????âx&.á5y*  |¢???ï??.!?}*  o4)??^g?.;ôr/  Üæ?j????.??æ*
½?0zH??}.??o*  a??y??»?.?é?*  ?µî?????.0?i/  o-?¢??^?.??7*  ]ùà*ñ?úû.?ò?*
,04?í>`m.ÜX?/  äÿ?¿?R?û.+??/  i?1?>?h&.?$æ*  ô?7?q?êh.q?[*  üC??å?ùJ.`¡//
0¢,?????.á?V/  ??<[ªZ?£.:²d*  ?.Ia6/         ??òæ??ÿ?.??[/  ?u@c/??I.?º?/
?¼/\?D??.;?Ç/  å? O.+??.æ??/  ì¡æf?8èö.???*  ?÷öb5D'µ.t?i*  ÜC???º??.?¢?/
?)|?¼{m?.?o¥*  å uäêï.?._?d/  ?-iæ???ó.?»ç*  «/-òBh{].|6?/  (?úé?;?!.???/
???½?¢ms.êic*  B7?ï!?°l.<?²*  ??|?I?[á..jn/  ????£?%ó.E?E*  ??¿ú???è.d,µ/
½;/n$-??.?¼»/  ?beíu??f.???/  ?íª?y«?ª.:nu*  öê,?ög»f.???/  ?ù?ÉID??.u??/
+??½\°rh.???/  ?bè?s?h±.??W*  íb?q??lj.}?n/  ?¬??ò?E?. ?£/  ?'??úG,?.K?ë*
½?v.i?0*       <b¥?F???.g:U*  ?ïf???{å.?g0/  =ºF??ên+.?)¬/  ??uh?ñm?.?,`/
0v???p??.?[ò/  ?B??NÑì?.¡??*  \?ig4 á?.??¥*  OggS.?*        ??ùhSÑ?£.÷¿?*
?0??w??f.c??*  «BÑ????x.???/  ïGÄ{ú??}.äja*  ò?j??é??.??½/  £?????û?.î??/
¬½y·&ä¬?.??)*  BQ???ç?*.#n?/  í??g?n$?.}??*  Ö¥j%«??P.µN?/  u?¥ïÉ?°?.???/
???¼°Y?;.ò?p*  ?bx!??-ù.v??/  ?i?ï?Än?.¿wB/  ???ók??ß.}ßa*  ü'?j?0??.c?t/
?????1:\.¡áå/  ç~5??ö?é.o??/  ???i??íè.?o?/  ò;???(lù.ï?ì/  ??ùj6?>«.&+?*
1æ?çä»>?.Åko/  ç±?7??.@S?*    îó????»>.#??*  ö?¬N.ä?d/      ?û???¬??.(kÑ/
?.1N?*         ?DZ?9???.¬??*  ?ì??!??ò.?A%/  ??"?òñ'?.>ç/   u??.?-?l.f?+*
1üvªæ[?æ.???*  ç???ä[½?. /?/  íÖæñ¡*¼?.o2?/  ö`Ñl????.JT?/  ?¡?|úL?z.???*
<1=?v?[?.???/  c@????á?.??ò/  ï?»?·sÜ?.???/  ò???ñº??.°+?/  '.÷Uó/
??.2?/         ?c?au~ç?.ïö}/  ???i_???.t/    ??º_ñ???.öîe*  uol  ,  ."  *
?}?}²???.???*  c?Çö??=H.+-a*  ???!?í?t.ú3?*  ??ô#????.ô^?*  uoò???a?.8ls*
2>?3{?9?.h?°*  |»c?ê\??.???*  ?¢?:iû??.!?(*  ö?ô??é??.???*  û???össû.?+&*
25???ÄI2.3ë@*  çé'ú?~eò.???/  ????ì??¥.ü?A*  o±?¬o@l¢.???*  û?öû3>gi.???*
?2:æ^??¼.?ª?/  çh?t_?áq.???*  î?u(òF±?.?«**  ??O??Pro./??/  ?:uqnlb?.???*
?;²???nâ.??Ç*  ç??K>??à.?$?/  ï?üUH«=à.?ª?/  oqt?s&?9.^Xk*  ?uqt}?.6?é/
²¬o9?që?.??¢*  ckze?ww*.?9?/  ? iu???v.o÷x*  {ô??q  ?.4fÿ/  _u???ß?f.a??*
??²??º]ó.öqo/  c.?Lf ?=.ù?h*  ?i?xö??¼.M??/  «ösi?$il.&/    ú?t?:ói£.\ô?/
2???«?z¿.??A*  ??£;C_^?.ñ6?/  ???j????.??`*  oßyz?e?°.??ä/  ût<??? ?.?n'/
31?0????.?aÜ/  ??C[???ñ..Q//  ^??J}_²?.²??*  ò?%????*.?Tç*  ??÷?Üû?h.?M?*
? ?????.3?ë*   Ç:ö?/ì²».?àa/  ¿Je¥»??N.?²f*  o??ü¼?}|.?ÿ?/  ??*?Uù.?wn*
? 3F?!?z.|-*   ?ÇôY???i.?/    jhé?|.%?`*     ?-ö?»ù??.æi{/  û/w?â??à.¡}u/
]4Öñ?~x%.??â*  ?c??q.9»?/     >j???ís?.>,m*  ¥??óù?uu.E?,/  µ@û??WiÆ.[c?/
???4sd?G.~??/  ?¡<?cw??.J¬?/  *jl??ae?.(¬?*  o?xzjê?..?s?*  ú?.z£???.l?$/
5â:g=?L?.???*  c÷} 3s°|.m?,/  ?jly?eu?.±mq*  ?·???ö?y.?j¥/  ü ?[5?â6.~¢:/
¢???5?@?.f??*  \|?d?.µ??*     £?jÑ????.?±~*  ¿¿`$?o·?.??z*  ü  ? ?  .?  /
*?????5t.ªó?*  D?"9?ä.äh"*    ?j??ñ??è.â°?*  p?.¢½8/        V|(·?¼D?.??ª/
5?t»ö?O.?^½*   D?aÜ?µ?~.·¿?/  jñïj??²s.uy?*  p??·)??2.é?(/  ?v^÷ÄÆ?$.?b?*
65ïö=CU?.?µt*  d?ë¢????.°i?*  ??j?ºeu>.???/  P²ùe3?^.¿??*   ve:#.?$+*
:}?¬6t+].?:µ/  ???-d°m.í?^/   j\???ß'?.??k*  ?P²vTZ¬T.]¿?/  V?ësq{?D.åyl*
6:uz?0.)Uê/    d?¥ö?±?r.-?r/  j#??ù?4?.?£s*  ?(p?æ?d?.j??*  v?g?I!?ï.??·*
??"6w??û.???*  ?????ê(?.??½/  j?Z?¬?æ.?!?/   p!(???äë.?mV/  ??«vî???.Eáó/
7²?-???C.?I¥/  e0?X"?%ó.#?Q/  ^???k?±÷.3°?/  ??????pë.???*  vÑ?U?ëÆ.·ëD*
7¿????Tÿ.??r*  ??eæi?DÉ.??¥/  k?ª6ÄS,l.2i$*  ?±p?]iÄ?.?gq/  &*?v??_?.vàh*
¿7>&??ú?.???*  ÉâQûXD?$.öx"*  ?k?}æ??½.pnc/  ??÷pJq?ù.ñtû*  ¢V(ÿd??$.yº?*
?7.w¬6/        ?ec?3;4?. ?}/  ?k|#æ?>?.?ñH*  pÖ?$S??£.???*  ?w?4?¬mt.?ë*
?8??æ?:}.v£?*  E?C?<?t?.nY?*  ????kªgè.?(?*  ??Pq??è¬.8\?/  w£é5?i7#.µ??*
8ç?.3?$h.ñ??/  É???è.7?z*     ???K?ëPr.?~?/  P?=?rWÖÜ.ei£*  W?=e{6Ç~.?7É/
??)9àY?\.&%0*  ?ê+èôOK[.rïê/  ?/Kï?jk).D?¼/  ?p:ßç???.å?|*  wì=6`èè?.j??*
??9}w??f.??s/  ??eïäx??.?<c*  ¥k?'??O¡.ëùæ*  ?p{??X??.$?$/  )?(?w?oc.±u?/
á<>°?_]?.???*  ê??ïñ?ô?.îá\/  ??k?òO!ü.??¿*  ? q«²???.&ëç*  ?wöz??å?.?x?/
*?.??à/        ek¼¬??ñµ.?qá*  ?kù?pg¢?*      ?Q?#»²G?.?o?*  wq?:qçù?.CÄ?/
?.?.¿?â/       (?el?»?J.?h?/  kµW??m??.K??*  ?[??q±?a.uj?/  ?wV|yº??.?y?*
<? ???å?.¡?0/  ?)e??ñ??.??h/  ?????l??.???/  ,?QáZM¬?.???/  x??¿???¼.£??*
?â½?X4<ß.["T/  ëö$ëeb??.?;?/  L8????y?.??i*  /??q~??c.u?"/  ¬x|=²ª??.«?9*
"a-)3}±?.S?*   ??êq~{?H.??Y/  ??l?«çî'.U?I/  «??që??¥.?a?*  x2???p??.P??/
á3?v?ªv?.á??*  e?sñ?î?t.??º/  l%·g&?iZ.âU2/  q?m?5?vú.?0l*  x:?????".??ä/
a??6?ò??.??>*  *??~_?ê?.£ú?/  ?°?l+???.?ìô*  ?q?&?ó?`.Hôä/  x?÷\a?<1.?æ?*
å9????ü?.???/  ??ê!ü???.?-?*  =??L???·.k6?*  Q?&X?D?2.av&*  x??Äv??á."?ñ/
?ÅA?0_Æ?.?g?*  éù?à??½3.^*$*   `?l???k.#u?/  (???@?r?.?^?/  ?xj?å¡??.??V/
aaæçèÿéö.¬??*  :Eu?ÇZ??.Y?Ä*  ???l??m?.?ô|/  ??¬$????.R[?*  ?x??j??..?hç*
??{^äån?.??s/  e·??ue%?.?l¢*  'l?+?º?æ.FÜ?*  ??]?????.rä?*  xj?·tå?b.???/
°?«?àa?·.{w?*  ?ê uº'±g.???/  ¥l???o??.??z/  »r?åVìª?.?%b/  ??xk.??i.?¢÷*
·âc??ggî.=tß*  e??úv??º.?í?/  L?w|;{??.Y?O*  ??r?????.E??*  X??\?ñCH.?}?/
Å?C?±?^,.m7æ/  e¥vw?Hÿ?.iJo/  ????m7²?.??ü/  ??r??¢ñ,.?{$*  xß?°;fà?.sìr*
-?.äco/        ??ê??w??.??°*  ??/me&??.??q/  rohn?»?ê.föe*  x???v??v.???*
?äc?£vü?.?O~*  ?±É'Z??¼.u??/  ÷?mfm??£.?²ñ*  #?,r?ó??.?ô?*  ?·x?<wú?.9ó»*
ª?dµ????.??\/  F?å??K?Y.ö7º/  ?m?/L?d£.?ç?/  roùv???ë.^??*  ?x?xé?h`.??ü/
Å?°?Dú??.ÿk½*  ?»F>Ä??Q.g?n*  môÅ??|??.y·z/  R?»·?.öX?*     ??y!÷???.?½z/
?æ??/          FC?.??ä*       ?M?,??o?.t?e/  ?rs?t|ïw.?¡?*  y??á1???.?µ?/
æ:å½cN?e.,ôd*  f]???\?D.z¢?*  m?r&?|ò?.???*  ????r,-«.sz?/  ¿?ÿ@bJ?H.¼??*
Æâ?ªh¢+».¥rs/  fÉn???MA.?çn/  m????·^?.?ß?/  ??r?v«÷^.i£?/  ÿ.¡Dö*
Æd?ë??Y?.?lg/  fGë.?oi*       m.?¢???*.{vá*  ?s5?k?º?.,»?*  y????mg7.j ?/
?æ?&?í-a.5?c*  fj?d%?w*.R??/  M?WN9??Ü.åí#*  ?+??s?B0."?ó/  ????y??n.9àµ*
ƺ0??E2².l?r/  ?f??K???.?$?/  ??????£?.n??/  "?s['??D.???/  Yw<???ÿ\.¡ò?/
?ÄEQn(.k](*    ;F?º(???.???/  ?*?ñ?¡??.&??/  s???g??x.Q?÷/  z?b??<??.N??*
=Æ?Qw??à.?T?*  »???f?qæ.ú??*  ñ???0¿m·.? ?/  ¥#?S?¬??.:p?*  ?z?efê?r.?æà*
-????æ~s.;??*  fS-e???|.«f?/  ???[ñ²:¼.?ß²*  s?°¢pS?Ä*      ?¥?z°??R.?(?/
+æv?áu??.ó?Y/  F???U?ª?.??é*  ?£n???!4.ç[?*  ß².?jò\!.???*  ??Zs?½?ú.?é½/
??æX&?H?.b\?/  ?g??_â??.£g%*  /???????.?ñ9/  ß?@??7ú?.p??*  ??"|?»(z.U|?/
?Ä$F÷ûG?.?og*  ?Gb}»Ç1?.6/    ñåa???$?.¢½?/  ß?f?÷?üà.??U/  zu=?oê.???*
á\?}???ï.???/  ?~g?ò0?l.}Ä?/  µ¬ñæx:??.?db/  s?????y?.?3°/  Z?~Uû?ª?.ç8?*
??ä?{~??.i??/  ?????g??.O?î*  n?ä(???i.???/  }/sz??8u.?7#/  z?zæo7??.8??*
[?âì?'m?.U<X/  "gpo?¡m9.???/  ?ñak??£?.???*  ?T??¼ó??.ä?W*  |? ?ª?8.???/
á?j?LO=ñ.p3Ç*  gtf`?ma`.???*  ??ñ<c???./f?/  t??1??Ѭ.ó??*  ? ?ª÷}?*.??è/
?å???#,?.k=¥/  gx|?s?!å.??(/  (n?i?"t[.NAO/  ,?t?æ??..?d¿/  ? ?gº?{?.m??/
?a?M«R?l.^??*  h~>?Æ6ôn.?`?/  ]N??m??².??ü/  t???EÅWO.ü?¥/   ì??öf;x.1?*
 ¡äm>?z?.?q?*  ?µh?à?j`.?'8/  '?nmª?o9.[¥G*  t?`?L?lT.qö?/  » ?kô???.?mô/
à?¿&°n?î.?7b/  hçü»???*.???/  ñnä???]ï.??ó/  T?Q+??!a.îo?*  ? ,ÿ?ÿ?k.x?]*
??Än????.?@º/  $?h?éæ`?.???/  ?????(¢n.?]r*  ?]?;?tsy."WV*  ? ?  ?  .?yv/
ª)ñ??ü?p.?wr/  hf?åz?:?.?h?/  ¡N???»{U.??g*  tü??0ç«ß.ròo*
?a?O=Pâ?.?Y?*  ,H?fY~?!.???*  nu_r·??ä.???*  ?t-w??¼q.r??*
?¥å?óß?æ.??(*  HK0?n}K?.kq?/  ñÜ?R??.vv*     T??°Xàg?.?U°/
?á?rj¬oj.\??/  ?H@????K.-?Ç/  ?????ñù .vï?/  tx???w\u.1°g/

okay, I can't reproduce the above problem, I think it may have been a faulty unmount or some kind of conflict with Rhythmbox (which was running at the same time). With RB not running and being careful to unmount well (run sync before unmounting), the patch seems to work, which is great. Just a couple of comments: it would be _great_ if it could work without using the Synchronise system, and also, isn't the point of gstreamer that you can do everything in one pipeline? Why are transcoding the file to the appropriate format and copying it to the phone separate steps? Couldn't a gstreamer pipeline which converts the file to the appropriate format and writes it to the phone in one operation be constructed?

But just minor niggles, this thing works well so far as I'm concerned.

Now to find someone to implement .apl playlist support for Linux, as that's the only thing my phone seems to speak...

Patch works well with my H140/iHP-140, one comment is that if you abort the transcode via clicking the X the synchronization just hangs.

This patch no longer applies against 0.11.1 or 0.11.2, and I can't build 0.11.0 any more. Can someone please update the patch? I'd do it, but my coding skills are precisely zero.

*** Bug 344820 has been marked as a duplicate of this bug. ***

Created attachment 80479 [details] [review]
Updated patch

This is an updated version of the patch that is working well for me.  Any testing would be appreciated.

Committed to SVN.

*** Bug 399566 has been marked as a duplicate of this bug. ***