CHD support with multiple definitions error - 3DS build

[mahoneyt944]

Seems with a recent merge of https://github.com/libretro/RetroArch/pull/13486 we now have an issue with our 3DS build. This causes a multiple definitions error with our mame chd support and the version from libretro-common.

[MrHuu]

Sorry about that, i didn't notice the core failed compiling. Are MAME2003 and MAME2003plus the only cores having issues?

libchdr included with retroarch libretro-common would be almost an exact copy from the MAME implementation. No wonder we have multiple definitions. Both libchdr libraries are functional on the 3ds but the functions and function names are duplicated when linking a static build.

symbols with multiple definitions:

chd_open
chd_close
chd_read
chd_get_metadata
chd_get_header

A very quick and dirty workaround could be renaming the symbols.

Renaming the symbols with, for example, objcopy --redefine-sym chd_open=mame_chd_open after the core library has been build allows retroarch to compile without issues. Runs without any obvious issues on my end.

If MAME2003 and MAME2003plus are the only cores affected, we could easily rename these symbols using the build script.

While this sounds like a great idea i'm my head, i do like to emphasize my software engineering knowledge is practically non existent. If there would be any reason on not to take such path, feel free to express your opinions and concerns.

Now that i've seen it working, 'not possible' is not an option. :wink:

@jdgleaver Are you willing to express your opinions about this?

Related PR: https://github.com/libretro/RetroArch/pull/13512

[mahoneyt944]

Chd support in this core is only v3, libretro-common is v5 which uses the same naming but are different.

We include legacy chd support unconditionally in our makefile so static builds including chd support too will see duplicate definitions. This would be true for most mame cores I would think.

Changing the naming could be possible but libretro-common code is suppose to be reusable and therefore have unique/ generic naming to not interfere with core code. Perhaps libretro-common should add a "libretro" or "lr" prefix to the chd functions in libretro-common to distinguish them from any core using legacy chd support?

[mahoneyt944]

@grant2258 @markwkidd what do you think about switching our chd support to v5 which is included in libretro-common? It seems that this version has provisions for reading older versions of chd files. I believe if we update our parameters and such, we should be able to still support the same chd files we have now but with the libretro-common pkg instead. There's not a lot of gain from this besides using more libretro libraries but that in itself might be worth it?

[markwkidd]

I think this would be a useful and welcome addition. I made an attempt a couple of years ago, but it was beyond me then. I don't recall that there was an insurmountable issue, more like a feeling that there are just a lot of fiddly bits to connect and it probably needed a fresh start.

Those are my thoughts at any rate :) I've gone back to working my day job on a part time basis, but I'm still a while away from doing much coding still. Life has been nuts here. But I do more or less try to keep up with the discussions in this repo

On Tue, Jan 18, 2022 at 11:44 PM mahoneyt944 @.***> wrote:

@grant2258 https://github.com/grant2258 @markwkidd https://github.com/markwkidd what do you think about switching our chd support to v5 which is included in libretro-common? It seems that this version has provisions for reading older versions of chd files. I believe if we update our parameters and such, we should be able to still support the same chd files we have now but with the libretro-common pkg instead. There's not a lot of gain from this besides using more libretro libraries but that in itself might be worth it?

— Reply to this email directly, view it on GitHub https://github.com/libretro/mame2003-plus-libretro/issues/1274#issuecomment-1016077068, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEVGC5XDJ3FTVIOZS6IJ6MTUWY6RBANCNFSM5MG5DFPQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you were mentioned.Message ID: @.***>

-- Mark W. Kidd (he/him/his) http://facebook.com/markwkidd (606)536-0115

[jdgleaver]

@MrHuu The only way enable frontend CHD support for static builds is to customise the libretro-common version of libchdr so all symbols are uniquely named (e.g. with a lr_ prefix). This has to be done at the frontend level - we cannot require cores to use a specific libchdr implementation, or modify their existing codebases to accommodate a foible of RetroArch (remember that the only contract between core and frontend is the libretro API - cores should not be concerned with any other frontend-specific issues).

The slight annoyance here is that any core which already uses the libretro-common version of libchdr would also have to be updated (and then the relevant .c files would be excluded for static builds). Note that this doesn't contradict the above paragraph - if a core uses random libretro-common code, then there is an implicit agreement that the core dev will have to maintain the codebase inline with libretro-common changes! I don't know how many cores this would affect (there's something of a mixture of libretro-common, upstream libchdr and older/custom versions used among cores).

[MrHuu]

Thanks for elaborating on the various aspects regarding this issue. The clear explanation of the issue is much appreciated. While such libretro-common implementation would be the proper solution in this case, i'm pretty sure this isn't planned on libretro's roadmap.

Being unfamiliar with the amount of cores relying on the libretro-common implementation and the amount of work it requires from the core developers, i don't think this is desired at this point of time. Instead it could have been beneficial when the library was initially added to libretro-common.

I'm not really sure which platforms aside from the 3DS would actually benefit from having a unique libretro-common version of libchdr. When looking at the 3DS platform, i'm only aware of the MAME2003 and MAME2003plus cores actually having conflicts with the current implementation of libchdr included with libretro-common.

From this point of view, changing MAME2003 to make use of the libretro-common implementation seems to be the least intrusive solution to this specific issue regarding the 3DS.

Frontend CHD support is only used to support RetroAchievements as far is i understand, which are not supported by MAME2003. So unless the core itself would benefit from making the transition @mahoneyt944 suggested i'm not sure it's worth the effort. Although making use of libretro-common when possible, could potentially benefit the compatibility across some platforms.

[ghost]

@mahoneyt944 your would need to update the common.c(mame) chd reads and probably keep the sha1 code it is changeable either that or rename the mame chd function so they dont conflict with the libretro headers. It really your choice how to handle it though

[mahoneyt944]

Closing this for now. We can start a new issue for upgrading chd support when the time comes.