Journey sample bug

[mahoneyt944]

Found a bug. Restarting through the RA menu causes the sample to not work.

Tested by starting the game with service menu dip switch activated to load into the service menu, use the cassette test. Note the sample works, enter RA, hit restart. The game should restart back into the service menu, now the sample will not play.

I think this has to do with the sample loading on restart, this sample being large maybe part of it too. May need to free loaded samples and reload?

[ghost]

automalloc free when reset. This can be fixed in a few ways reload samples on reset or use malloc instead of automaloc in teh samples code. Both ways are fine just need to make sure you freeing only when you allocate else youll start getting segfaults.

[ghost]

just to show you her here needs changed.

https://github.com/libretro/mame2003-plus-libretro/blob/1fd30e32ec948da4227b5eb7af327fddd6c7115b/src/common.c#L325

This was for low end devices to load on demand only clearly more work needs done on reset if you want or you can just change to malloc I think only two need changed from memory or a core option can be implemented to choose malloc or auto malloc but this would confuse user imho

edit double check your samples update your using normal samples and ost for sample settings they used to be seperate from each other so you could just disable ost samples and not normal ones. Youll need to trace this one since there are a few changes wont be hard to fix the way you want it though

[ghost]

I downloaded the sample form mr do semes to be working fine here maybe you have a bigger sample that the one used there.

Edit well thats strange no issues from updating from online update it fine but compiling yourself the issue happens.

[INFO] Built: Oct 23 2021 [INFO] Version: 1.9.11 [INFO] Git: dd36576f23

using a diffrent RA build fails however fails im 99% sure this is a core issue will have a look at the weekend if i get time unless you want to deal with it shouldnt be too hard to trace.

[mahoneyt944]

I'm using the current build in 1.9.12 and the sample issue is there. The first time the game loads it works fine if you soft reset in ra the sample won't work

[mahoneyt944]

What I do is this.

Start journey Open dip menu turn on service menu dip Close content to save the cfg Open journey again it should load to the service menu Test the cassette, it should work Leave cassette test to "off" Open ra quick menu Hit restart Go back to game and try cassette again Now it doesn't work.

If you completely close content and reload journey fresh it works fine

[ghost]

yea should be easy enough to fix up here is the trace problem is with bug like this is sometimes they wont trigger if the memory is untouched and left teh way it is. However youll need to make a choice to sort the big sample load on demand or just load the samples one requires more effort but will work the problem is any automalloc is cleared on reset and the samples arent reloaded. To fix this properly youll have to either always use auto malloc and reload samples on reset or use malloc and dont reload samples on reset

==28503==ERROR: AddressSanitizer: heap-use-after-free on address 0x7fc1d4859412 at pc 0x7fc2035baf58 bp 0x7fff8c041280 sp 0x7fff8c041270
READ of size 2 at 0x7fc1d4859412 thread T0
    #0 0x7fc2035baf57 in mixer_channel_resample_16 src/sound/mixer.c:325
    #1 0x7fc2035bde0c in mixer_channel_resample_16_pan src/sound/mixer.c:496
    #2 0x7fc2035bf3f7 in mix_sample_16 src/sound/mixer.c:582
    #3 0x7fc2035c0986 in mixer_update_channel src/sound/mixer.c:724
    #4 0x7fc2035c0a99 in mixer_sh_update src/sound/mixer.c:750
    #5 0x7fc2034bce4f in sound_update src/sndintrf.c:1303
    #6 0x7fc20348b3ff in updatescreen src/mame.c:1210
    #7 0x7fc2031c787e in cpu_vblankcallback src/cpuexec.c:1424
    #8 0x7fc2034fceb5 in timer_adjust_global_time src/timer.c:327
    #9 0x7fc2031c1937 in cpu_timeslice src/cpuexec.c:654
    #10 0x7fc2031bce2c in mame_frame src/cpuexec.c:363
    #11 0x7fc2030d7ffa in retro_run src/mame2003/mame2003.c:409
    #12 0x560b01e62d98 in core_run (/home/grant/Source/retroarch/retroarch+0xebd98)
    #13 0x560b01e68f40 in runloop_iterate (/home/grant/Source/retroarch/retroarch+0xf1f40)
    #14 0x560b01e6a027 in rarch_main (/home/grant/Source/retroarch/retroarch+0xf3027)
    #15 0x7fc22235ab24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)
    #16 0x560b01e4949d in _start (/home/grant/Source/retroarch/retroarch+0xd249d)

0x7fc1d4859412 is located 162834 bytes inside of 16214044-byte region [0x7fc1d4831800,0x7fc1d57a801c)
freed by thread T0 here:
    #0 0x7fc2265fef19 in __interceptor_free /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:127
    #1 0x7fc203199466 in auto_free src/common.c:993

previously allocated by thread T0 here:
    #0 0x7fc2265ff279 in __interceptor_malloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:145
    #1 0x7fc203199069 in auto_malloc src/common.c:943

SUMMARY: AddressSanitizer: heap-use-after-free src/sound/mixer.c:325 in mixer_channel_resample_16
Shadow bytes around the buggy address:
  0x0ff8ba903230: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0ff8ba903240: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0ff8ba903250: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0ff8ba903260: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0ff8ba903270: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
=>0x0ff8ba903280: fd fd[fd]fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0ff8ba903290: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0ff8ba9032a0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0ff8ba9032b0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0ff8ba9032c0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0ff8ba9032d0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==28503==ABORTING
[grant@grant-aspirea31523 blaster-barcade]$ 
=================================================================
==28550==ERROR: LeakSanitizer: detected memory leaks

[mahoneyt944]

I think automalloc and reload on reset. If this benefits lower spec. What do you think?

[ghost]

depends how much work you willing to put into it im easy either way

[mahoneyt944]

Let's go that route then. I guess this is already using automalloc now so we just need to reload the samples.

[ghost]

The samples always used automalloc even in the old mame code as far as i remember we will need to comfirm though its been a while. I will look into how the original code reset and how we reset now this might even just be an over looked bug that was always there.

[mahoneyt944]

This looks to be the difference here. Small samples using auto malloc and large using malloc? https://github.com/libretro/mame2003-plus-libretro/blob/755e64ed8c7f1fdc5d69e65084a3355c51107937/src/common.c#L319-L327

[mahoneyt944]

If you restart on small samples it works, maybe we need to free the large malloc samples on reset.

[ghost]

samples are set to a high size anyway

src/common.h:100:#define GAME_SAMPLE_LARGE 10000000 // 10MB

10000 meg well its actually less meant to be divided by 1024 for real size as that value is in bytes

[ghost]

@mahoneyt944 its the resources freeing happening. Samples are handled different from other sound devices they are freed and not reloaded I know our runloop has changed need to find where we should handle the samples properly.

https://github.com/libretro/mame2003-plus-libretro/blob/1fd30e32ec948da4227b5eb7af327fddd6c7115b/src/cpuexec.c#L342 removing this stops the auto_mallocs freeing and everything is fine. Ill need to check the original mame source and see if a inhereted bug or something that changed this end. I have to drop the subframe of car and see if i need a new steering rack so might not be able to do this if i get the time i will

[mahoneyt944]

I've been rebuilding my tron machine. The thing was starting to become a fire hazard. It has 4 uv lights that bake the wire harnesses 24/7 and the crt is basically a big magnet for dust. Lots of fun ha

[ghost]

yea I know the feeling of fixing things fixed my mums tv wasnt coming on popped some caps in the psu after testing for bas ones and it turned on fine didnt think much else of it gave her it back. Get a call its making a screeching sound if you leave it off a while and turn it on yea you guessed two more bad caps on the main board. First time ive came across this in a main board on a tv though didnt have any surface mount caps so just put some radials in it jobs done. Honestly its must have been so damn time consuming diagnosing bad caps without a scope or esr meter back in the day.

edit ps get some Polyimide Film Self Adhesive tape round your harness I use this for my bga machine for thermocouples dont cost much either

[ghost]

you know what i need some sleep i converted the to kilobytes instead megs 10 megs as it is set now:)

[mahoneyt944]

How many individual samples are that large? I know journey is due to it being a whole song. Probably the ost Samples too.

[ghost]

not sure to be honest but here is a fix for now till one of gets the time to look into it more. The sample support will need updated and the mixer as well to support this in any meaningful way and we defo need to loose the auto_mallocs for dynamic allocating de-allocating.

here is the fix for now the mixer only sets up one time it can be updated on the fly. For example load journey into service mode dont play the sample then reset it. Then play the sample it will work. Now reset again and youll see its gone,

Ive put a fix here temp fix here until someone has the time to implement it.

https://github.com/grant2258/blaster-barcade/commit/e95f63a1aa6fcbd3114249f1157139ea5a0ff072

With this fix youll see whats going on play the sample reset and do it again and it will start where it left off where you where so that paused sample needs dealt with on reset at the very least. I will sort it have a few spare moments but best fixing it for now it never worked properly anyway I fixed ost as teh bigger samples in sf2 always caused me issues i fixed it in my branch though.

[ghost]

This will fix the issue im not willing to track everywhere this pointer is used any free with a reference to it will cause issues its up to you if you want to add a de allocate the mixer

[mahoneyt944]

Where it's paused at should start there after a soft reset since in the real machine, the 8-track player wouldn't have rewound itself either. So that sounds right to me. As far as freeing things, I haven't looked into any of that and admittedly doesn't sound like a fun task.

I think it would be best to heavily test a new and complete implementation here before commiting anything to be sure we aren't introducing new issues.

[ghost]

Not sure exactly where your coming from we aren't emulating a tape cassette. This core also never had pause support or unloading of samples. Either way you'll need to re write the code to keep samples loaded and stop samples on reset that's will work with current code all you need to do is stop all sample to current code added easy enough Or you'll have to do some real work on the mixer and samples and support states on reset. Is up to you if you want to do this

[mahoneyt944]

Journey's sample that we use here, was originally played via a 8-track player inside the arcade machine. when the bonus level ends it just cut power to the 8-track player and would power up when the next bonus level was encountered..

So our sample support is effectively recreating this functionality.

[mahoneyt944]

Here's a video showing the setup at 3:30 https://youtu.be/lNikyy11q5U

[ghost]

I know that but when you close mame you'll need a save state to remember as well. If you willing to put the time in to do that fair enough.. personally I think it's not worth the effort but if your up for it. I'd say go for it.. I'd personally stop samples on reset and keep them loaded or at the very least let the sample plater handle the alloc and free so the they don't get freed on reset. Anyway will leave ithis 9ne n your hands .

[mahoneyt944]

On a fresh boot starting at the beginning of the song is ideal I think. I don't see any reason to save that data between sessions. On a soft reset though we shouldn't free the sample and such as you described. Seems most logical to me. So I think we agree on this. These changes just need tested with a variety of samples and situations before we push it. I'm willing to help test but I'd have to study this area of the code more to actually do anything with it. It's not an area I've messed with much. I'll look at it over the next few days when I get a chance.

[ghost]

Well your looking at this issue from playing tapes only and one case use for journey. The samples really should be stopped on reset by default else a looped sample could repeat forever. If you want to keep low end device sample support loading and unloading when needed youll needed to updated the code to support this with a reset and youll also need to add code to check what sample number is playing on the channel to do it effectively, I wont be fixing it beyond the above which sorts the issue is up to you if you want to spend time on it though