Aldaviva / WinampNowPlayingToFile

⚡ When Winamp plays a track, this plugin will save the track metadata and album art to files.
Apache License 2.0
17 stars 1 forks source link

Unhandled exception, Album Art #5

Closed Dainah-0xdeadc0de closed 1 year ago

Dainah-0xdeadc0de commented 2 years ago

Plugin has problems when playing songs from SOME Web Stations online (not all). Errors will popup infinitely from the station while it's playing.

`Unhandled exception while updating song info on song change: System.ArgumentException: URI formats are not supported.

at TagLib.File.Create(IFileAbstraction abstraction, String mimetype, ReadStyle propertiesStyle)

at WinampNowPlayingToFile.Business.NowPlayingToFileManager.extractAlbumArt()

at WinampNowPlayingToFile.Business.NowPlayingToFileManager.update()`

Aldaviva commented 2 years ago

Oh, thanks for the report and stack trace. I hadn't considered that case, but it makes sense why that's a problem. I'll try to fix that.

Aldaviva commented 2 years ago

I was able to reproduce this issue with the Dance Wave! Icecast MP3 stream, found in the SHOUTcast Directory.

error message windows

I'm not sure what makes this stream different from others which don't throw this exception.

http://135.125.239.164:8080/dance.mp3
Network received: 151207 bytes
Server: Icecast 2.4.0-kh15 (MSCP)
Content-Type: audio/mpeg
Metadata received: 73 bytes
Metadata interval: 16384 bytes
Stream name: Dance Wave!
Current title: All about Dance from 2000 till today!

This other stream is also using an MP3 encoder, but does not throw an ArgumentException:

http://stream.antenne.de:80/chillout
Network received: 776287 bytes
Server: streaMonkey streaming Server Native
Content-Type: audio/mpeg
Metadata received: 304 bytes
Metadata interval: 16000 bytes
Stream name: ANTENNE BAYERN Chillout
Current title: El Profesor - Bella Ciao (HUGEL Vocal Radio Cut)

Maybe if the stream URI ends with .mp3, then TagLib throws an ArgumentException? When I added a unit test, it threw a NotSupportedException instead, with the message The given path's format is not supported..


TagLib has a nasty habit of throwing a huge range of undocumented exceptions. There were already 4 I knew about, but now that there are 6 I'm just going to start catching the Exception superclass because adding a new case for each unpredictable exception is untenable.

Aldaviva commented 2 years ago

This should be fixed in 1.0.5.0. Give that a try and let me know if it's working. It did fix the error messages for the stream I found which used to reproduce the problem.

Luna-Nocturna commented 1 year ago

While the original poster didn't respond I'm afraid to I'm also getting these errors when listening to any audio stream. Even when using the latest (1.1.0.0) version.

Aldaviva commented 1 year ago

OK, are you able to share one or more URLs of streams that cause this issue? I can try to reproduce and fix this then.

Luna-Nocturna commented 1 year ago

That's a lighting fast response, thanks!

A good example would be any of the MP3 streams from Rainwave ( https://rainwave.cc/ ). I'm aware they do create unique URL's in their m3u depending if you're logged in or not. Though I've just tested it also seems to occur when not logged in and you get the default ones without your unique string.

Aldaviva commented 1 year ago

Stream URLs

Playlist: https://rainwave.cc/tune_in/5.mp3.m3u MP3 stream: https://relay.rainwave.cc:443/all.mp3

Error message

Unhandled exception while updating song info on song change:
System.ArgumentException: The path is not of a legal form.
   at System.IO.Path.LegacyNormalizePath(String path, Boolean fullCheck, Int32 maxPathLength, Boolean expandShortPaths)
   at System.IO.Path.NormalizePath(String path, Boolean fullCheck, Int32 maxPathLength, Boolean expandShortPaths)
   at System.IO.Path.InternalGetDirectoryName(String path)
   at System.IO.Path.GetDirectoryName(String path)
   at WinampNowPlayingToFile.Business.NowPlayingToFileManager.findAlbumArtSidecarFile(Song currentSong)
   at WinampNowPlayingToFile.Business.NowPlayingToFileManager.findAlbumArt(Song currentSong)
   at WinampNowPlayingToFile.Business.NowPlayingToFileManager.update()

Since this is happening when trying to find the album art file, I think this is caused by #9.

Strangely, when this same URL is passed to NowPlayingToFileManager.findAlbumArtSidecarFile(Song) in a unit test, the code under test throws a different exception (NotSupportedException) which is already being handled correctly.

Aldaviva commented 1 year ago

There were two different sources of ArgumentException when playing a stream URL.

  1. One was being thrown by the call to Path.GetDirectoryName(string) when the argument is the empty string, which Winamp sometimes briefly emits for stream URLs (possibly as the playlist is being loaded, or as the stream's buffer is being initially filled). I added a pattern match to skip saving now playing metadata in this case. The actual metadata will get saved soon after when the stream actually begins playing.
  2. The other was being thrown by the call to new DirectoryInfo(string) because the plugin and unit tests were running with different values for the Switch.System.IO.UseLegacyPathHandling AppContext switch.
    • For the plugin running inside Winamp, the value of this switch was being set to true for some reason (I assume a registry value, a side effect of the C++/CLI bridge, or how the SharpAmp framework was targeted or compiled). This made the passing of URIs to new DirectoryInfo(string) throw an ArgumentException, which was not handled by the plugin.
    • For the plugin running inside unit tests, the value of this switch was being set to false which is the normal, expected value. This makes passing URLs to new DirectoryInfo(string) throw a NotSupportedException, which this plugin has handled correctly since version 1.1.0. Therefore, tests were passing.

I'm fixing this by forcing Switch.System.IO.UseLegacyPathHandling to false in the plugin, which will make the plugin behave the same in this respect when run inside Winamp as when run inside unit tests.

I've added more URLs to the unit tests and also tested this manually with the Rainwave MP3 stream.

As a side note, I also learned that Winamp silently refuses to download playlist files from URLs whose paths end with .m3u or .m3u8, and that you must instead download the playlist using another user agent (like a browser or your operating system) and then load that into Winamp as a local file instead of as a URL.

Aldaviva commented 1 year ago

Both #5 and #11 should be fixed in the development snapshot build 24. You can download that and let me know if they're both working for you. If so, I'll release it as 1.2.0.0.

Luna-Nocturna commented 1 year ago

Both #5 and #11 should be fixed in the development snapshot build 24. You can download that and let me know if they're both working for you. If so, I'll release it as 1.2.0.0.

I can confirm I won't get pop ups with multiple types of streams any more. Great job!

However and I hate to be the bringer of bad news... The plugin seems to ignore my emptyAlbumArt.png in the Winamp installation directory. I've also tried putting it in the output directory as I made a custom one for it together with all my other OBS assets seeing if that was a solution but it stills outputs a 1x1 black pixel instead of the emptyAlbumArt.png I made.

Aldaviva commented 1 year ago

Hmm okay thanks I'll take a look at that.

The plugin looks for that file in the current working directory of the Winamp process. Usually this is the Winamp installation directory, but it can be different if you customized the shortcut you use to launch Winamp. I usually verify the CWD using Process Explorer. Or maybe that feature is just broken.

Luna-Nocturna commented 1 year ago

I usually have it start on it's own opening music/playlist files. It's working CWD is the default in my case: C:\Program Files (x86)\Winamp

I may have found the issue: As I explained I have it output to my assets folder which is a sub folder of my OBS installation. Launching Winamp elevated does make the emptyAmbumArt.png work. Most likely because usually you need Admin Privileges when messing with files in Program Files.

However the normal art in txt work fine without elevation. As a solution I can choose to output elsewhere though. I've been considering to put my assets elsewhere as a whole as using Program Files is kind of a bad practice nowadays. It's just OBS often seems to default to it's main folder with it's opening wizards rather than last opened ones.

Luna-Nocturna commented 1 year ago

Update: It's not the OBS Directory that it needed the elevation for but Winamp it's own directory. If it's not elevated it's not allowed to access emptyAlbumArt.png. So moving around my assets and plugin outputs elsewhere won't be a solution. Installing Winamp outside Program Files (x86) where it doesn't require elevation would be a solution but not really preferable.

Aldaviva commented 1 year ago

Interesting, thanks for reporting these findings. I'm not sure why the plugin is exhibiting this behavior when not elevated, but I can try to reproduce it and fix it (I run Winamp elevated so I haven't encountered this issue before). I assume emptyAlbumArt.png doesn't have any custom permissions set, aside from the inherited ACL from the Winamp installation directory.

Aldaviva commented 1 year ago

I installed Winamp and this plugin in a virtual machine where the user is in the Administrators group but does not run programs elevated by default.

The issue does not reproduce, so emptyAlbumArt.png in the Winamp installation directory is successfully copied to the destination file. Winamp is not being run elevated in this test, and emptyAlbumArt.png has only default permissions, inherited from its parent directory, which includes Read & Execute assigned to Users from C:\Program Files (x86)\.

Windows Server 2019-2023-07-15-04-44-50

If you'd like, here is a development build that shows an error dialog with an exception and its stack trace when emptyAlbumArt.png can't be read.

Luna-Nocturna commented 1 year ago

Well this is embarrassing... no idea what I did differently or if there's something I done wrong earlier. When trying the dev build it worked flawless. Swapping back to the snapshot it also works without problems now. I'm sorry for having investigate a non-issue.

I'd also like to thank you once more for making this plugin, it really does what I had been looking for!

Aldaviva commented 1 year ago

It's all good, that's a perfectly fine outcome.

I'm glad you like this plugin and that it's useful for you!