Jump to content
IGNORED

New MAME release


mizapf

Recommended Posts

LOG_WARN should be output by default when you use "-log" or "-oslog". You can add more debug flags in the source file. The websocket client, however, cannot output via the MAME logging facility, as it is an external library; you have to add printf lines.

 

Edit: The location where I suspect the issue is in the file https://github.com/mamedev/mame/blob/master/src/lib/util/client_ws.hpp, line 304, in the "handshake()" method. The client_ws is actually not part of a third-party library but of MAME, and could probably be fixed more easily if required.

 

More detail: The problem might arise after the asio::async_read_until(..."\r\n\r\n"), which reads up to the blank line after the header of the HTTP response.

Link to comment
Share on other sites

Wow.  You weren't kidding about "lots of asynchronous handling with callbacks and lambda expressions."  I've never before experienced a stack trace in GDB taking minutes to print while deciphering the function names due to the sheer complexity of the templates in the Boost asio library.

 

I wonder if we're seeing different symptoms.  The handshake looks fine, and the received ASYNC message is parsed correctly.  The breakdown as far as I can tell happens immediately after the ASYNC here in client_ws.hpp.

					//Close connection if masked message from server (protocol error)
					if(first_bytes[1]>=128) {
						const std::string reason("message from server masked");
						auto kept_connection=connection;
						send_close(1002, reason, [](const std::error_code& /*ec*/) {});
						if(on_close)
							on_close(1002, reason);
						return;
					}

The first_bytes usually contains {0xff, 0x82 or 0xff} so it triggers the above close on the websocket.  From the traffic on WireShark, there is no masked message.  I'm having trouble figuring out where this data is actually read from...

 

I'm also very suspicious about this function in class Message in client_ws.hpp:

			std::string string() const {
				std::stringstream ss;
				ss << rdbuf();
				return ss.str();
			}

If I print the message string in tipi_card_device::websocket_incoming()

case 1:

        LOGMASKED(LOG_WARN, "Got text message: size=%zu %s\n", message->size(), message->string());

 

Sometimes it will show extra garbage control chars at the end of the ASYNC string... something's not right.

 

Edited by PeteE
  • Like 2
Link to comment
Share on other sites

There is one issue with the logging: The websocket runs on a separate thread, and the logging is not thread-safe. That is, it may mess up the log by seemingly unprintable characters (I guess it is only NUL characters, caused by different locations of the file pointer in the threads). Maybe you should replace the logging output by printf.

 

As I said, it took me many hours to track down the issue as far as I described in the posting above.

Link to comment
Share on other sites

In client_ws.hpp, there seems to be a possibility that there could be leftover data in the message->streambuf before the asio::async_read is called, and the asio::transfer_exactly doesn't account for that.  I've tried changing the lines

asio::async_read(*connection->socket, message->streambuf, asio::transfer_exactly(2),

to:

asio::async_read(*connection->socket, message->streambuf, asio::transfer_exactly(std::max(2 - message->streambuf.size(), std::size_t{0})),

And this helps in some cases, but not all.  For example, after a console reset, it gets stuck on the ASYNC string websocket packet, even though there is enough data (7 bytes) already in the message->streambuf.size().  Maybe it doesn't like transfer_exactly(0)?  It works sometimes.

 

Also, need to copy leftover data from one message to the next:

					//Next message
					std::shared_ptr<Message> next_message(new Message());
					//Copy any leftover data in the streambuf into the next message
					next_message->streambuf.commit(buffer_copy(
						next_message->streambuf.prepare(message->streambuf.size()),
						message->streambuf.data()));
					read_message(next_message);

If you need an actual patch, let me know.

 

Also! Disable or remove this, otherwise it will recurse endlessly when there are already enough bytes in message->streambuf:

					if(bytes_transferred==0) { //TODO: This might happen on server at least, might also happen here
						read_message(message);
						return;
					}

 

 

Edited by PeteE
  • Like 1
Link to comment
Share on other sites

Well, the above code doesn't work, but my hypothesis was correct.  Using std:max to calculate the number of bytes didn't work because std::size_t is unsigned, so it would end up requesting a negative-converted-to-unsigned number of bytes, and wait forever.  I also noticed that reading message->string() multiple times would consume the stringbuf every time, so the next call could return an empty string or part of the next message on the socket.  My fixes are here:  https://github.com/mamedev/mame/compare/master...peberlein:mame:master

 

Edit: You could also remove the Transfer Mode configuration option now.  The server sends "ASYNC" if it supports that mode, otherwise the client should keep sending syncs when appropriate.

Edited by PeteE
  • Like 3
  • Thanks 1
Link to comment
Share on other sites

Thank you for your work! As it seems, it works so far for me, but I did not fully understand why.

 

How does it solve the problem that the ASYNC is already in the streambuf, but the async_read in the read_message never got invoked because there was no reaction from the epoll_wait?

Link to comment
Share on other sites

On 6/16/2022 at 6:45 AM, globeron said:

 

it needs the .rpk format  (currently I almost converted most .bin to .rpk format, very straight forward and edit the .xml file) to be used with Batocera (uses Mame for the TI and works better than the TI/SIM on the retropie) (but I need some time to add the images and sample videos for Batocera then will share it).

 

 

@mizapf  are you able to run these .rpks in MAME )?    (they do work in js99er.net when I load the rpk's)

 

superfly.rpk
 

honey_hunt.rpk

 

TerryTurtle.rpk   (it does start in JS99, but indicates it requires a MBX console)

(similarly Champion Baseball does the same, but that one starts in MAME / Batocera until the screen that an MBX console is needed)

 

ImHiding.rpk

(this gives a bluescreen, also in JS99er.net,  and same when selecting it from the js99er.net software list)

 

 

 

 

 

 

Edited by globeron
Link to comment
Share on other sites

5 hours ago, mizapf said:

Thank you for your work! As it seems, it works so far for me, but I did not fully understand why.

 

How does it solve the problem that the ASYNC is already in the streambuf, but the async_read in the read_message never got invoked because there was no reaction from the epoll_wait?

My guess is that the async_read doesn't look at how much data is already in the streambuf.  So even though it had 7 ASYNC bytes in it already after reading the websocket headers, it would epoll_wait until 2 more bytes arrived.  That's why I wrote the message.transfer function which will return how many bytes are needed to fill streambuf to the amount needed, and if the streambuf already has that many, it will return 0, allowing the async_read to immediately call the lambda to process the existing data.  The biggest clue was printing out the value of message->streambuf.size() before each async_read.

 

Glad it fixed it for you too.  Will you be able to merge in my changes?  If there's anything else I need to do, let me know.

 

 

Edited by PeteE
  • Like 1
Link to comment
Share on other sites

The Superfly RPK is not working (see -oslog, lots of invalid memory accesses), but the superfly.zip (see WHTech). Interestingly, the zip version has a ROM dump and a GROM dump.

 

Also, the ZIP versions of Honey Hunt, Terry Turtle, and I'm Hiding are working. Remember that the ZIPs contain the original ROM dumps as far as I could see.

 

 

  • Like 1
Link to comment
Share on other sites

@PeteEI did some research during the last days as well. It seemed to me that the handshake() would have worked if the async_read (called by read_message) did not wait for new data but just processed what is left in the queue.

 

OK, and if you can make async_read leave the epoll_wait trap by declaring 0 bytes to be read, this explains why it works.

 

We should find a better name for "transfer"; the name is misleading. I'd expect some function that moves data by that name and not one that returns a value. When I read asio::transfer_exactly(message->transfer(2)), how should one call that part in parentheses? Probably needed_for(int n)?

 

Or maybe the transfer method returns the complement, i.e. transfer1(n) = n-transfer(n). So transfer1 could be called available.

 

One good news: I think we are the only people who include the client_ws in MAME, so there is no problem with breaking other drivers.

 

 

Link to comment
Share on other sites

5 minutes ago, mizapf said:

We should find a better name for "transfer"; the name is misleading. I'd expect some function that moves data by that name and not one that returns a value. When I read asio::transfer_exactly(message->transfer(2)), how should one call that part in parentheses? Probably needed_for(int n)?

 

Or maybe the transfer method returns the complement, i.e. transfer1(n) = n-transfer(n). So transfer1 could be called available.

 

One good news: I think we are the only people who include the client_ws in MAME, so there is no problem with breaking other drivers.

I had difficulty deciding on the name.  I like "needed_for".

Link to comment
Share on other sites

OK, I just pushed the client_ws.hpp to the master branch. I'll check the tipi implementation once more, already removed the transfer mode setting.

 

Edit: Also pushed the new tipi.{h,cpp}. I'll build new binaries and put them on WHTech.

  • Like 1
Link to comment
Share on other sites

I uploaded the new builds to WHTech for Windows, Linux, and Raspbian as "mame20220710b_*". If you like you can download them and unpack them as usual over your current installation.

 

This release contains the fix for the TIPI with real Raspberry Pi.

  • Like 3
  • Thanks 1
Link to comment
Share on other sites

18 hours ago, mizapf said:

The Superfly RPK is not working (see -oslog, lots of invalid memory accesses), but the superfly.zip (see WHTech). Interestingly, the zip version has a ROM dump and a GROM dump.

 

Also, the ZIP versions of Honey Hunt, Terry Turtle, and I'm Hiding are working. Remember that the ZIPs contain the original ROM dumps as far as I could see.

 

 

@mizapf  can you help how to start the .zip files in MAME ?

 

 

cd \mame
mame64.exe ti99_4a  -cart c:\mame\carts\superfly.zip -ioport peb -ioport:peb:slot2 32kmem -ioport:peb:slot3 speech -ioport:peb:slot8 h

 

cd \mame
mame64.exe ti99_4a -gromport multi -cart c:\mame\carts\superfly.zip -ioport peb -ioport:peb:slot2 32kmem -ioport:peb:slot3 speech -ioport:peb:slot8 h

 

 

probably not as there are more grom files

cd \mame
mame64.exe ti99_4a -gromport single -cart c:\mame\carts\superfly.zip -ioport peb -ioport:peb:slot2 32kmem -ioport:peb:slot3 speech -ioport:peb:slot8 h

 

 

 

 

Link to comment
Share on other sites

The ZIP files are not referenced as files (like the RPKs). They are pre-registered in the software list (hash/ti99_cart.xml) and are referenced by name. The ZIP file is normally named as <cartridgename>.zip.

 

That is, you have to use the plain file name without extension.

 

The ti99_cart.xml is essentially a collection of the former layout.xml, but by another schema.

 

mame ti99_4a -cart exbasic
mame ti99_4a -cart editass
mame ti99_4a -cart superfly
mame ti99_4a -cart honeyhnt

 

If you misspell it, you get a list of possible corrections.

 

The ZIP files must be located in a path specified in the ROMpath.

Link to comment
Share on other sites

1 hour ago, mizapf said:

The ZIP files are not referenced as files (like the RPKs). They are pre-registered in the software list (hash/ti99_cart.xml) and are referenced by name. The ZIP file is normally named as <cartridgename>.zip.

 

That is, you have to use the plain file name without extension.

 

The ti99_cart.xml is essentially a collection of the former layout.xml, but by another schema.

 


mame ti99_4a -cart exbasic

mame ti99_4a -cart editass

mame ti99_4a -cart superfly

mame ti99_4a -cart honeyhnt

 

If you misspell it, you get a list of possible corrections.

 

The ZIP files must be located in a path specified in the ROMpath.

 

Thank you. I will try it  (it seems .zip files do not work in Batocera, because of the file structure setup), 

but I prefer the .rpk format for Batocera setup to keep it standardized.

 

these .rpk also do not work but they work in js99er.net 

 tetris.rpk

 

spad_xiii.rpk

 

 

compu-car.rpk

 

Link to comment
Share on other sites

22 minutes ago, globeron said:

 

Thank you. I will try it  (it seems .zip files do not work in Batocera, because of the file structure setup), 

but I prefer the .rpk format for Batocera setup to keep it standardized.

 

The ZIPs are the standard for MAME (and the RPKs are the exception), but I guess you mean standard with respect to our emulator environments.

 

If Batocera relies on MAME, it should be able to use the ZIPs, as the ZIP loader is a core function of MAME (used for the system ROM ZIPs as well, like ti99_4x.zip). Sounds like a configuration issue to me.

 

Edit: If RPKs don't work, try to use "gromemu" instead of "standard" as the PCB type (layout.xml). The "standard" type only accepts 6K GROMs (real GROMs), not 8K (which is why I call it "GROM Emulation").

 

  • Like 1
Link to comment
Share on other sites

9 hours ago, mizapf said:

 

The ZIPs are the standard for MAME (and the RPKs are the exception), but I guess you mean standard with respect to our emulator environments.

 

If Batocera relies on MAME, it should be able to use the ZIPs, as the ZIP loader is a core function of MAME (used for the system ROM ZIPs as well, like ti99_4x.zip). Sounds like a configuration issue to me.

 

Edit: If RPKs don't work, try to use "gromemu" instead of "standard" as the PCB type (layout.xml). The "standard" type only accepts 6K GROMs (real GROMs), not 8K (which is why I call it "GROM Emulation").

 

 

Thank you, I will try to play around with the different configurations, some others I got to work last time, but these RPKs seem to give a bit trouble ? 

(and I am trying to understand why JS99er.net does not have the issue loading these .rpk ?)

 

 

Link to comment
Share on other sites

6 hours ago, globeron said:

and I am trying to understand why JS99er.net does not have the issue loading these .rpk ?

JS99er is probably more relaxed about the different PCB types, e.g. it doesn't distinguish between original and emulated GROMs. You can't use JS99er to validate that RPKs are correctly configured.

  • Like 3
Link to comment
Share on other sites

Yes, the RPKs were designed at a time when the GROMs were not yet emulated at circuit level in MAME, and when we had full GROM space dumps. The ZIP cartridges, however, have per-circuit dumps (6K), and to avoid confusion ("standard" would have meant different things for RPK and for ZIP), I restricted the standard type to 6K GROMs, and added the "gromemu" as the type which allows 8K per GROM. The technical background is that there are cartridges pretending to have GROMs, but actually use an EPROM and an address counter, thus emulating GROMs.

 

  • Like 1
Link to comment
Share on other sites

On 7/9/2022 at 5:33 PM, mizapf said:

I uploaded the new builds to WHTech for Windows, Linux, and Raspbian as "mame20220710b_*". If you like you can download them and unpack them as usual over your current installation.

 

This release contains the fix for the TIPI with real Raspberry Pi.

 

Thank you @mizapf and @PeteE! Finally I can debug and experiment with the TIPI ROM without pulling chips while in the comfort of the same PI I use on real hardware. 

  • Like 6
Link to comment
Share on other sites

Except for the >1800 crubase makes me sad.  Maybe more mame users are Geneve users than 4A users.

 

<port tag=":ioport:peb:slot7:tipi:SW1" type="DIPSWITCH" mask="31" defvalue="24" value="16" />

How does 24 map to >1800 and 16 map to >1000  ?    oh, it is the decimal MSByte of the crubase... 

 

That's surprising, as the hardware only has a 4 bit jumper block. 

 

And super surprising that I can't find a general mame command line argument for setting something so formally identifiable as the named dipswitch... but I guess I can script edits to the file in cfg/. I commonly want to test >1000 and >1100 and never care about >1800... 

 

I'm sure this is nothing that 'sed' can't handle. 

Link to comment
Share on other sites

In those cases, I suggest to use a special config directory. Use "-cfg_directory xxx" and apply the settings in the MAME menu; they will be stored in xxx/ti99_4a.cfg.

 

The problem is that the current GeneveOS requires the TIPI to be set to >1800, and if you forget it, it simply does not work at all and gives you no hint about the problem. Even I forgot to set the CRU base back to >1800 after doing some other work with the Geneve and wondered what I could have broken in the meantime, until I remembered to set the address back to >1800.

  • Like 1
Link to comment
Share on other sites

1 hour ago, mizapf said:

In those cases, I suggest to use a special config directory. Use "-cfg_directory xxx" and apply the settings in the MAME menu; they will be stored in xxx/ti99_4a.cfg.

 

The problem is that the current GeneveOS requires the TIPI to be set to >1800, and if you forget it, it simply does not work at all and gives you no hint about the problem. Even I forgot to set the CRU base back to >1800 after doing some other work with the Geneve and wondered what I could have broken in the meantime, until I remembered to set the address back to >1800.

 

The DSK mapping of TIPI used in a 4A won't work for people either if they include an actual floppy controller in the system as is typical. And it too just won't work and gives no hint about the problem. This is about authors preference, and that is OK. js99er.net doesn't even let you set the TIPI crubase. So even though the crubase emulation for TIPI is not true to the hardware in MAME, it is welcomely flexible. 

 

 

#!/bin/bash

TIPI_CRU=${TIPI_CRU:-1000}
SW1=`echo "ibase=16; ${TIPI_CRU%00}" | bc`

sed -i "/tipi:SW1/s/ value=\"..\"/ value=\"${SW1}\"/g" ./cfg/ti99_4a.cfg

./mame ... $@

 

With the above scripting I can easily invoke my 4a mame script as:

 

TIPI_CRU=1100 ./tipi.sh -cart ~/dev/github/jedimatt42/fcmd/FCMD.RPK

 

to change the default. Or I can skip including the environment override and the script will set it to my preferred default, without having to get back into the UI. 

  • Like 1
Link to comment
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.
Note: Your post will require moderator approval before it will be visible.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

Loading...
  • Recently Browsing   0 members

    • No registered users viewing this page.
×
×
  • Create New...