After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 647202 - When Transcoding, Banshee uses the pre-transcode size for figuring the amount of space used on the device
When Transcoding, Banshee uses the pre-transcode size for figuring the amount...
Status: VERIFIED FIXED
Product: banshee
Classification: Other
Component: Device - USB Mass Storage
2.0.0
Other Linux
: Normal major
: 1.x
Assigned To: Banshee Maintainers
Banshee Maintainers
Depends on:
Blocks:
 
 
Reported: 2011-04-08 18:42 UTC by Michael Alan Dorman
Modified: 2011-08-22 14:00 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
This is some debugging statements to demonstrate the problem more clearly and the stupidest possible fix (1.64 KB, patch)
2011-05-19 15:10 UTC, Michael Alan Dorman
needs-work Details | Review
Updated patch with debugging and possible (stupidly simple) fix. (1.64 KB, patch)
2011-06-16 12:19 UTC, Michael Alan Dorman
none Details | Review

Description Michael Alan Dorman 2011-04-08 18:42:03 UTC
I keep my music collection in FLAC format.  I have a 1GB iPod Nano I use when I go on walks.  I have a smart playlist that makes a random selection of 10 hours of music (a conservative estimate of how much should fit, since I've got LAME configured for fairly high compression) to sync to the Nano, and I configured Banshee to sync the Nano with that playlist.

When I initiate a sync, Banshee will start transcoding files, and will place the transcoded files onto the iPod *up to the point where the _un_transcoded files total 1GB*, at which point it stops.  The bar showing the space allocation on the player shows full, even though df -m shows 800MB+ free, and, in fact, the legend at the bottom of the bar _also_ displays 800MB+ free.

Beyond this point, Banshee will continue to transcode files, but it won't copy them over, and when all the transcoding is done, ejecting the iPod and trying to play results in the little flashing lights indicating a problem with the iPod db.

My hypothesis is that Banshee is using the pre-transcode size of the files to calculate free space, rather than the post-transcode size.  Hopefully this should be a small thing to correct.  I may even look at the code myself, though I'm not particularly well-versed in C# or the like.
Comment 1 Michael Alan Dorman 2011-05-19 15:10:04 UTC
Created attachment 188136 [details] [review]
This is some debugging statements to demonstrate the problem more clearly and the stupidest possible fix

I'm not going to pretend that this should be the proper fix to this bug---I don't know C#, I don't know the Banshee code base outside of tracing through this one little thread of code---however, this one-liner patch (plus a few debugging statements that helped me find where to muck with things) seems to resolve my problem.

From a software design perspective, it seems to me that it would be better if the transcode service, instead of doing what I perceive it to be doing---handing the trackinfo for the original track, coupled with a reference to the new track's URI---simply handed some sort of track object to the OnTrackTranscoded handler, so that the bits of code that are actually trying to copy the file to the DAP are working with the thing they care about, not trying to do some sort of synthesis.

But that's just me.
Comment 2 Leo 2011-06-15 00:06:08 UTC
I have the same problem and tried your patch, but when I try and compile I get the following error:

  MCS   ../../../bin/Banshee.Dap.dll
llist@LeosLinux:~/tmp/Downloads/banshee-2.0.1/src/Dap/Banshee.Dap$ make
  MCS   ../../../bin/Banshee.Dap.dll
./Banshee.Dap/DapSource.cs(339,27): error CS1955: The member `Banshee.Collection.Database.DatabaseTrackInfo.FileSize' cannot be used as method or delegate
Compilation failed: 1 error(s), 0 warnings
make: *** [../../../bin/Banshee.Dap.dll] Error 1

Thanks
Comment 3 Michael Alan Dorman 2011-06-15 01:50:28 UTC
(In reply to comment #2)
> I have the same problem and tried your patch, but when I try and compile I get
> the following error:

I wish I had some suggestion of something to try, but I really know very little about Banshee's code and even less about C#---I don't understand why that line would compile for me and not for you, I don't understand what the error message is trying to tell me, nor do I have any idea how to fix it.

I will see if I can find some time to try and figure out what's going on, but don't hold your breath---I really do wish I could help, as I would like to see this issue fixed upstream, but I fear I'm not the guy who's going to be able to do it.
Comment 4 Leo 2011-06-15 02:34:12 UTC
Same here. No C++ skills, but not sure why this would work/compile for you and not for me.

I wouldn't waste too much time as I'' get back to the developers with this; they asked me to try your fix in the first place.

Cheers
Comment 5 Leo 2011-06-15 22:51:51 UTC
I've had another look and believe that the setter for track expects an argument (hence the error)

I changed the code to 

track.FileSize = (Banshee.IO.File.GetSize (fromUri));

which works for me.

Not sure why your code works for you, though, or the impact of this change on the rest of Banshee

Cheers
Comment 6 Andrés G. Aragoneses (IRC: knocte) 2011-06-16 10:12:12 UTC
Comment on attachment 188136 [details] [review]
This is some debugging statements to demonstrate the problem more clearly and the stupidest possible fix

(In reply to comment #5)
> I've had another look and believe that the setter for track expects an argument
> (hence the error)
> 
> I changed the code to 
> 
> track.FileSize = (Banshee.IO.File.GetSize (fromUri));
> 
> which works for me.
> 
> Not sure why your code works for you, though, or the impact of this change on
> the rest of Banshee
> 
> Cheers

Leo, thanks for the review! Michael, do you mind updating your patch to address Leo's advice, and post the new version here?

Thanks
Comment 7 Andrés G. Aragoneses (IRC: knocte) 2011-06-16 10:12:48 UTC
Confirming bug now that 2 people see the problem.
Comment 8 Michael Alan Dorman 2011-06-16 12:18:45 UTC
> I changed the code to 
> 
> track.FileSize = (Banshee.IO.File.GetSize (fromUri));
> 
> which works for me.
> 
> Not sure why your code works for you, though, or the impact of this change on
> the rest of Banshee

Weird.  Well, I tried that variant, and it also seems to work for me, so I'm happy with it.
Comment 9 Michael Alan Dorman 2011-06-16 12:19:59 UTC
Created attachment 190033 [details] [review]
Updated patch with debugging and possible (stupidly simple) fix.

This patch is updated from feedback about what works for others.  I can only guess that this is a mono compiler version issue.
Comment 10 Michael Alan Dorman 2011-06-16 12:21:47 UTC
> Leo, thanks for the review! Michael, do you mind updating your patch to address
> Leo's advice, and post the new version here?

Yeah, sure, I updated it, and confirmed that the chances Leo proposed still work for me.

I have *no idea* why the parens might make a difference--though I might guess it's compiler version issue--so it really would be a good idea to see if you could get someone who actually knows C# to review it. ;)
Comment 11 Bertrand Lorentz 2011-08-17 16:07:25 UTC
Thanks you for your bug report, your investigations and the patch !
Sorry it took so long to process it.

I've committed a different one-line change that does the same thing, but in TranscoderService, after the track is transcoded and at the same time the MIME type is updated : 

http://git.gnome.org/browse/banshee/commit/?id=31c1a54

Could you update to the latest version from git master and check that it fixes your issue ?
I don't have an iPod to confirm it myself, but my change has the same effect as yours.

Feel free to re-open this bug if the problem is not fixed.
Comment 12 Michael Alan Dorman 2011-08-22 11:53:56 UTC
I just wanted to confirm that the updated patch seems to work for me.
Comment 13 Andrés G. Aragoneses (IRC: knocte) 2011-08-22 11:56:08 UTC
Michael, Bertrand pointed you to a commit in master, not to a patch.

Do you mean you retrieved current master branch and without applying any patch, it works for you?
Comment 14 Michael Alan Dorman 2011-08-22 13:56:58 UTC
(In reply to comment #13)
> Michael, Bertrand pointed you to a commit in master, not to a patch.
> 
> Do you mean you retrieved current master branch and without applying any patch,
> it works for you?

Hi, Andres,

Since my interest is in having a working Debian package, all of my work has been done using the debian source package and adding my single patch file.  The code in question has been stable enough that I've used the same patch against 2.0.0 to 2.1.0.

Since the link Bertrand pointed me to has a link, clearly labeled 'patch' (it resolves to http://git.gnome.org/browse/banshee/patch/?id=31c1a5494333793bbf128f9f5a47bd7a104d7288) the simplest thing for me to to was download that patch file and substitute it in for the patch I had been maintaining, rebuild the debian package, and test the resulting binary---which appears to work equivalently.
Comment 15 Andrés G. Aragoneses (IRC: knocte) 2011-08-22 14:00:17 UTC
Awesome, marking VERIFIED then.

The fix will be included in the next version of Banshee (will be called "2.1.3" most likely).