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] Fix GetItemsToScan scan in sub-directories #17036

Merged
merged 1 commit into from
Jan 8, 2020
Merged

[videoplayer] Fix GetItemsToScan scan in sub-directories #17036

merged 1 commit into from
Jan 8, 2020

Conversation

bahusoid
Copy link
Contributor

@bahusoid bahusoid commented Dec 13, 2019

GetItemsToScan currently always scans for subtitles in additional directories. It should scan for requested extensions instead.

Fixes #17014 (technical details here). Verified on this win32 test build

Also fixes scan for external audio tracks (ScanForExternalAudio) and .sup subtitles (ScanForExternalDemuxSub) in sub-directories for video

@bahusoid bahusoid changed the title Remove subtitles scan from GetItemsToScan [videoplayer] Fix GetItemsToScan scan in sub-directories Dec 13, 2019
@jjd-uk
Copy link
Member

jjd-uk commented Dec 16, 2019

I can't help reviewing this, but I note you seem to have done this blind and have not tested the fix for yourself. If you've not been able to generate your own test build we can kick one off for you if you let us know what OS you need the build for (as long as it's not Linux).

@bahusoid

This comment has been minimized.

@jjd-uk
Copy link
Member

jjd-uk commented Dec 16, 2019

Ok I've used our Jenkins CI system to kick off a build, see https://jenkins.kodi.tv/view/Helpers/job/BuildMulti-PR-Manually/310/ for status, once it reports successful build (normally 30min to complete) it will upload to http://mirrors.kodi.tv/test-builds/windows/ and it will have PR number in filename, but it may take up to another 30 min for to appear on your local mirror.

@bahusoid
Copy link
Contributor Author

bahusoid commented Dec 16, 2019

@jjd-uk Thanks

I confirm it really fixes #17014 - chapters are back in test build.

It also would be good to fix it in Leia too. Seems I need to create a backport pr for Leia branch. Am I right?

@jjd-uk
Copy link
Member

jjd-uk commented Dec 16, 2019

Let's wait first for someone to review what you've done, I'm not one of the core devs so can't do that.

@bahusoid

This comment has been minimized.

@arnova
Copy link
Member

arnova commented Dec 24, 2019

Looks sane to me, @ace20022 ?

@ace20022
Copy link
Member

Looks sane to me, @ace20022 ?

to me too ;)

@ace20022 ace20022 self-requested a review December 30, 2019 13:44
@jjd-uk
Copy link
Member

jjd-uk commented Jan 4, 2020

Jenkins build this please

@jjd-uk jjd-uk added Component: Video Type: Fix non-breaking change which fixes an issue v19 Matrix labels Jan 6, 2020
@jjd-uk
Copy link
Member

jjd-uk commented Jan 6, 2020

@arnova would you mind hitting merge if this is ok with you as @ace20022 has approved, and I assume the Win UWP build failure is down to the known python 3 issues with UWP.. As I'm not a C++ guy not comfortable doing it myself.

@bahusoid
Copy link
Contributor Author

bahusoid commented Jan 7, 2020

@arnova Yeah can you please merge it and add backport related labels (backport pr - #17098). Thank you!

@jjd-uk
Copy link
Member

jjd-uk commented Jan 7, 2020

As Win UWP build issues are now fixed in master I'll do another...

Jenkins build this please

@arnova arnova merged commit 152988f into xbmc:master Jan 8, 2020
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Jan 21, 2020
[videoplayer] Fix GetItemsToScan scan in sub-directories
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Video Type: Fix non-breaking change which fixes an issue v19 Matrix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Chapters are missing in mkv files with external subtitles loaded from Subs sub-folder
4 participants