fholger / RocksmithToTab

Exports Rocksmith 2014 songs to Guitar Pro tabs
http://www.rocksmithtotab.de
BSD 2-Clause "Simplified" License
211 stars 36 forks source link

Fixed several bugs that prevent some files to be opened in GP #30

Closed killergege closed 8 years ago

killergege commented 9 years ago
killergege commented 9 years ago

I noticed the issue with the Volbeat DLC, 4 out of 5 files created couldn't be opened by GP. My goal was to have the generated files as similar as possible as the same file after saving it in GP.

I'm not really sure about the last one. It seems that voices & bars shouldn't be reused (some references are broken) but it seems that the file created by GP is lacking a lot of bars (it stops at ID=235 while it should go up to 479). I don't understand how it works in that case as without Bar I see no link between Track and Voices. It might be my GPX decoder that doesn't work properly and doesn't write all the Bar elements that exist in the file. I'll do more testing on that.

fholger commented 9 years ago

Thanks a lot for your work. Unfortunately, I'm currently busy with finalizing my PhD thesis, so I probably won't be able to prepare another update for RocksmithToTab for a couple of weeks. I'll leave this open till then, hope you don't mind.

The GPX file format is quite a bit on the strange side. Except for master bars, I believe everything can be reused and should be to limit file size, but it's been a while since I looked at the specifics :)

killergege commented 9 years ago

Well, it could probably be reused in theory, but then you'll have to update all the references to match that (for instance on the automations), maybe by maintaining a reference dictionary as you are changing the bar numbers. There might also still be a problem with those references as you don't create the bars in the same order GP is doing it (you do it track by track, "horizontally", GP is doing it all tracks bar by bar, "vertically"), so there might still be some issues with the automations, I didn't look how the ids are build in that section of code.

To test it, I used AlphaTab as unlike GP, it's open source so I can see what's going on (GP just gives a generic error... great) and it's a bit more restrictive as it crashes when it doesn't find the references :) By the way, AlphaTab absolutely didn't like the reuse of the bars, it messed everything up, it would never work with this tool. But who knows how GP is handling those. GP seem quite fine with it most of the time, although removing the reuse code fixed my issue. I still don't understand when it works and when it doesn't.

I agree this format is a bit strange and it's annoying that they didn't use standard zip format or didn't publish any specifications (although they said they would when they released GP6). I also used some information from the blog of a MuseScore developer to try to understand it better. (https://jpirie23.wordpress.com/2014/05/11/implementing-initial-support-for-the-gpx-format/), but you probably already know all of this :).

Don't worry about the pull request, it's not urgent and it will give me more time to test it and maybe find a way to reuse the elements in a way that GP likes :). Or at least understand how GP creates the file. By the way, you did a really fine job, the code is clean, easy to read and to understand. it would be great to work with this kind of code everyday ^^.

fholger commented 9 years ago

I think the reuse should be fine, in principle. But of course, it can only work if the bars are actually identical in all reuse places, so there is room for some edge cases where my reuse might not be restrictive enough. I'll double-check when I have time. Thanks for the report.

fholger commented 8 years ago

Almost half a year passed, whoops. Sorry about that. I'm finally done with my PhD exam, so I have some time to work on a (hopefully final) version of RocksmithToTab.

Anyway, I reviewed your changes, and they are all valid finds which I'll apply. Again, thanks for your work and for reporting them. However, I would very much like to keep the reuse of bars etc. due to the significant reduction in file size. I know that GP6 itself does reuse these elements, so I would rather find the actual bug than disable that feature. You mentioned that it fixed issues for you; could you point me to a specific example, e.g. a specific track that didn't work properly when reuse is enabled?

Thanks a lot! And again, sorry for the delay.

songless-bird commented 8 years ago

Thanks for this great tool :) I got nearly all of the DLC and the first thing i do after buying is to extract the tabs, first as gpx and for the songs where this does not work as gp5. I just wanted to chime in with two points: 1) so far only two songs could not be opened at all (All that remains - This calling and Queens of the Stone Age - No One knows) and 2) if you need testing which tabs export in gpx or gp5 i am willing to help.

fholger commented 8 years ago

Thanks :) You don't need to do any specific testing, but if you spot any problems, please report them in the issues section so that I know about them and can look at fixing them.

killergege commented 8 years ago

As I said in the first comment, I mostly had issues with the Volbeat DLC. I'm pretty sure I had issues with other tabs but they are hard to find as I must open all of them to check. I may do that if I have some time.

The main problem I see with that "reuse" thing is due to the architecture of the app. If I remember well you read the data track by track but the GP file structure is beat by beat (or the other way around), which makes the reuse a bit more complicated. I did spend some time to try to fix the reuse but never finished and didn't insist too much as the app is working and hard drive space is not really an issue :).

I didn't analyse many genuine GP files so I don't know if they do reuse elements but I would be surprised if they do (it complicates a bit the editing of a file, no ?), I guess they rely a lot on their compression algorithm to keep the files lightweight.

fholger commented 8 years ago

Alright, I'll look into it, thanks. GP6 absolutely does reuse elements, the whole file structure is engineered towards that feature to make the filesize smaller. And it doesn't really complicate that much. It makes no difference when reading the file, it's only in the writing that you have to ensure that anything you reuse is truly identical to what you need. My suspicion is that there's probably an edge case somewhere where I reuse something that is not fully identical.

fholger commented 8 years ago

So I tested the Volbeat DLC and could find no problems with it, all five tracks open fine when converted to gpx. Out of curiosity, what is the exact version of Guitar Pro you have installed? Mine is 6.1.9 r11686 on Windows.

killergege commented 8 years ago

Also tested it. I tested all of the GPX generated by version 1.1 and they are all OK. Just to be sure I tried again version 0.9.9 and the Volbeat files were corrupted.

I guess it was one of my other edits that fixed it. Not sure which one though :). Or you did an edit that allowed the "reuse" code to be enabled again. I didn't analyse your recent commits.

Everything's OK then !

fholger commented 8 years ago

Glad to hear it, thanks for testing :)