moonlight-stream / moonlight-embedded

Gamestream client for embedded systems
https://github.com/moonlight-stream/moonlight-embedded/wiki
GNU General Public License v3.0
1.51k stars 325 forks source link

Adding OpenMAX Support #311

Closed Migs3 closed 8 years ago

Migs3 commented 8 years ago

Not really an issue, but Not sure how to add a request for this...

Any chance of adding OpenMAX support rather than ALSA or PulseAudio? ALSA doesn't support multichannel PCM over HDMI yet on the Raspberry Pi (2) due to a kernel/driver issue and PulseAudio is a nightmare to setup and configure... OpenMAX seems to work very well on other applications, just wondering if this would work better for this here...? I am considering about diving into the code to see if I can implement it by piecing together parts from hello_audio and maybe from kodi, but my c is rusty... Something that would take me a few weeks to put together and debug might be done in 5 minutes by someone that knows the code better...

Thanks!

Migs3 commented 8 years ago

Well, this is what I came up with, which compiles, but It just gets a segmentation fault when starting the audio... Not that it's the problem, but there's gotta be a better way to reference the OMX/IL API. Especially since the BROADCOM that's already included in moonligh-pi references much of the OpenMAX libraries/includes as is... but I don't think I can put this into moonlight-pi without more major modifications to audio.h and platform.c to expand it into the moonlight-pi target. But anyway, this is my omx.c file at least. I made a few more modifications in order to get it to compile and reference OMX instead of ALSA, platform.c, audio.h, and also added a few libraries and include folders...

snip See https://github.com/Unknownforce351/moonlight-embedded

Migs3 commented 8 years ago

Figured out the issues with segmentation fault, I wasn't initializing the buffer properly. Got it working as it should, however, all I get is bad static now. I've switched to using ilclient, as it seems to handle the component commands better.

I'm wondering if the bad sound is coming from the casting of uint8_t onto the pcmBuffer decoded from opus? If so, what's the proper way to convert a short to uint8_t?

I did, however, notice that all I'm getting is static when I use -surround on just master... Not sure what changed that would affect that... I think the RPi just had an update to some firmware pieces, maybe that could have messed with it a bit?

Either way, Here's the most recent code:

snip see https://github.com/Unknownforce351/moonlight-embedded

dead commented 8 years ago

Not sure if this will help but, take a look a this: https://github.com/xbmc/xbmc/blob/f42c66f130805765b0c30d947980d5f2af847cde/xbmc/cores/omxplayer/OMXAudio.cpp#L1164-L1172

I think you need properly memcpy the buffer into buff_header->pBuffer

Migs3 commented 8 years ago

That was the first way I did it, but got the same result, so I switched to casting, but still the same thing... I'm considering using OMX's audio_decode, but that adds a lot of code :(

dead commented 8 years ago

Try setting buff_header->nOffset = 0; buff_header->nFlags = OMX_BUFFERFLAG_TIME_UNKNOWN; buff_header->nTimeStamp = 0; And definitely use memcpy.

I think you are in the way!

Migs3 commented 8 years ago

buff_header->nTimeStamp = 0; This doesn't work because it's expecting an OMX_TICKS which is typedef OMX_S64 OMX_TICKS which is typedef signed long long OMX_S64

Not too sure how to fill that value properly.

Migs3 commented 8 years ago

I added this:

static inline OMX_TICKS ToOmxTicks(int64_t value) {
    OMX_TICKS s;
    s.nLowPart = value;
    s.nHighPart = value >> 32;
    return s;
}

and made it: buff_header->nTimeStamp = ToOmxTicks(0);

I'll test when I get home, I'm at work right now, working hard... obviously ;)

Migs3 commented 8 years ago

For testing, I've forked and committed changes here: https://github.com/Unknownforce351/moonlight-embedded

In case anyone wants to test or follow along.

Migs3 commented 8 years ago

Fixed it, it's working properly now. The Buffer size was being sent incorrectly. Although my surround sound is still not working for me, but I think it's a problem on my end. Can someone with working surround sound and a raspberry pi connected via HDMI test with this?

dead commented 8 years ago

Good to hear! I don't have a surround system so I can't test it...

Migs3 commented 8 years ago

Not sure why my surround sound is bad with surround option on, it's like it's playing too fast, I can hear voices, make out sounds, but for example, in Diablo III, Tyreal has a deep voice, but it sounds like he just inhaled a bunch of helium. along with crackling of course...

dead commented 8 years ago

LOL. Seems like just a miss configuration or a bad parameter in the code... I don't know where the * channelCount came from, but PulseAudio use this way too...

In Kodi: unsigned pitch = (m_Passthrough || m_HWDecode) ? 1:(m_BitsPerSample >> 3) * m_InputChannels; and omx_buffer->nFilledLen = samples * pitch;

Pitch is 1 or (16 >> 3) * channelCount So maybe this could work: int rc = omx_play_buffer(pcmBuffer, decodeLen * sizeof(short) * (16 >> 3) * channelCount);

cgutman commented 8 years ago

opus_multistream_decode's return value isn't what you think it is. It returns the number of samples decoded. Since each sample is a 16-bit value for each channel, the total length of decoded audio data in the output buffer is: opus_multistream_decode() * sizeof(short) * channelCount

dead commented 8 years ago

Oh, thanks for the clarification @cgutman...

Well, looking at the Kodi code

memset((unsigned char *)omx_buffer->pBuffer, 0x0, omx_buffer->nAllocLen);
memcpy((unsigned char *)omx_buffer->pBuffer, &m_wave_header, omx_buffer->nFilledLen);
omx_buffer->nFlags = OMX_BUFFERFLAG_CODECCONFIG | OMX_BUFFERFLAG_ENDOFFRAME;
omx_err = m_omx_decoder.EmptyThisBuffer(omx_buffer);

Maybe you need initialize with a "m_wave_header" like that, when using more than two channels... (I bet this will fix the problem, it is the main difference between your and kodi code)

And when decoding the first packet kodi do this:

if(m_setStartTime)
{
  omx_buffer->nFlags = OMX_BUFFERFLAG_STARTTIME;
  if(pts == DVD_NOPTS_VALUE)
    omx_buffer->nFlags |= OMX_BUFFERFLAG_TIME_UNKNOWN;

  m_last_pts = pts;

  CLog::Log(LOGDEBUG, "COMXAudio::Decode ADec : setStartTime %f\n", (float)val / DVD_TIME_BASE);
  m_setStartTime = false;
}
Migs3 commented 8 years ago

I don't know if it's a code issue anymore... I think I screwed something up on my Pi. Cause even with Alsa, it now has bad audio if I use -surround, which it did not before, it still worked, just didn't get output from anything other than 2 channels. I'm reloading Raspbian on another sd card to see if I can at least get it working on Alsa again. Then try my code again.

Migs3 commented 8 years ago

Well, no dice, my -surround isn't working anymore on raspbian jessie. Not sure if it's because of the latest updates to the kernel or something in embedded, or in common... but using -surround produces bad noise using fresh build from master. I know it's not a config thing because kodi puts out surround sound with no issues... When I get more time I have to try previous builds.

dead commented 8 years ago

I tested it here with -surround and the audio was very low and with static... And for some reason I had to restart my tv because after testing it.

Without -surround the sound was normal...

The output (was the same with or without -surround)

Moonlight Embedded 2.1.4 (EMBEDDED;PI) Connect to 192.168.1.2... Stream 1280 x 720, 60 fps, 20000 kbps Initializing platform...done Resolving host name...done Starting RTSP handshake...done Initializing control stream...done Initializing video stream...done Initializing audio stream...done Initializing input stream...done Starting control stream...done Starting video stream...done Starting audio stream...OMX error OMX_ErrorSameState done Starting input stream...done

Using a normal version of moonlight the -surround worked without any problem.

dead commented 8 years ago

Found out the problem... Was the sPCMMode.nChannels = OUT_CHANNELS(num_channels); Using only sPCMMode.nChannels = num_channels; the surround works!

Migs3 commented 8 years ago

Interesting. I still have a problem using -surround from the non-OMX version... I'll have to test this tonight though, I would be a happy man if this gets my surround working! Thanks!

As far as the OMX_ErrorSameState error, it doesn't seem to affect the audio or the quality or anything. It just seems like either it's sitting in Idle state already for some reason after you run it for the first time. The error does not happen the first time around, but each time after that it does. I might just add a check in to see what state it's in and then only request to change the state, if it's not in the desired state. OMX is supposed to go into Loaded state when it gets initialized, then after initialization, it should be put into Idle state so that a couple other commands can be put in, then the transition to Executing state happens, so that buffers can start being filled and emptied. Seems like either something with the teardown/destroy/deinit functions doesn't quite clear it up as it should, so a simple check state before setting state should resolve the error message.

Migs3 commented 8 years ago

This should fix that error and surround then.

https://github.com/Unknownforce351/moonlight-embedded/commit/ccadfa596a666e5350fc584790629a68cdcc750a

Migs3 commented 8 years ago

Updated to latest Master and added -omx option (assuming my implementation method works lol)

dead commented 8 years ago

Why use OMX instead PI? I had to make some changes to compile here.

CMakeLists.txt:

add_library(moonlight-pi SHARED ./src/video/pi.c ./src/audio/omx.c ${ILCLIENT_SRC_LIST})
target_include_directories(moonlight-pi PRIVATE ./third_party/ilclient ${BROADCOM_INCLUDE_DIRS} ${GAMESTREAM_INCLUDE_DIR} ${MOONLIGHT_COMMON_INCLUDE_DIR} ${OPUS_INCLUDE_DIRS})
target_link_libraries(moonlight-pi gamestream ${BROADCOM_LIBRARIES} ${OPUS_LIBRARY})

And platform.c

case SDL:
    return &audio_callbacks_sdl;
#endif
#ifdef HAVE_PI
case PI:
    return (PAUDIO_RENDERER_CALLBACKS) dlsym(RTLD_DEFAULT, "audio_callbacks_omx");
#endif

And #include "OMX_Core.h" to #include "IL/OMX_Core.h"

With this you don't need the FindOMX stuff and the ilclient is included in the moonlight-pi.

Migs3 commented 8 years ago

Just didn't know how to cross it into the moonlight-pi target. But after thinking about it... OpenMAX isn't exclusive to the PI either to my knowledge... Don't want to limit the use of it to only on the PI, do we?

dead commented 8 years ago

But the ilclient is for pi only. :cry:

Migs3 commented 8 years ago

Ahhh, true, I guess I'd have to move away from using ilclient and strictly use OMX functions then... hmm OK.

dead commented 8 years ago

Not sure if there is any other device that really support OpenMAX beside pi. Some people that use Odroid said it support too, but that is not really true. For Odroid, people should just put an Android OS and download the moonlight in the play store.

Migs3 commented 8 years ago

OK, Removed the FindOMX stuff, cleaned up code a bit, removed the surround sound mapping changes, because ALSA mapping is different from OMX. Opus and OMX mappings line up, so no need to change them. Also, I think one of the PCM flags were wrong, I set the eNumData to Signed Data, where I think the buffer data actually is unsigned, so I switched that to Unsigned.

I also fixed my -surround issues on my setup by updating my Nvidia drivers to 362.00 and downgrading to GFE 2.9... Not sure whether it was the drivers or GFE 2.10 that was causing the sound corruption, but I now have surround sound! I still have one problem, the Center Channel is either SUPER quiet or it's simply not being rendered. It's possible the difference between signed and unsigned data was the cause, but I won't know that until I get home and test. I double checked the num_channels code for setting up the OMX device, but that seems fine... Not sure where the problem could be just yet. More testing to do...

Migs3 commented 8 years ago

Unsigned made it not work anymore, so that isn't it. Not sure why the Center Channel doesn't work... All the code seems fine, I know the center channel works in Kodi, so it can't be a hardware setup issue...

Migs3 commented 8 years ago

Ahhhh, it was a mapping thing, lol... I couldn't hear it because it was LFE and Center swapped. The rest of the channels are fine, after messing with it and playing a 5.1 sound file louder, I heard center coming from the sub and LF noise coming from the center. Messed with mapping a bit and voila, it's working! Going to update code and cleanup and put in a pull request.

dead commented 8 years ago

nice! :smile:

Migs3 commented 8 years ago

After playing for a while all weekend, I'm gonna hold off the pull request until I can test some more, cause I noticed some fairly major issues... The main one is that the sound, for no apparent reason or cause that I can tell, will simply cut out for a bit. This also makes the feed super choppy. I'm streaming over ethernet, so I know it's not a wifi signal/consistency problem, I'm not sure if it's a bitrate issue or not, I do use -bitrate 30000, for the little extra available bandwidth, assuming that function works properly. But I haven't noticed any specific cause or pattern to it. I have noticed however, that if I pause the game, and wait, it seems to "clear up" faster, whereas if I continue playing like that, it seems like it doesn't want to come back at all. So with that... it seems like it's almost getting overloaded and doesn't know how to compensate, so it just drops all packets or something... I don't remember these kinds of issues using ALSA, but I also wasn't using -surround before. Could it be adding that much bandwidth that it could be causing those kinds of issues? Any way of determining that?

Another issue I noticed, it screwed up the mapping of channels once... center was coming from right rear all of a sudden. Cleared up when I restarted the stream... This only happened a single time, so it could have been a fluke, haven't had it happen since, so I'm not sure exactly what happened there. But still worth mentioning.

I also still have to figure out why the OMX_ErrorSameState keeps happening, I am thinking it's because bcm_host_init is already getting called by the the video engine and might be causing a conflict there, because I noticed that it uses OMX to render video on the PI.

Migs3 commented 8 years ago

The issues were limited to my setup. After a reboot of my host, the audio issues I was having went away. GFE is very touchy apparently... Lol... I went through and standardized/cleaned up the code to match the rest of the project and the other audio code structures. Instead of the -omx option, I made it use -audio with either "omxhdmi" or "omxlocal" as a device (OMX audio_render can use hdmi or the analog (local) output) This is fairly "polished" and finalized now. Going to submit a pull request now.