search

univrsal / tuna

Song information plugin for obs-studio
GNU General Public License v2.0
751 stars 59 forks source link

Album not returned for Web browser source #156

Closed mihawk90 closed 3 years ago

mihawk90 commented 3 years ago

Tuna version: 1.6.1 (just updated)

Describe the bug Currently no song information (except title) is returned for information coming from the Web browser source. This affects both the file-writing ability, as well as the JSON returned for the Widget. This unfortunately renders the Web browser source essentially non-functional. This actually also breaks the Widget completely (until #155 is merged).

To Reproduce

  1. Use Web browser source with any of the supported services (personally I was using Pretzel, but you'll need #154 for testing)
  2. Go to the Tuna settings and create the file with the format string Title: %t Artist: %m Track Number: %n Album: %a Progress: %p Length: %l Time Left: %o
    • you will already see a warning that "some" format options are not supported, but that is in fact shown for every single one (except the whitespace options), even when actually supported (like %t).
  3. check the file created, for example: 
    Title: Under Leaves Artist: m Track Number: n Album: a Progress: 0:44 Length: 2:17 Time Left: 1:33
  4. Go to http://localhost:1608 : 
    {
    "cover_url": "https://img.pretzel.rocks/artwork/Ljf7WM5/large.jpg",
    "duration": 137000,
    "progress": 71000,
    "status": "playing",
    "time_left": 66000,
    "title": "Under Leaves"
}

Expected behavior Ideally all submitted format options would be written to file and returned as JSON

Screenshots Funny enough, this is actually the only option that works: image

Log Not much Tuna related in there, but here we go: https://obsproject.com/logs/p3BL6-QoW79TwE9a

univrsal commented 3 years ago

Works fine for me with soundcloud, are you sure this is not an issue with pretzel? I did update the supported format options though. image

mihawk90 commented 3 years ago

Hm I'll have to look into that again later then 🤔

mihawk90 commented 3 years ago

OK so the artist was indeed an issue with my Pretzel implementation. That's what I get for not copy/pasting code for once ;) I didn't realise Tuna expected an array of artists.

So that's fixed now in the linked commit.

However, I can still not get the Album to show, and that doesn't seem to be working in your Screenshot either. The soundcloud code clearly defines and sends the album:

https://github.com/univrsal/tuna/blob/92d2bb9006385f85c0af4d94c817f899c85af952/deps/tuna_browser.user.js#L91-L94

But it is not in the JSON either.

Is this dependant on 8861a41 ? I wouldn't think so as Artist works just fine without it as well.

univrsal commented 3 years ago

No I think those are only used for the format dialog right now.

mihawk90 commented 3 years ago

Updated the issue title since it's only the album title now.

Artist was resolved because of my mistake and track number isn't supported at all (which arguably is fine because I don't think any web player even lists track numbers). But it doesn't seem Album is working properly.

univrsal commented 3 years ago

Okay, so the metadata is checked when writing the song to json, which should probably be simplified a bit. Album is just plain missing here, which is an easy fix.

mihawk90 commented 3 years ago

Mh coming back to this, if I understand correctly just copying and changing the blocks above should be enough right? But that gets me thinking are we using album? album_url? both? The browser script still has mixed usage. scratch that, it's not mixed, but just confusingly named, the value is sometimes URL sometimes name though. I could technically do both, but the file-write only supports one of them anyway (whichever that is). I could technically also check for both and use whatever is available (preferring album over album_url?).

Ideally we should probably clean up the Browser script though if one of them is unused (as per your comment here), or decide on actually using both if available.

What's your preference?

univrsal commented 3 years ago

The capability is called cover but I think that cover_url is more precise, as it will always either be http(s)://link.to/cover.png or file:///link/to/cover.png. So it should probably be cover_url everywhere which would require changes here:

mihawk90 commented 3 years ago

Yeah cover makes sense, that's not a question :)

I was more referring to album and album_url. Should I include another block for album_url so we can eventually use both? Or just leave it out for now (and therefore only sending album makes sense in the script)?

edit: Actually, I'm dumb... there's no set_album_url() or anything anyway so I couldn't even do that...