nightingale-media-player / nightingale-hacking

Working tree for the community fork of Songbird, Nightingale. If building, use the sb-trunk-oldxul (development) branch, with the tag 1.12.1 tag for stable, for now. The master-xul-9.0.1 branch is the current progress in building Nightingale with XULRunner 9 and builds, but is broken. All help in terms of patches and pull requests is welcome.
https://getnightingale.com
GNU General Public License v2.0
185 stars 41 forks source link

MP3 ID3 Tag Data is not imported into Library Metadata if the source is a URL. #226

Open cptskippy opened 11 years ago

cptskippy commented 11 years ago

Version: Nightingale 1.12, Build 2432 (20130401204705) Operating System: Windows 7 Professional x64

I have my music library hosted on a web server and serve it up via HTTP. I have .m3u playlists that are basically just lists of the URLs to the .mp3 files.

In Songbird, if you imported a playlist that contained urls to .mp3 files, Songbird would download enough of the .mp3 files to parse the ID3 Tag Data. In Nightingale, if you import a playlist that contains urls to .mp3 files, Nightingale stores the #EXTINF line as the Title and stops. When the song is played back, the .mp3's ID3 Tag Data is parsed and displays in the playback bar but the Metadata is not updated. Nightingale correctly parses files from unc paths in .m3u files, just not urls.

Nightingale: image

Songbird: image

--- Want to back this issue? **[Post a bounty on it!](https://www.bountysource.com/issues/1060317-mp3-id3-tag-data-is-not-imported-into-library-metadata-if-the-source-is-a-url?utm_campaign=plugin&utm_content=tracker%2F230233&utm_medium=issues&utm_source=github)** We accept bounties via [Bountysource](https://www.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F230233&utm_medium=issues&utm_source=github).
freaktechnik commented 10 years ago

This might be related to #243

johnmurrayvi commented 10 years ago

This is easily reproducible... the unit tests fail for remote files, particularly in the MetadataManager tests.

This issue seems to be related to TagLib and moving to the unpatched version. POTI had the FileIO, LocalFileIO, and FileIOTypeResolver classes. None of the ChannelFileIO stuff was touched when the metadata handler was altered to support the unpatched version.

In particular, this frequently fails sbMetadataManager.cpp#L346.

johnmurrayvi commented 10 years ago

Hi @cptskippy, Would you mind trying one of these testing builds?

cptskippy commented 10 years ago

I installed Nightingale_1.12.2a_windows-i686-testing_taglib-1.7.2-patched_debug-tests.exe on a clean system, the installation went through but I got a number of NSGlue_Assertion pop ups during launch. I ignored through them and it eventually crashed before loading past the splash screen. I Relaunched and got numerous pop ups but the UI did eventually load.

I imported a simple playlist...

\#EXTINF:-1,ACDC - '74 Jailbreak - 01 - Jailbreak
http://my.domain.com/ACDC/%2774%20Jailbreak/01%20%2D%20Jailbreak.mp3
\#EXTINF:-1,ACDC - '74 Jailbreak - 02 - You Ain't Got A Hold on Me
http://my.domain.com/ACDC/%2774%20Jailbreak/02%20%2D%20You%20Ain%27t%20Got%20A%20Hold%20on%20Me.mp3
\#EXTINF:-1,ACDC - '74 Jailbreak - 03 - Show Business
http://my.domain.com/ACDC/%2774%20Jailbreak/03%20%2D%20Show%20Business.mp3
\#EXTINF:-1,ACDC - '74 Jailbreak - 04 - Soul Stripper
http://my.domain.com/ACDC/%2774%20Jailbreak/04%20%2D%20Soul%20Stripper.mp3
\#EXTINF:-1,ACDC - '74 Jailbreak - 05 - Baby Please Dont Go
http://my.domain.com/ACDC/%2774%20Jailbreak/05%20%2D%20Baby%20Please%20Dont%20Go.mp3
\#EXTINF:-1,ACDC - Back In Black - 01 - Hells Bells
http://my.domain.com/ACDC/Back%20In%20Black/01%20%2D%20Hells%20Bells.mp3
\#EXTINF:-1,ACDC - Back In Black - 02 - Shoot to Thrill
http://my.domain.com/ACDC/Back%20In%20Black/02%20%2D%20Shoot%20to%20Thrill.mp3
\#EXTINF:-1,ACDC - Back In Black - 03 - What Do You Do For Money Honey
http://my.domain.com/ACDC/Back%20In%20Black/03%20%2D%20What%20Do%20You%20Do%20For%20Money%20Honey.mp3
\#EXTINF:-1,ACDC - Back In Black - 04 - Giving The Dog a Bone
http://my.domain.com/ACDC/Back%20In%20Black/04%20%2D%20Giving%20The%20Dog%20a%20Bone.mp3
\#EXTINF:-1,ACDC - Back In Black - 05 - Let Me Put My Love Into You
http://my.domain.com/ACDC/Back%20In%20Black/05%20%2D%20Let%20Me%20Put%20My%20Love%20Into%20You.mp3

When the playlist was loaded the tracks were initially named using the EXTINF info but after a few minutes the list refreshed and all tracks just displayed their filename. Right clicking on a track and selecting View Metadata showed that the Name had been replaced with the filename.

Throughout clicking on tracks or other parts of the UI resulted in popups.

Is there a log file generated that would be of use to you? I looked around and didn't see one and tried logging the console out but it didn't capture the relevant output.

johnmurrayvi commented 10 years ago

Ah crap, I forgot to set a variable in the debug builds to stop those stupid popups :( There could be a log that gets created at /AppData/Roaming/Nightingale/console.log, but it may not be as useful. Would you mind trying the release version, and seeing how that is? The release build won't have all of those popups... Alternatively, if you are running the debug build through the console, you can set the variable XPCOM_DEBUG_BREAK=warn to prevent them as well. Thanks!

cptskippy commented 10 years ago

I uninstalled the test build, deleted all profile data, and installed the release version. It installed without a hitch. I then imported the same playlist.

Immediately after import, this is what I see in the Library...

Post Import

After a minute or so the Library clears and refreshes to this...

Moments Later

It looks like the Metadata fetch is failing and it's defaulting to the track's filename. Personally I'd prefer it keep whatever value is in there rather than replace it with the filename.

The console.log didn't appear to have anything related to this problem, it looks like it's all exception XUL logging. Let me know if you want it.

Tracks play back fine and show the correct Track time. It takes a moment to display the track time, I assume because it doesn't calculate it until it has downloaded the track completely.

johnmurrayvi commented 10 years ago

Huh, this is really interesting... If I can, I'll to try to reproduce the issue. I have a few ideas of what it might be, but I'm not certain now.

johnmurrayvi commented 10 years ago

Well this is weird. I can recreate the issue, or one very similar to this, even with the same TagLib POTI used... Which version of Songbird were you using from the screenshot in the initial post?

cptskippy commented 10 years ago

I'm not sure which was the last version I was using but it was a 2.x version, I'm also not sure what version I used initially but I'd upgraded several times over the years. I went back and installed versions 1.10.1-2160, 1.10.3-228, and 2.0.0-2311 to see if I could find out when the feature last worked. None of them worked, they're all behaving exactly like Nightingale so I'm at a loss.

During the time that I've used Songbird/Nightingale I have upgraded my server hosting the tracks twice, once from Windows 2003 to 2008r2, and then from 2008r2 to 2012r2. Unfortunately I can't verify if those upgrades coincide with the functionality breaking.

I'm attempting to capture the IP traffic from Nightingale and an early version of Songbird to see what sort of requests are going to my server and the responses.

johnmurrayvi commented 10 years ago

Interesting... POTI did release the 2.x code, so maybe something got changed in the metadata handler?

Here's what I've been able to recreate so far: I have a USB drive with an album on it (16 songs) plugged into my router and have it set up as an FTP server. I made an M3U playlist with ftp:// entries to the songs. I'll start with a clean profile/library, then import the playlist. No matter what order I have the entries in, or how many entries there are, the first four import the correct metadata, and the rest don't. It's really bizarre...