VirtualBox

Ticket #14883 (assigned defect)

Opened 4 years ago

Last modified 17 months ago

Sound Blaster 16 Issues Located

Reported by: over_clox Owned by: pentagonik
Component: audio Version: VirtualBox 5.0.10
Keywords: Sound Blaster SB16 Cc:
Guest type: other Host type: other

Description

In Dev16.cpp - sb16WriteAudio, there seems to be a complete oversight in passing any variable to indicate 8/16 bit depth audio, and also the buffer size and calculations need to be adjusted to accommodate for the 64KB standard of the 8237A DMA controller.

Please refer to line 1519 of the current build at: https://www.virtualbox.org/browser/vbox/trunk/src/VBox/Devices/Audio/DevSB16.cpp

Also, I have provided very organized links to official documentation in the forums at:  https://forums.virtualbox.org/viewtopic.php?f=4&t=12425&start=15#p347179

A summary of the troubleshooting measures that lead me to this can be found 2 posts later:  https://forums.virtualbox.org/viewtopic.php?f=4&t=12425&start=30#p347199

Fixes to these issues should theoretically get the long-standing major problems with SB16 emulation out of the way and get most programs and games requiring the typical SB16 features working properly.

Change History

comment:1 Changed 4 years ago by frank

The SB16 implementation is far away from being complete but, sorry to say, your remarks are not very helpful. Of course we aware of the official SB16 documentation and it's also not helpful to enumerate all places with 'todo' tags.

comment:2 Changed 4 years ago by over_clox

My bug ticket was more than helpful, it points out the exact line numbers at fault. The documentation was posted there for the benefit of other volunteer coders. Also, highlighting the todo lines was intended for other volunteer coders that might want to fill in a couple todo sections.

My bug ticket isn't the list of incomplete lines I noted, it's right here, lines 1519 and 1522 where I found an incorrect buffer size and oversight into audio bit depth.

comment:3 Changed 4 years ago by frank

Looking forward to the patches...

comment:4 Changed 4 years ago by frank

Removed the last inappropriate posting.

comment:5 Changed 4 years ago by over_clox

Fair enough, I figured that much anyways. I just don't like when it seems everyone else has nothing but empty discouraging words when I'm really the only one that was doing any actual work and making any actual progress on the issue.

I've gotten down to the root of the main issue. I wasn't expecting anyone to just up and jump on it either. You seemed to take some personal insult to my consolidated information post, which is just ridiculous as I posted that for other volunteers to have everything they might need in one place.

I'm just a volunteer.. WAS a volunteer. It's not my job to (re)write all the code too, so that holding your hand out with "Looking forward to the patches..." is a little off base too as it's completely non-productive.

If you can waste keystrokes discouraging me, why not jump in and help fix the bugs instead?

I dunno, but I'm out. Good luck to any other volunteers, I pass the issue along to the next guy now, whoever cares to work on it.

comment:6 Changed 4 years ago by michael

I noticed this ticket but did not follow it too closely. over_clox, I can see that you are a bit upset at the reaction to your analysis of the code. Do I correctly understand that you opened this bug ticket for the benefit of volunteers who might be interested in doing fixes, or are you hoping that the team at Oracle will do the fixes?

The first seems reasonable, and the second is also a reasonable thought but is probably not something we will be able to do: the SB16 emulation code is not something which we maintain. Unless my memory deceives me, the code paths are not even active in a normal virtual machine configuration. We ported the code very minimally from Qemu in case it was of use to other people, but as it is something which so far has no visible use to Oracle or its paying customers we have not made much attempt at improving it, though we have put company time into integrating the few fixes which came from the community (integrating fixes requires work from us too). We rely on the fact that if there is enough need for fixes in a particular user group someone with the skills needed will produce patches. See the disclaimer in the GPLv2 licence though.

We will leave this ticket open in case anyone feels like working on patches for the code areas you pointed out and does not just do the analysis themselves. If you do not feel like writing the code but would still like to have the fixes, you could look for someone else willing to do it, either in the VirtualBox community or the Qemu one. You might even find that someone has already fixed the Qemu code, in which case you just need someone (which could also be yourself) to test that they can be applied to VirtualBox and adjust them if necessary.

Sorry if that was a bit long.

comment:7 Changed 4 years ago by over_clox

Ah, thank you for your response Michael, and thanks for understanding. I totally understand what you're saying there, and that the SB16 is a mostly unused feature for paying customers. My efforts were all volunteer contributions from my own knowledge base and official documentation, and going back on my original thread and overall view count prompted me to dig a little deeper into the issue. Someone had even taken interest back in 2012 and messaged me requesting my source code to help debug the issue themselves, but I was not around at that time and I realized I had not organized my findings very well and decided to fully correct that as well.

I was just trying to poke a productive stick at the issue, mostly figuring any efforts on this would come from other volunteers and never had any expectation for Oracle to focus on non-mainstream features. That's what baffled me, it was like the moderators themselves totally forgot that volunteers contribute to the efforts too, and their empty discouraging words seem to summarize that they feel no need for ANY productive efforts towards "Other OS" or "Legacy OS" features whatsoever, as I figured that entire section of the forum would be much more maintained by volunteers than any other area of VirtualBox anyways.

It's left me with a bad taste in my mouth and I've decided to leave my findings where they stand for any other volunteers to pick up where I have left off. I stayed on topic and kept to productive efforts whereas the moderators themselves did not, and I simply cannot bring myself to keep tolerating insults to productive efforts. They should grow up and go moderate someone else and leave me and other productive volunteers alone.

Thank you again Michael for understanding, at least someone does. I leave the issue to anyone willing to contribute, no expectations of a fix anytime soon of course, just whenever someone with knowledge trips over the ticket I guess... have a good day.

comment:8 Changed 4 years ago by over_clox

Hmmm, I'm looking back on my old SB16Wave.bas and now that I see this I should point out that the way I programmed it I chose to use a 16KB buffer, so out of my own confusion on all this I went back to the official documents to refresh my memory to see if I had indeed programmed it properly.

On page 7 of the DMA datasheet, under the "Current Word Register" section, it basically states that this register is initially programmed to the length of the buffer being played, and is decremented until transfer is completed. When this value cycles from 0x0000 to 0xFFFF, this indicates that the transfer has completed and the buffer may be reloaded again to initialize a new transfer, no matter the initial buffer size. This is the value being read in my DMADone routine which I mentioned for debugging purposes.

So in summary, this confirms my program is effectively made to correct basic specifications in this regard, and as a reminder this has already been confirmed to work on both real hardware and within DOSBox.

I hope this might relieve anyone's confusion that may have also noticed my choice to use a 16KB buffer within SB16Wave.bas. Though I'm still retracting myself from further handling the VirtualBox side due to unnecessary drama, I have offered my source code to assist in debugging and just wanted to triple check that there was not some mistake I had made to cause this particular situation.

comment:9 Changed 17 months ago by lerdmann

A few corrections/additions

1) the "Current Word Register" needs to be programmed with the intended number of transfers MINUS 1. That's because the DMA controller needs to detect a "wrapover" from 0 to 0xFFFF in order to stop the transfer.

2) the dedicated 16-bit DMA controller (channels 5-7 I seem to remember) is wired up so that a transfer means a 16-bit entity (WORD) while the dedicated 8-bit DMA controller (channels 0 - 3 I seem to remember) is wired up so that a transfer means a 8-bit entity (BYTE).

3) reading the "Current Word Register" to check if the current count = 0xFFFF (the TC "Terminal Count" value) in order to detect end of transfer is possible but not reliable. It's better to read the Status Register which has an explicit "TC" bit for each DMA channel that it supports. Likewise it has a "Request" bit for each DMA channel that indicates if a request is currently active for this DMA channel. The only thing to take into account is that reading the Status Register will also clear it so you better save its contents if you have to deal with multiple DMA channels executing in parallel.

comment:10 Changed 17 months ago by pentagonik

  • Owner set to pentagonik
  • Status changed from new to assigned

comment:11 Changed 17 months ago by pentagonik

Thanks for the comments / additions. We have to see when we find time to validate and squeeze this in. Otherwise, as VirtualBox is Open Source, feel free to submit a patch for those additions / fixes so that we can easily integrate those. Thanks!

Note: See TracTickets for help on using tickets.

www.oracle.com
ContactPrivacy policyTerms of Use