Fix Android song unable to find a specified URI

[Mindfulplays]

Changed Android Song to differentiate between Assets/URI-based DataSource.

@tomspilman @mrhelmut

[mrhelmut]

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

[Mindfulplays]

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

OpenFd throws if the file is not found (it's for a file I am Playing outside the ContentManager system). I do agree its ugly but this lets me use MG Song with my own files (I wonder if this is useful at all to others? If not I can close this PR).

[Mindfulplays]

Squashed commits to ease PR review, thanks!

[dellis1972]

Can we make use of Uri.TryCreate to query if the argument is a Uri then call the appropriate method?

Uri.TryCreate ("http://foo", UriKind.Absolute, out Uri result);

This code above will return true in this case, but false if you just passed say Assets/foo.xnb

[Mindfulplays]

Can we make use of Uri.TryCreate to query if the argument is a Uri then call the appropriate method?

Uri.TryCreate ("http://foo", UriKind.Absolute, out Uri result);

This code above will return true in this case, but false if you just passed say Assets/foo.xnb

This is an Android problem not an dotnet Uri problem :( Android assumes that you know that you know the file you are accessing. The Uri itself would still be considered valid either way.

Or perhaps I am missing something else?

[dellis1972]

So the Pr description says this is about knowing the difference between an "Asset" and a "Uri".
The problem is Game.Activity.Assets.OpenFd(_name); will throw if passed a Uri rather than an "Asset".
Things retrieved via the ContentManager do not usually have a Uri based filenames if I remember rightly. Its more like "Assets/songs/mysong.xnb" , which Uri.TryCreate will return false for.
If that is the case we could do

if (!Uri.TryCreate ("http://foo", UriKind.Absolute, out Uri result)) {
    var afd = Game.Activity.Assets.OpenFd(_name);
    if (afd != null)
       _androidPlayer.SetDataSource(afd.FileDescriptor, afd.StartOffset, afd.Length);
} else {
    _androidPlayer.SetDataSource(_name);
}

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 adb logcat output to figure out what is happening.

So you have some samples of the kinds of values that get passed to _name in this case? one for assets and the other for direct file access?

[mrhelmut]

Couldn't we just have something like _name.StartsWith("file://") to differentiate content from external files?

            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);
            }

[Mindfulplays]

Good idea @mrhelmut fixed, please take a look.

[dellis1972]

Looks ok to me 👍

[dellis1972]

@mrhelmut can we get one last look at this one please? If its good, I'll merge it.

[Mindfulplays]

Gentle ping...

[mrhelmut]

I'm a bit on the fence with the unnecessary changes I commented.

[Mindfulplays]

FWIW, the code 'works' :) but yeah, lmk if it might break existing code.