search

joncampbell123 / dosbox-x

DOSBox-X fork of the DOSBox project
GNU General Public License v2.0
2.56k stars 373 forks source link

Wrong usage of libalsa API snd_card_get_name() #2406

Closed Zero0one1 closed 3 years ago

Zero0one1 commented 3 years ago

Describe the bug I found a wrong usage of libalsa API snd_card_get_name() when reading the source code: https://github.com/joncampbell123/dosbox-x/blob/710d58580ef4164aaa1fd72a15ec43928ef2fc7d/vs2015/sdl/src/audio/nto/SDL_nto_audio.c#L87

According to the API document, the snd_card_get_name() accept 2 parameters not 3.

To Reproduce Steps to reproduce the behavior:

  1. Go to '...'
  2. Click on '....'
  3. Scroll down to '....'
  4. See error

Expected behavior A clear and concise description of what you expected to happen.

Screenshots If applicable, add screenshots to help explain your problem.

Environment (please complete the following information):

Additional context Add any other context about the problem here.

joncampbell123 commented 3 years ago

Done

Wengier commented 3 years ago

According to the QNX API the snd_card_get_name() function does take 3 parameters (see: http://www.qnx.com/developers/docs/6.5.0/index.jsp?topic=%2Fcom.qnx.doc.neutrino_audio%2Flibs%2Fsnd_card_get_name.html). But you are right that according to the libalsa API only 2 parameters are expected.

Zero0one1 commented 3 years ago

@Wengier So the QNX API document is wrong?

Wengier commented 3 years ago

@Zero0one1 The document may be correct for QNX's Audio Library. But there can be different specifications for the same function in different implementations.

Zero0one1 commented 3 years ago

@Wengier Sorry to bother you again. I don't know much about QNX and didn't get the difference between the two snd_card_get_name() and how to decide which one to use.

Can I say QNX's Audio Library is based on ALSA and modified by the developers of QNX? If I follow their specification, my code can only run on the QNX because their snd_card_get_name() lies in the <sys/asoundlib.h> not <alsa/asoundlib.h>. So we change 3 parameters to 2 in order to work on other platforms.

However, I don't find QNX in the supported platforms of README.md. So why we refer to the QNX document?

Thanks for your time. 😃

I found 2 new API misuses of snd_card_get_longname(): https://github.com/joncampbell123/dosbox-x/blob/4a819268a50673cb9fcb7111d7fcb2abf53d15c7/vs2015/sdl2/src/audio/qsa/SDL_qsa_audio.c#L506-L509 https://github.com/joncampbell123/dosbox-x/blob/4a819268a50673cb9fcb7111d7fcb2abf53d15c7/vs2015/sdl2/src/audio/qsa/SDL_qsa_audio.c#L564-L567

Wengier commented 3 years ago

@Zero0one1 The point is that there does exist an alternative specification of the snd_card_get_name() function with 3 parameters, although it is probably only used by certain platform(s) and/or some older version(s) of the API. On the other hand, the main ALSA API does appear to use the snd_card_get_name() function with 2 parameters.

Zero0one1 commented 3 years ago

@joncampbell123 I found another 2 API misuses. Can you take a look? I can't re-open this issue.

Wengier commented 3 years ago

I guess the files SDL_qsa_audio. are specifically for QNX Sound Architecture (QSA) and the files SDL_nto_audio. for QNX Neutrino (NTO), instead of general ALSA libraries.

Edit: Check out https://github.com/joncampbell123/dosbox-x/commit/0b35d1d5ec5c60369210bd8308fd40863e70ab0f#commitcomment-49384529 and the latest code.

Zero0one1 commented 3 years ago

Got it. Thank you for the explanation. 😸