Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: SWS/IX: Import m3u/pls playlist - crash when cancelling file loading dialog #1041

Closed
nofishonfriday opened this issue Aug 23, 2018 · 6 comments

Comments

@nofishonfriday
Copy link
Collaborator

@nofishonfriday nofishonfriday commented Aug 23, 2018

m3u crash

(click image for bigger version)

cfillion added a commit to cfillion/sws that referenced this issue Aug 23, 2018
Also fixes a memory leak of the temporary filename buffer returned by
BrowseForFiles when the import is not cancelled.

(closes reaper-oss#1041)
@cfillion
Copy link
Collaborator

@cfillion cfillion commented Aug 23, 2018

It was also leaking the filename buffer when not cancelling...

@nofishonfriday
Copy link
Collaborator Author

@nofishonfriday nofishonfriday commented Aug 23, 2018

Hah, you were a tad faster. 🔫 I was just about to open my fix PR when I noticed yours (missed the leaking though).,
Thanks.

edit:
Scratch below, I think I misunderstood the functionality of the 'No' option.

===

While at it, I found another minor quirk, when clicking 'No' in the 'Cannot find some files. Create items anyway?' dialog it creates the items anyway.

case 7 : // No

should be 'return' instead of 'break' in line 268 I think.
Would you add that too to your PR ?

@cfillion
Copy link
Collaborator

@cfillion cfillion commented Aug 23, 2018

Oh sorry! I wasn't sure whether you wanted to fix it...

Maybe the message box should instead say "The following files cannot be found. Create items for them anyway?" to be clearer.

@nofishonfriday
Copy link
Collaborator Author

@nofishonfriday nofishonfriday commented Aug 23, 2018

No problem, doesn't matter to me who fixes something in the end. :)

Maybe the message should instead say "The following files cannot be found. Create items for them anyway?" to be clearer.

That's what I thought it should do, but here I import a playlist where files are not found, when I click 'No' it still creates the (offline) items, that's what confuses me.

m3u test

@cfillion
Copy link
Collaborator

@cfillion cfillion commented Aug 24, 2018

Looks like another bug to me! I'm guessing PCM_Source_CreateFromFile used to return NULL at the time this feature was written when the file didn't exist.

@nofishonfriday
Copy link
Collaborator Author

@nofishonfriday nofishonfriday commented Aug 24, 2018

This seems to fix I think:
L 296:
if(pSrc && pSrc->GetSampleRate() > 0 || includeMissing)

edit:
Nevermind, just saw that you pushed a fix already, thanks.

@swstim swstim closed this in e995634 Feb 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants