Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ac97 import #3141

Closed
wants to merge 7 commits into from
Closed

Ac97 import #3141

wants to merge 7 commits into from

Conversation

TheDeadFish
Copy link
Contributor

Purpose

Import of AC97 sample driver

Proposed changes

Import AC97 sample driver into drivers\wdm\audio\drivers\ac97
Insert missing definitions required to build into ddk

-- Explanation of commit "fixed buffer issue" --
Every time the DMA enable bit is written to 1, the emulated hardware also increments the buff index. The DMA enable bit was being set repeatedly during playback which screwed up the buffering. The fix checks if DMA is enabled, and if so it does not try and enable it again.

issues

Garbled buffers (sometimes it works perfectly)
Crashes

@Zero3K
Copy link
Contributor

Zero3K commented Sep 6, 2020

The debug log of the crash that I am having with this driver is attached. It is happening when I ran OrgView and let it play audio for 20+ minutes. I also applied the define mentioned at https://jira.reactos.org/browse/CORE-13202?focusedCommentId=124755&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-124755 since the driver doesn't output audio without it.

ac97-crash-reactos.log

@JoachimHenze
Copy link
Contributor

JoachimHenze commented Sep 6, 2020

Colin, I added you for review to check the license one last time. We discussed it here the last time https://jira.reactos.org/browse/CORE-16999?focusedCommentId=122887&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-122887 and you found it to be ok (assumed MIT-licensed).
Remember also your old concern that you raised here https://jira.reactos.org/browse/CORE-16365.
Personally I can not judge whether this is MIT or MS-LPL.

@binarymaster binarymaster added the enhancement For PRs with an enhancement/new feature. label Sep 7, 2020
@Doug-Lyons
Copy link
Contributor

When I opened the "Original driver source" as shown and navigate up a few levels to find a reference to a license, I find this page:
https://github.com/microsoftarchive/msdn-code-gallery-microsoft/tree/master/Official%20Windows%20Driver%20Kit%20Sample/Windows%20Driver%20Kit%20(WDK)%208.1%20Samples
It shows: "License - MS-LPL" and if we look at https://jira.reactos.org/browse/CORE-16365 we can see Colin's statements as follows:
"Many of these samples have been released under Ms-LPL (not Ms-PL), which limits derivative works to run on the "Microsoft Windows operating system product". This may or may not pose a problem for ReactOS, but I want to stay away from Ms-LPL if possible."

@TheDeadFish
Copy link
Contributor Author

TheDeadFish commented Sep 7, 2020

msdn-code-gallery-microsoft repo appears to have conflicting licenses. There is an MIT license at the root of the entire thing. https://github.com/microsoftarchive/msdn-code-gallery-microsoft/blob/master/LICENSE. Very confusing, it looks like MS just concatenated a bunch of separate repos together, the file paths are so long they exceed max_path!

@Doug-Lyons
Copy link
Contributor

@TheDeadFish, That is a very good point. It seems that the license at the TOP should be the controlling one.

@Zero3K
Copy link
Contributor

Zero3K commented Sep 7, 2020

This might apply to the repository that is being linked to:

microsoft/Windows-classic-samples#24

@JoachimHenze
Copy link
Contributor

@TheDeadFish does this driver work flawlessly in Windows 2003 Server?

@TheDeadFish
Copy link
Contributor Author

The driver does work pretty well on win2k3. I am not sure about flawless as in without any dropouts because I am not sure my VM runs perfectly. It must be noted that it only works on windows if built with the microsoft tools, the reactos ddk does not produce functioning audio drivers for windows.

@Zero3K
Copy link
Contributor

Zero3K commented Sep 10, 2020

The only issue with it when installed on 2K3 is that it gets stuck in a loop of a couple of seconds of recently played audio after playing some for a while. It might be related to the noise occurring in ReactOS when playing audio with it installed.

@ColinFinck
Copy link
Member

With GitHub clearly stating "microsoftarchive/msdn-code-gallery-microsoft is licensed under the MIT License" [1] and Raymond Chen officially confirming the relicensing [2], I see the imported AC97 sample driver being available under the permissive MIT license, which is perfect for us!
May even be worth to check if some more code we imported under MS-PL is now available under MIT.

To eradicate any doubts, I suggest copying the LICENSE file [1] to the root of the imported AC97 driver.

[1] https://github.com/microsoftarchive/msdn-code-gallery-microsoft/blob/master/LICENSE
[2] microsoft/Windows-classic-samples#24

@Zero3K
Copy link
Contributor

Zero3K commented Sep 10, 2020

@ColinFinck Its already there as license.txt.

@Zero3K
Copy link
Contributor

Zero3K commented Sep 11, 2020

Here's a debug log of a different crash caused by the same lines of code shown in the previous one that I made with WinDbg:

ac97-crash-reactos-msvc.log

@Zero3K
Copy link
Contributor

Zero3K commented Sep 28, 2020

The garbled and hanging audio is caused by a "bug" in VirtualBox 5.x and up.

@blackcrack
Copy link

blackcrack commented Sep 29, 2020

https://www.virtualbox.org/ticket/19927

( @ALL : for something like that should you announcing here and add a ticket at Oracle, make a login at oracle and you can add some Bugreports at VBox ;) therewith have we are !all a advantage of this and a better running Virtualbox (i meant there with a better open source VirtualMashin .
And at last, linking the ticket here also to see all the Report . )

https://www.virtualbox.org/wiki/Bugtracker

best regards
Blacky

@NailYggdrago

This comment was marked as abuse.

@learn-more
Copy link
Member

@NailYggdrago aka Brian Kirk has been blocked for continuing his harassment, even though he has been banned under multiple accounts already.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement For PRs with an enhancement/new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants