Memotech-Bill / PicoBB

BBC BASIC for Raspberry Pi Pico
zlib License
35 stars 5 forks source link

Support BBCSDL-like sound in console edition #7

Closed rtrussell closed 2 years ago

rtrussell commented 2 years ago

I'm sorry to raise this as an 'issue', but I couldn't find a more appropriate way. To enable BBCSDL-like sound some changes to the files in your repository are necessary. Here are the details:

bbpico.c

146a147,148
> #elif PICO_SOUND == 3
>     ", SDL Sound"
1701a1704,1707
> #if PICO_SOUND == 3
>   envels = (signed char*) (filbuf[0] + 0x800);    // Envelopes
>   waves = (short*) (envels + 0x100);  // Sound wave buffer
> #endif

Makefile

3c3
< SOUND=N
---
> SOUND=SDL
11a12,13
>     ../../BBCSDL/src/sound_sdl.c \
>     ../../BBCSDL/src/bbccos.c \
14d15
<     ../../src/bbccos.c \

CMakeLists.txt These changes were made in an ad-hoc fashion; they cause the other builds to fail so need to be re-worked by somebody who understands CMake!

28c28
<     ../../src/bbccos.c
---
>     ../../BBCSDL/src/bbccos.c
62a63,64
>     pico_multicore
>     hardware_pwm
87a90,103
>   elseif ( ${SOUND} STREQUAL "SDL")
>     message(STATUS "Generate sound like BBCSDL")
>     target_compile_definitions(bbcbasic PUBLIC
>       -DPICO_SOUND=3
>       PICO_MCLOCK
>       PICO_STACK_SIZE=0x3B800
>       PICO_CORE1_STACK_SIZE=0x800
>       )
>     pico_set_linker_script(bbcbasic
>       ${CMAKE_SOURCE_DIR}/../pico_gui/gui_pico.ld
>       )
>     target_sources(bbcbasic PRIVATE
>       ../../BBCSDL/src/sound_sdl.c
>       )
166,167c182,183
<     target_compile_definitions(bbcbasic PUBLIC PICO_STACK_SIZE=0x3C000)
<     pico_set_linker_script(bbcbasic ${CMAKE_SOURCE_DIR}/bbc_pico.ld)
---
> #    target_compile_definitions(bbcbasic PUBLIC PICO_STACK_SIZE=0x3C000)
> #    pico_set_linker_script(bbcbasic ${CMAKE_SOURCE_DIR}/bbc_pico.ld)
173,174c189,190
<     target_compile_definitions(bbcbasic PUBLIC PICO_STACK_SIZE=0x3C000)
<     pico_set_linker_script(bbcbasic ${CMAKE_SOURCE_DIR}/bbc_pico.ld)
---
> #    target_compile_definitions(bbcbasic PUBLIC PICO_STACK_SIZE=0x3C000)
> #    pico_set_linker_script(bbcbasic ${CMAKE_SOURCE_DIR}/bbc_pico.ld)
180,181c196,197
<     target_compile_definitions(bbcbasic PUBLIC PICO_STACK_SIZE=0x3C000)
<     pico_set_linker_script(bbcbasic ${CMAKE_SOURCE_DIR}/bbc_pico.ld)
---
> #    target_compile_definitions(bbcbasic PUBLIC PICO_STACK_SIZE=0x3C000)
> #    pico_set_linker_script(bbcbasic ${CMAKE_SOURCE_DIR}/bbc_pico.ld)
Memotech-Bill commented 2 years ago

I have already made the necessary changes to Makefiles and CMakeLists.txt

I missed:

if PICO_SOUND == 3

envels = (signed char) (filbuf[0] + 0x800); // Envelopes waves = (short) (envels + 0x100); // Sound wave buffer

endif

Are you able to provide a bit more context for this.

rtrussell commented 2 years ago

Are you able to provide a bit more context for this.

It's code which is in BBCSDL (in bbcmos.c) but was deleted from the Console Mode editions because they don't support sound, allowing PAGE to be lowered and making more space available for the user's program. It is necessary to reinstate it in the Pico edition to support BBCSDL-like sound again.

Without it sound shouldn't have worked at all, indeed in my code there is a specific test for waves == NULL (which it will be without that code) to make sure that it doesn't play sound from unallocated memory and make a terrible noise. So it should just have produced silence when you tested it.

Memotech-Bill commented 2 years ago

I meant "context" in terms of position in the source code. However I worked it out.

Your stack sizes required another link file. The GUI link file reserved a large amount of space for the video buffers which you do not require.

I think I have made and comited all the necessary changes, but I am currently short of testing time.

rtrussell commented 2 years ago

meant "context" in terms of position in the source code.

Oh. I thought the line numbers in the diff output provided that information. Here's the code as it appears in my version (note that the additions must come after the other allocations, not where they do in the BBCSDL version):

// Start interpreter:
int entry (void *immediate)
    {
#ifdef PICO_GUI
    allocbuf ();
#else
    accs = (char*) userRAM;     // String accumulator
    buff = (char*) accs + ACCSLEN;      // Temporary string buffer
    path = (char*) buff + 0x100;        // File path
    keystr = (char**) (path + 0x100);   // *KEY strings
    keybdq = (char*) keystr + 0x100;    // Keyboard queue
    eventq = (void*) keybdq + 0x100;    // Event queue
    filbuf[0] = (eventq + 0x200 / 4);   // File buffers n.b. pointer arithmetic!!
#if PICO_SOUND == 3
    envels = (signed char*) (filbuf[0] + 0x800);    // Envelopes
    waves = (short*) (envels + 0x100);  // Sound wave buffer
#endif
#endif
Memotech-Bill commented 2 years ago

Implemented some time back.