-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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
[Omega] Fix loading ISO files on Android arm-v7a #25156
Conversation
The builder has ignored the argument:
Locally it has worked, any advice? |
No idea, ping @fuzzard |
Maybe |
Format is correct (also seen here) as it works locally. Maybe when configuring it's redefined, better wait for ping |
Works! thank you both |
My concern is what is disabling the iso9660p library actually going to do? I dont understand how this is a new problem in Omega over Nexus. Nothing has changed regarding libcdio in Omega as far as i could see. is there something specifically changed in Omega that somehow caused this? Does this even work in Nexus? |
I've checked that the last release that opens an iso file is 18.9, at that time libudfread was used. From 19.0 onwards crashes with the invalid argument message in libcdio was added in 19.0 (#16262) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested on Sony TV (arm-v7a): tested DVD ISO from USB storage and SMB.
Also tested Blu-Ray BDMV folder and BD ISO from SMB for possible regressions. All is working fine.
apart from fuzzard's concerns.. wasn't the error only happening when trying to open an ISO from internal storage? not SMB/USB |
USB also was crashing for me in this Sony TV with Play Store version |
What about making the UDF implementation higher priority? xbmc/xbmc/filesystem/FileDirectoryFactory.cpp Line 158 in 161ccd0
|
Could also be fixed in this way, but it would affect all platforms. To reduce the risks better the current solution that only affects Android arm-v7a. |
I would check if that solves the problem. Also has this change been made to master? I see it's PR'd to the Omega branch. |
Master targets Android API 24, at this level |
Could libcdio be patched to fix the issue? |
A 64-bit fseeko function or something similar should be implemented, but at the moment since it doesn't affect Master and there is a workaround that works for v21 I don't think it's worth. |
@lrusak reasoning @joseluismarti fine with you? Given that it is about v21 stable? |
I don't really care, I was just suggesting alternatives. There was some discussion in #android-external on slack about alternatives. I'm not sure where that got to though and ultimately it may be up to @fuzzard |
Yeah - also not my usecase. So whatever was the winner of the discussion should make it. |
there was no real winner. i just raised the concern that disabling iso9660pp could probably break playback of physical dvd‘s… but android is also not my use case nor can i test dvd playback anymore. not an easy decision. maybe merge as is and revert if shit happens 🤷 |
I also don't use this feature just to search for the fix, in the tests I've done I can see the content of the iso file and the playback also works. The reporter and someone else confirm that it works, apparently well. |
Good - if we all don't know. Let's do what @howie-f suggested. |
Thanks everyone involved. |
Description
There is an issue on 32-bit Android devices when trying to open an ISO file from the internal storage, kodi force closes and displays the following debug message:
The
libcdio
library is calling thefseeko
function with a 64-bitoff_t
argument and this function is not available in Android API 21, was added in 24.The fix would be to target 24 as done in v22.
I propose a workaround to continue at level 21 in v21: disable
iso9660pp
so thatlibudfread
is used to open ISO files, similar to how network ISO files are opened.How has this been tested?
Tested to open and play a DVD ISO file on a 32-bit Android device.
Types of change
Checklist: