Add various file sanity checks

[Naxesss]

Makes further progress on #12091

Note: The below code is used in both CheckAudioInVideo and CheckTooShortAudioFiles. I tried declaring a function in IWorkingBeatmap and implementing it in WorkingBeatmap, but something to do with the Mock<TestWorkingBeatmap> inheritance/setup caused a few of the tests to fail and I still haven't managed to figure out why, so ended up undoing that.

Stream data = context.WorkingBeatmap.GetStream(storagePath);
var fileCallbacks = new FileCallbacks(new DataStreamFileProcedures(data));
int decodeStream = Bass.CreateStream(StreamSystem.NoBuffer, BassFlags.Decode, fileCallbacks.Callbacks, fileCallbacks.Handle);

---

The verify tab now detects 0-byte files, audio tracks in video files, and too short audio files. This saves the time needed to otherwise open the properties and audio of each individual file to check for these Ranking Criteria violations.

[peppy]

Are you able to trim the test videos/files down to be 100kb or less? Will require a force push as well. Rationale is to avoid bloating the git repository for the sake of test data alone.

[Naxesss]

Good call, forgot videos can be quite heavy.

[peppy]

Going to block this as a personal reminder that it needs to be rebased/force-pushed before merge.

[peppy]

@Naxesss have you looked into the linux test failures on these? likely bass related.

[Naxesss]

Just looked at them now and I'm not really sure what's going on. I also think it's very likely bass related. Ideally I'd just tell the user that they failed parsing the audio, but since I get the same error code regardless of successfully finding no audio or failing to parse existing audio, that's not going to work (as far as I tested earlier anyway).

The way Mapset Verifier handles this is using a separate library called TagLib which basically checks whether the video has audio tags, and that probably works on Linux as well. Is that worth pursuing you think? I believe it's also used for things like video resolution checks.

[peppy]

@smoogipoo when you find a moment can you take a look at this and see if you have any quick ideas/thoughts? curious if the issue repros locally.

[bdach]

Not @smoogipoo but this does reproduce locally on linux. The video with audio in question has a MPEG-4 AAC audio track, which is not supported on linux without installing extra software. Docs of BASS_StreamCreateFileUser also hint that linux cannot read AAC audio streams. I couldn't even play the video outside of VLC without first doing

$ sudo apt-get install gstreamer1.0-libav

Installing that still doesn't make the test pass, though. Unsure why is that, as BASS docs above claim that system-wide fallbacks will be tried after its internal codecs/plugins.

[smoogipoo]

It's weird to be relying on system libraries in the first place.

[peppy]

If it's the video check which is failing, can we potentially use the existing VideoDecoder (maybe expose a flag like HasAudio) instead of bass?

[bdach]

I've had a very quick and dirty look at whether VideoDecoder could be used. It does detect the audio stream on linux, but I used VideoDecoder directly, which is not valid, as I should be going through GameHost.CreateVideoDecoder(), which is not easily available from within a check - unless passed in manually?

And even with that I did not test how this would behave on other platforms.

[peppy]

To bump this, I think using taglib is a good direction. I seem to recall seeing a recent c# (managed) project which does similar things to taglib, but can't seem to find it today.

[Naxesss]

The audio-in-video check now uses TagLib-Sharp instead of ManagedBass, and that seems to work on Linux given the CI results. Also makes the code more readable and handles exceptions better.

Had to add a StreamFileAbstraction class (extending TagLib.File.IFileAbstraction) to access the file through its Stream object, as opposed to its full filepath. Have put that in the osu.Game.IO.FileAbstraction namespace.

[peppy]

This will need testing on android and iOS.

[peppy]

I've refactored the disposal to be in line with expectations. We generally avoid leaving disposal to finalizers ever.

[bdach]

Willing to go test android, just didn't manage to get to it in time today.

[peppy]

Approving pending android testing for merge.

[bdach]

Have tested android on two devices and it doesn't crash, but I did spot a rather elementary oversight (storyboard is not null checked in one of the checks). Doesn't crash but it's ugly.

Will be pushing a fix for it shortly.

[bdach]

Actually, on a re-check, it's more of a problem with BeatmapManagerWorkingBeatmap and kind of out of scope here, I think? The problematic case I found is as follows:

1. Launch game
2. Enter editor (as if trying to create a new beatmap)
3. Without touching anything or saving the beatmap, enter the "verify" tab
4. A "storyboard failed to load" notification will appear.

This happens because BeatmapManagerWorkingBeatmap attempts to read the storyboard from an .osu file that does not exist yet:

osu/osu.Game/Beatmaps/WorkingBeatmapCache.cs

Lines 194 to 200 in 47b6fb0

	protected override Storyboard GetStoryboard()
	{
	Storyboard storyboard;
	try
	{
	using (var stream = new LineBufferedReader(GetStream(BeatmapSetInfo.GetPathForFile(BeatmapInfo.Path))))

BeatmapInfo.Path is null there, causing the exception.

I think that scenario can be fixed separately? It's kinda orthogonal to the implementation of the check and will be failing everywhere else if someone tries to access a storyboard of an unsaved beatmap. I'm going to tentatively approve this on that basis, but I am definitely interested in @peppy's opinion on this as well.

[peppy]

let's open an issue and fix post realm