strawberrymusicplayer / strawberry

:strawberry: Strawberry Music Player
https://www.strawberrymusicplayer.org/
GNU General Public License v3.0
2.55k stars 165 forks source link

Add Support Back for .spc #991

Closed brandflake11 closed 1 year ago

brandflake11 commented 2 years ago

For technical issues, questions and discussion please use the forum on https://forum.strawberrymusicplayer.org/ Any issues related to feature requests will be closed. See the README for more details.

Check the Changelog to see if the issue is already fixed: https://github.com/strawberrymusicplayer/strawberry/blob/master/Changelog

If it's fixed, try the latest development build from: https://builds.strawberrymusicplayer.org/

Describe the bug In Clementine, .spc files are completely supported and added to the music library. I would love to use Strawberry instead, as it is updated and has smoother playback. However, I really like having my .spc files in my library. Could you add support back into Strawberry?

To Reproduce Add a music library with .spc files and notice it doesn't add them. Clementine does though.

Expected behavior Searching for music will add .spc files to your library and find appropriate metadata, just like in Clementine

Screenshots: If applicable, add screenshots to help explain your problem.

System Information:

Additional context Clementine has support for spcs. I'm hoping this was just overlooked and not unsupported. Strawberry has support for vlc playback and vlc can play .spc files. Maybe it's just a matter of allowing Strawberry to add .spcs to its library? I don't know, but I am willing to do anything that is needed to help.

jonaski commented 1 year ago

I am aware. It was added to Clementine in 2018 by @Eoin-ONeill-Yokai, long after I forked. The code is here: https://github.com/clementine-player/Clementine/blob/master/ext/libclementine-tagreader/gmereader.cpp I am not familiar with the SPC format so I won't work on it, even if I was, it's not a priority. @Eoin-ONeill-Yokai A PR is welcome if you are willing to work on it.

Eoin-ONeill-Yokai commented 1 year ago

@jonaski @brandflake11 I could add add this again. IIRC, some of the important part of the code should be mostly portable since it's kind of isolated away from some of clementine's back end, but it's been a few years.

I might download the source code and take a look at porting it, since I have had interest in the project. I've been a bit busy lately. :) It's worth noting that SPC (and VGM and others) don't have 100% accurate playback on GStreamer so even with support of the file type, it's not perfect.

Really, the holy grail for me is getting SPC / VGM / PSF and other chiptune formats all working on linux like how it works on Foobar while also having the niceties of media library like Clementine and others provide. It's a task I am still passionate about, but unsure of how to get past all the red tape of actually improving the gstreamer plugins and supporting more formats. Lazily, I'm still running foobar through wine for my more esoteric collection.

Anyway, hope that's not too much of a rant.

jonaski commented 1 year ago

Great. I think porting it to Qt 6 should be easy, the only thing I see is QTextStream::setCodec needs to be QTextStream::setEncoding with Qt 6, so a ifdef there so it works with both Qt 5 and 6. https://doc.qt.io/qt-6/qtextstream.html#setEncoding

brandflake11 commented 1 year ago

Thanks so much all for looking into this. I love clementine and strawberry and being able to add my spc files to my music library is awesome! Let me know if there's anything I could do to help, like testing, etc.

Eoin-ONeill-Yokai commented 1 year ago

@jonaski I took a look a bit at Strawberry's source today, noticed something a bit different here and I have a few questions.

1) Seems like there's a new class (TagReaderBase) that is the base class to all TagReading operations. This is obviously good since it means less hodge-podge and more support for more filetypes -- but what's the intended workflow here for adding non-TagLib tag readers? Would I simply need to make an additional GMETagReader class extending TagReaderBase and then make an instance of that within the tagreaderworker.h file?

It wouldn't be a terrible idea to try to make a static collection of all tag readers which we copy instances from when starting a thread, where the order of the tag readers determines the order of operations (so to speak) but I digress...

2) Out of my own curiosity, is there any possibility that a similar design to the above could be done for additional playback engines (such as non-gstreamer?) I only ask since there are some file types that don't (or can't) support gstreamer that might be nice long term goals.

3) Lastly, do you mind assigning this issue to me and reopening? Having it assigned and open would make it a bit easier to find this issue again in the future.

jonaski commented 1 year ago
  1. It's because I implemented support for TagParser as an alternative to TagLib. Yes, you could sub-class it and make an instance in it in TagReaderWorker.
  2. Having two different engines enabled at the same time would probably conflict, probably a better idea to implement the missing formats in gstreamer.
jonaski commented 1 year ago

Fixed with #1014