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

VideoPlayer: catch silly CRedirectException, fixes crash #10387

Merged
merged 1 commit into from Sep 4, 2016

Conversation

@FernetMenta
Copy link
Member

commented Sep 3, 2016

fix http://trac.kodi.tv/ticket/16874

We really should not deal with C++ exceptions as long catch blocks can't be forced.

@MilhouseVH /cc

@MilhouseVH

This comment has been minimized.

Copy link
Contributor

commented Sep 3, 2016

Tested on RPi2, problem appears to be fixed - thanks.

@arnova

This comment has been minimized.

Copy link
Member

commented Sep 3, 2016

@FernetMenta : IMHO the proper fix for this should not use CCurlFile open/close to het the final URL but just have a separate function in CCurlFile (without the exception part) giving us the final URL. Seems much cleaner ?

@FernetMenta

This comment has been minimized.

Copy link
Member Author

commented Sep 3, 2016

@arnova not sure what exactly you are proposing. Do you have time to do this?

@arnova

This comment has been minimized.

Copy link
Member

commented Sep 4, 2016

@FernetMenta : Working on it. I think in half an hour or so.

@arnova

This comment has been minimized.

Copy link
Member

commented Sep 4, 2016

@FernetMenta : What I had in mind doesn't seem that straightforward after all. But it still feels like we shouldn't be bothered with handling such exceptions at this level?

@FernetMenta

This comment has been minimized.

Copy link
Member Author

commented Sep 4, 2016

@arnova not sure if I got your question. What level do you mean?

@arnova

This comment has been minimized.

Copy link
Member

commented Sep 4, 2016

VideoPlayer level. The redirect was implemented as a hack for IFile, therefor it's handled there. Anything else not going through IFile, requires special treatment which sucks imho.

@FernetMenta

This comment has been minimized.

Copy link
Member Author

commented Sep 4, 2016

yes, VideoPlayer only need to catch the error.
Are you ok with this PR now?

@arnova

This comment has been minimized.

Copy link
Member

commented Sep 4, 2016

I think I can live with it ;-), although it's not as pretty as I would like (note: can't think of anything else atm)...

@FernetMenta

This comment has been minimized.

Copy link
Member Author

commented Sep 4, 2016

@arnova thanks. still enough work to keep you busy in the future :)

@FernetMenta FernetMenta merged commit ffad046 into xbmc:master Sep 4, 2016
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@FernetMenta FernetMenta deleted the FernetMenta:16874 branch Sep 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.