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
Fix Android song unable to find a specified URI #8111
base: develop
Are you sure you want to change the base?
Conversation
Not a big fan of a try/catch within a try/catch and exception being silenced. Suggestion: var afd = Game.Activity.Assets.OpenFd(_name);
if (afd != null)
_androidPlayer.SetDataSource(afd.FileDescriptor, afd.StartOffset, afd.Length);
else
_androidPlayer.SetDataSource(_name); // assuming that if anything is wrong here, the user did something wrong with their URI and we shouldn't silently handle the exception to let the user know |
|
76c7e64
to
043ea41
Compare
Squashed commits to ease PR review, thanks! |
Can we make use of
This code above will return |
This is an Android problem not an dotnet Or perhaps I am missing something else? |
So the Pr description says this is about knowing the difference between an "Asset" and a "Uri".
I could be wrong about the Uri, its been a while since took a look at that code. On a side note its not really good practice to swallow exceptions in a library if you can help it. There are a number of reasons why a file might not exist. The main one on Android is someone using the incorrect Filename casing as its a case sensitive file system. If we swallow the exception the developer will have no idea what happened in code, the only hint would be the Song not playing. Not everyone will read the So you have some samples of the kinds of values that get passed to |
Couldn't we just have something like if (assetUri != null)
{
_androidPlayer.SetDataSource(MediaLibrary.Context, this.assetUri);
}
else if (_name.StartsWith("file://"))
{
_androidPlayer.SetDataSource(_name);
}
else
{
var afd = Game.Activity.Assets.OpenFd(_name);
if (afd == null)
return;
_androidPlayer.SetDataSource(afd.FileDescriptor, afd.StartOffset, afd.Length);
} |
- `Assets.OpenFd` throw if the given filename does not exist. - So a hack to try `OpenFd` if not `SetDataSource` next with cascading `try/catch`.
043ea41
to
8e6782e
Compare
Good idea @mrhelmut fixed, please take a look. |
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.
Looks ok to me 👍
@mrhelmut can we get one last look at this one please? If its good, I'll merge it. |
Gentle ping... |
I'm a bit on the fence with the unnecessary changes I commented. |
FWIW, the code 'works' :) but yeah, lmk if it might break existing code. |
Possibly then, also include a Can you also add a Unit test around this feature (or validate with an existing test)? @Mindfulplays |
Changed Android
Song
to differentiate between Assets/URI-basedDataSource
.@tomspilman @mrhelmut