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

Fixes a crash when a file's embedded art is invalid #15605

Merged
merged 1 commit into from Mar 1, 2019

Conversation

Projects
None yet
4 participants
@hugegreenbug
Copy link
Contributor

commented Feb 25, 2019

Description

If a file's metadata contains embedded art and that art is not valid, kodi will crash when a user tries to edit the thumbnail from the Context Menu -> Information -> Choose Art -> Thumbnail. This patch checks to see that the m_info object is not null to prevent a crash.

Motivation and Context

This change is required to fix a crash when editing the thumbnail of a movie/episode that has invalid embedded art.

How Has This Been Tested?

Tested on MacOS with an mkv that has invalid embedded art and scanned to the library as a movie. Then from the movie library and the movie's context menu Information -> Choose Art -> Thumbnail was selected. This would previously cause the crash. With the patch, there is no longer a crash.

Screenshots (if appropriate):

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Clean up (non-breaking change which removes non-working, unmaintained functionality)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that will cause existing functionality to change)
  • Cosmetic change (non-breaking change that doesn't touch code)
  • None of the above (please explain below)

Checklist:

  • My code follows the Code Guidelines of this project
  • My change requires a change to the documentation, either Doxygen or wiki
  • I have updated the documentation accordingly
  • I have read the Contributing document
  • I have added tests to cover my change
  • All new and existing tests passed
@pkerling

This comment has been minimized.

Copy link
Member

commented Feb 25, 2019

It is not immediately clear to me how invalid art would cause m_info to be nullptr. Can you expand on that briefly?

@hugegreenbug

This comment has been minimized.

Copy link
Contributor Author

commented Feb 25, 2019

I am not sure if the invalid art is causing m_info to be nullptr. After I removed the art in the metadata, the crash stopped even without the patch. That could be because the metadata was re-written. I will try to find the true source. Either way though, the patch just makes sure that m_info is not null, which can happen.

@pkerling

This comment has been minimized.

Copy link
Member

commented Feb 25, 2019

It depends. If m_info shouldn't be nullptr (like the code currently indicates), the crash is a good thing and the problem must be fixed at the root instead. I'd have to dig deeper to see whether that is true of course (I don't normally touch that part of the code). Are you fine with putting this on hold until you found the root cause so we can reason about it?

@Rechi

Rechi approved these changes Feb 25, 2019

@MartijnKaijser MartijnKaijser merged commit a50b1df into xbmc:master Mar 1, 2019

1 check passed

default You're awesome. Have a cookie
Details

@MartijnKaijser MartijnKaijser added this to the Leia 18.2-rc1 milestone Mar 2, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.