Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature Request] Can we get zip support? #44

Open
longelf opened this issue Mar 29, 2022 · 6 comments
Open

[Feature Request] Can we get zip support? #44

longelf opened this issue Mar 29, 2022 · 6 comments

Comments

@longelf
Copy link

longelf commented Mar 29, 2022

My CD+G collection is zipped in individual zip files for each track, containing the cdg and mp3 file. KF does not currently see them unless I unzip them, and I don't see zip support in the Road to v1.0, are there plans to implement it?

@longelf longelf changed the title Can we get zip support? [Feature Request] Can we get zip support? Mar 29, 2022
@bhj
Copy link
Owner

bhj commented Mar 30, 2022

Hey there, thanks for the request!

I agree it would be nice if archives "just worked", though IMO in the long run you're better off spending the CPU cycles just once to unzip. The MP3s also then become much easier to work with for tagging, adding ReplayGain metadata, streaming, etc. Combined with the complexity and speed tradeoffs, it's not currently planned, but a PR would definitely be considered.

@longelf
Copy link
Author

longelf commented Mar 30, 2022

I agree, it's all about tradeoffs, in this case CPU cycles and program complexity vs storage space. Other Karaoke software does support zipped media without these being an issue, and when you have a large collection the storage space requirements can become a problem. In terms of overhead I only see an issue when playing the media as it would have to unzip to a temp folder then delete it afterwards, but in my world of PHP the built in zip functions make this pretty simple and wouldn't overly tax the server. No need to unzip when adding a track to the database, just use the zip filename as "Artist - Track Name" just as it currently does with the CDG filename.

@bhj
Copy link
Owner

bhj commented Mar 30, 2022

I agree, it's all about tradeoffs, in this case CPU cycles and program complexity vs storage space.

With the abundance of storage these days, it's a good tradeoff IMO. Not saying it won't happen, but not currently planned.

it would have to unzip to a temp folder then delete it afterwards

It might be better to do this in-memory if possible. The files are streamed to the browser and songs can be paused/skipped, so it's not necessarily easy to know when we're "done" with them. Might come down to whether it's better to have a config for memory limit or temp storage limit - with memory there are no path or permission issues, at least.

No need to unzip when adding a track to the database, just use the zip filename as "Artist - Track Name" just as it currently does with the CDG filename.

FWIW the scanner reads the audio file for duration and ReplayGain, and we pass all tags to the parser (the default parser may currently only use the filename, but a custom parser can use the tags).

@mjmeans
Copy link

mjmeans commented Jan 3, 2023

Just my 2 cents... All multimedia files, like AVI, MP4, etc, are all containers with included packets of metadata, audio streams, video streams, closed caption streams, etc. All these streams are essentially multiple individual files. The point of having a container file instead of separate files for each stream that all have to be named the same is to make libraries manageable. I see MP3+G files zipped up into a single zip functionally equivalent to an MP4; having a single audio and video stream.

When I wrote a Microsoft Store Windows 8 Karaoke Player app long ago I supported ZIP files and had to deal with the issues inherent in ZIP files such as internal file names being different from each other or different from the ZIP file's name, and errant other files inside the ZIP file. The common naming standard for people running Karaoke shows was "{disc-track} - {artist} - {song}". This was to make it clear which vendor the track came from in case you were ripping actual CDGs to ZIP files using software from TriceraSoft. Remember them? They invented the MP3+CDG(ZIP) format, Anyway, if you were using these ripped files in a show and got audited, you had to prove that you purchased the original CDG disc, hence the disc and track name as part of the file name. So, in my app I needed to support that standard. However, I found that some people had ZIP files with mismatched internal files names or names that didn't match the main ZIP file name, or extra internal files like a text file stating where the track was purchased from. The ultimate solution was to extract the first CDG file and the first MPG file and ignore the rest. By the time I was testing pitch shifting and a new version, Microsoft had closed the Windows 8/8.1 store, so that project ended.

Anyway, I think ZIP file libraries are still the most common and makes the most sense since a ZIP file is equivalent to other multimedia files, simply a container of the paired audio and video streams.

@unRARed
Copy link

unRARed commented Jan 20, 2023

Found this wonderful app today as a "point and click" option in my QNAP NAS (through a 3rd party index mind you). I, too, am in favor of supporting zip (any maybe also FLAC if possible)? Could you perhaps use a mechanism for watching the queue stack to know what files to extract / close... i.e. something new added to the queue, immediately extract it and store in var. once said song is performed, var.close() and rm?

In any case, thanks for making this! Looks like this will work just as well as LYRX that I have used in the past for serving the primary need.

@bhj
Copy link
Owner

bhj commented Jan 28, 2023

@unRARed Thanks, cool to know it's working on QNAP stuff! Re: FLAC files, yes, the scanner already looks for a .m4a before .mp3, so good first PR maybe? 😉

Definitely in favor of adding ZIP support now that 1.0's out, and there are some good edge cases documented here. One of the design philosophies so far has been to keep the server as simple (read: dumb) as possible, so it doesn't know anything about the playback state of any given room, making some things a little trickier. I'm hoping we could basically unzip to memory, and let Node handle the caching and GC as much as possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants