Allow archive format for all single-track file types. #74
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Unrar support removal: Oh yes :)) Play directory: IMHO, such features should be in a player, not library: Library should concern itself with format support / emulation and keep itself simple. (But it's just me and I'm not a maintainer here.) |
sezero
reviewed
Nov 7, 2023
myQwil
force-pushed
the
listable
branch
2 times, most recently
from
9d34702
to
90e52f4
Compare
sezero
reviewed
Nov 7, 2023
myQwil
force-pushed
the
listable
branch
2 times, most recently
from
5287206
to
6ad61db
Compare
myQwil
changed the title
myQwil
force-pushed
the
listable
branch
2 times, most recently
from
6f6eebd
to
a12fedf
Compare
These functions are defined later on in the header, so they are not pure virtual functions.
myQwil
force-pushed
the
listable
branch
2 times, most recently
from
1425fe6
to
2763c4f
Compare
myQwil
changed the title
myQwil
force-pushed
the
listable
branch
2 times, most recently
from
60f68cb
to
3eaf0ed
Compare
myQwil
force-pushed
the
listable
branch
2 times, most recently
from
fa19693
to
065a012
Compare
sezero
reviewed
Nov 25, 2023
Allows for creating playlists of single-track file types.
|
My old compiler (gcc 4.9) emits the following warnings. Maybe you should use explicit initialization like when it was in library code. |
|
Configuration fails if SDL2 is not located: You should move unrar thingies within the diff --git a/player/CMakeLists.txt b/player/CMakeLists.txt
index de6eae1..4f894ab 100644
--- a/player/CMakeLists.txt
+++ b/player/CMakeLists.txt
@@ -27,9 +27,6 @@ if(SDL2_FOUND)
set_property(TARGET gme_player PROPERTY CXX_STANDARD 11)
target_link_libraries(gme_player PRIVATE ${SDL2_LIBRARIES} gme::gme)
# Is not to be installed though
-else()
- message(STATUS "** SDL library not found, disabling player demo build")
-endif()
if(GME_UNRAR)
if(UNRAR_FOUND)
@@ -54,3 +51,6 @@ else()
message(STATUS "RAR file format excluded")
endif()
+else()
+ message(STATUS "** SDL library not found, disabling player demo build")
+endif() |
The following makes it warning-free for me diff --git a/player/Archive_Reader.cpp b/player/Archive_Reader.cpp
index 75e5a31..db888d9 100644
--- a/player/Archive_Reader.cpp
+++ b/player/Archive_Reader.cpp
@@ -28,7 +28,7 @@ blargg_err_t Rar_Reader::restart( RAROpenArchiveData* data )
blargg_err_t Rar_Reader::open( const char* path, bool skip )
{
blargg_err_t err;
- RAROpenArchiveData data = {};
+ RAROpenArchiveData data = { NULL, RAR_OM_LIST, 0, NULL, 0, 0, 0 };
data.ArcName = (char *)path;
// determine space needed for the unpacked size and file count.
diff --git a/player/Archive_Reader.h b/player/Archive_Reader.h
index e385f36..bbc0f01 100644
--- a/player/Archive_Reader.h
+++ b/player/Archive_Reader.h
@@ -46,7 +46,7 @@ public:
#endif
class Rar_Reader : public Archive_Reader {
- RARHeaderData head = {};
+ RARHeaderData head;
void* rar = nullptr;
void* bp = nullptr;
blargg_err_t restart( RAROpenArchiveData* ); |
|
I went with |
OK |
|
I'm OK with this (although I didn't valgrind it) |
sezero
reviewed
Nov 26, 2023
player/Music_Player.cpp
Outdated
| @@ -5,6 +5,8 @@ | |||
| #include <string.h> | |||
| #include <ctype.h> | |||
| #include "SDL_rwops.h" | |||
| #include "Archive_Reader.h" | |||
| #include "Gme_File.h" | |||
There was a problem hiding this comment.
Is it a good idea to use library's private headers here?
As I understand it, you use it for gme_type_t_ details:
Are there no alternatives?
There was a problem hiding this comment.
maybe a function could be added to gme.cpp to perform essentially the same check. i'll look into it
There was a problem hiding this comment.
Maybe something like this? patch.txt
diff --git a/gme/gme.cpp b/gme/gme.cpp
index bb8b52a..6ffccaf 100644
--- a/gme/gme.cpp
+++ b/gme/gme.cpp
@@ -429,3 +429,16 @@ const char* gme_type_system( gme_type_t type )
assert( type );
return type->system;
}
+
+int gme_type_trackcnt( gme_type_t type )
+{
+ assert( type );
+ return type->track_count;
+}
+
+int gme_type_same_ctor( gme_type_t a, gme_type_t b )
+{
+ assert( a );
+ assert( b );
+ return a->new_emu == b->new_emu;
+}
diff --git a/gme/gme.exports b/gme/gme.exports
index ea16df7..c9e9ed4 100644
--- a/gme/gme.exports
+++ b/gme/gme.exports
@@ -61,3 +61,5 @@ gme_vgz_type
gme_disable_echo
gme_set_fade_msecs
gme_load_tracks
+gme_type_trackcnt
+gme_type_same_ctor
diff --git a/gme/gme.h b/gme/gme.h
index 89fa4d6..4e3c273 100644
--- a/gme/gme.h
+++ b/gme/gme.h
@@ -232,6 +232,14 @@ BLARGG_EXPORT gme_type_t const* gme_type_list();
/* Name of game system for this music file type */
BLARGG_EXPORT const char* gme_type_system( gme_type_t );
+/* Number of tracks for format
+ * @since 0.6.4 */
+BLARGG_EXPORT int gme_type_trackcnt( gme_type_t );
+
+/* Whether the formats use the same emulator creator
+ * @since 0.6.4 */
+BLARGG_EXPORT int gme_type_same_ctor( gme_type_t, gme_type_t );
+
/* True if this music file type supports multiple tracks */
BLARGG_EXPORT int gme_type_multitrack( gme_type_t );
diff --git a/player/Music_Player.cpp b/player/Music_Player.cpp
index 57521ff..4069430 100644
--- a/player/Music_Player.cpp
+++ b/player/Music_Player.cpp
@@ -6,7 +6,6 @@
#include <ctype.h>
#include "SDL_rwops.h"
#include "Archive_Reader.h"
-#include "Gme_File.h"
/* Copyright (C) 2005-2010 by Shay Green. Permission is hereby granted, free of
charge, to any person obtaining a copy of this software module and associated
@@ -168,12 +167,12 @@ gme_err_t Music_Player::load_file(const char* path , bool by_mem)
{ // copy data and file sizes
gme_type_t t;
RETURN_ERR( in.read( bp ) );
- if ( (t = gme_identify_extension( in.entry_name() ))
- && t->track_count == 1 )
+ if ((t = gme_identify_extension(in.entry_name())) != nullptr &&
+ gme_type_trackcnt(t) == 1 )
{
if ( !emu_type )
emu_type = t;
- if ( t->new_emu == emu_type->new_emu )
+ if ( gme_type_same_ctor(t, emu_type) )
bp += (sizes[n++] = in.entry_size());
}
}There was a problem hiding this comment.
For now, we can leave out the constructor comparison. That won't be useful until we can properly read .vgz files inside of an archive, if that ever ends up happening.
sezero
approved these changes
Nov 26, 2023
|
@Wohlstand : Will this get in ? |
|
Going to merge in this evening. I want to take a deeper look first. |
|
I tested out the thing, it works good, but, it can't open VGZ (GZ-compressed) files loaded via the
But, this is more problem of the EDIT: Okay, I see the |
|
Oh wait, I actually had an old copy of the PR state, now I refreshed it, and I see the libarchive support has gone 👀 Anyway, repacking these files into RAR, the problem is the same: VGM files gets played, but VGZ aren't. |
|
Updated tests: |
|
Sorry about that. I probably should have updated my first comment saying that some stuff had been taken out. If you got it to work with VGZ files, feel free to put libarchive support back in. |
|
As far as I can see the purpose of this patch is supporting playback of rsn files (actually moving the existing rsn support from library side to player side), and not playback of vgz files embedded in rar archives. The latter should come later in a new patchset, in my opinion. |
Wohlstand
approved these changes
Mar 27, 2024
|
Do I understand correctly that building RSN/RAR support is optional? Thank you for doing this. |
Absolutely! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Rather than having RAR archives reserved specifically for SPC/RSN files, archives are now open to any single-track, or listable, file types. The archive's contents are scanned and an emulator is made for the first listable file type found.
Support has been implemented for RAR and ZIP. Both are optional, with RAR support being enabled with the presence of the UnRAR library, and ZIP support being enabled with the presence of the LibArchive library.