[GSoC 2015] Add VideoFrameH264 to CRetroplayerVideo

[acmiyaguchi]

This set of patches adds support for decoding H264 frames through the Retroplayer by taking advantage of the ffmpeg backend of Kodi. This is done in two parts, by adding the functionality into CRetroplayerVideo, and the corresponding callbacks to the Game API.

Having this functionality would help with the work oan game.moonlight.

As a note, this has only been tested on a 64 bit Linux machine, in particular Ubuntu-14.04. I haven't gotten around to building on osx yet.

[garbear]

OK, I applied the patches to my Netplay branch.

I prefer to include interdependent changes in a single commit. that way, reverting is easier and doesn't require tracking down multiple commits. (and would have made rebasing quicker) So, this pr would look like: db9068d. separate commits is fine if it aids reviewability, but in this case the combined commit is much nicer. see the commit's diff - only 7 lines deleted. The closer to zero the number of deleted lines is, the sexier the patch.

The game api version needs to be bumped when a change is made. so this commit should be squash into the PR's commit (EDIT: you'll need to change this to 1.0.8 until I merge some Game API changes). I also approve of good-looking commit logs, so I've started prefixing game API bumps like this: https://github.com/garbear/xbmc/commits/retroplayer-game-api

I added some whitespace improvements that can also be squashed: 35bd9e8. This conforms braces to our coding style, aligns variables with the class's other members, and as a bonus changes the diff from 7 lines deleted to 4 lines deleted (a 43% increase in patch sexiness)

Also, before I take a deeper look, 3934a08 should be squashed as well - better consistency means less surprises later down the road. And prefer unsigned to signed, especially when the variable (size in this case) doesn't make sense as negative.

While I await my GTX card (and a new mobo - my southbridge is busted), I plan to hook this patch up to one of my computer vision projects. the cv project uses theora, so my first change will probably be to rename VideoFrameH264 to VideoPacket

[acmiyaguchi]

You should wait to merge this, there seems to be a regression somewhere.

Program received signal SIGSEGV, Segmentation fault.
0x0000000000000000 in ?? ()
(gdb) bt
#0  0x0000000000000000 in ?? ()
#1  0x0000000000b829ca in GAME::CGameClient::InitSerialization (
    this=this@entry=0x3887a40) at GameClient.cpp:353
#2  0x0000000000b8508f in GAME::CGameClient::OpenFile (this=0x3887a40, 
    file=..., player=player@entry=0x3e2fa40) at GameClient.cpp:305
#3  0x0000000000a38843 in CRetroPlayer::OpenFile (this=0x3e2fa40, file=..., 
    options=...) at RetroPlayer.cpp:89
#4  0x0000000000d8e45f in CApplicationPlayer::OpenFile (this=0x2ab7470, 
    item=..., options=...) at ApplicationPlayer.cpp:102
#5  0x0000000000d7f71f in CApplication::PlayFile (this=0x2ab6e50, item=..., 
    bRestart=bRestart@entry=false) at Application.cpp:3207
#6  0x0000000000b8fc93 in CGUIWindowGames::PlayGame (
    this=this@entry=0x2fd6810, item=...) at GUIWindowGames.cpp:225
#7  0x0000000000b901b1 in CGUIWindowGames::OnClick (this=0x2fd6810, 
    itemNumber=2) at GUIWindowGames.cpp:199
#8  0x0000000000d5cf77 in CGUIMediaWindow::OnMessage (
    this=this@entry=0x2fd6810, message=...) at GUIMediaWindow.cpp:295
#9  0x0000000000b8f263 in CGUIWindowGames::OnMessage (this=0x2fd6810, 
    message=...) at GUIWindowGames.cpp:96
#10 0x00000000009330ee in CGUIBaseContainer::OnClick (
    this=this@entry=0x3e8a090, actionID=100) at GUIBaseContainer.cpp:755
#11 0x0000000000933558 in CGUIBaseContainer::OnMouseEvent (this=0x3e8a090, 
    point=..., event=...) at GUIBaseContainer.cpp:682

[acmiyaguchi]

Ah this should be all well now, there was a lonely file that didn't get touched during the squash. I've tested this against the latest version of game.moonlight, fixing the segfaults.

[garbear]

this was removed, right?

[acmiyaguchi]

Good catch, I seemed to have glossed over it.

[dead]

Using DVDVideoCodec to decode the frames, just a draft.
https://gist.github.com/dead/01af35a9fd94088f1e99#file-retroplayervideo-cpp-L98-L125

[garbear]

I was thinking of doing something similar. I started rewriting RetroPlayer from scratch, given the addition of Audio Engine binary callbacks and the VideoPlayer rename/refactor. I'm working on a retroplayer-17alpha1 branch to take advantage of these changes.

I took a different approach to adding H264 support to the Game API using "video streams": 892c367. I also dropped the Game API's audio callback (see WIP kodi-game/game.moonlight@9eb99bd)

I'm now evaluating the right amount of integration with VideoPlayer. I worry that if RetroPlayer completely bypasses VideoPlayer, we'll have code duplication or miss out on some features (like AV sync), whereas if RetroPlayer is fully integrated (e.g. it becomes a file stream that is played directly by VideoPlayer kinda like PVR) then we'll encounter too much latency and complexity dealing with VideoPlayer's 70,000 lines of code.

I'm still in code-reading mode, so if you have any insights I'd like to hear them

[dead]

I think the best way is to bypass all the VideoPlayer, unfortunately. I'm not sure about the AV sync, moonlight-embedded doesn't seem to care much about and it work good. So the video streams will be decoded by a RetroPlayer with a specified codec/profile? It looks good enough to me.

[a1rwulf]

Please take into account that we need OpenGL support for emulators like mupen64.
My original plan was to figure out how to do this when you finished, but this seems to be a good time to jump into the discussion.
I thought about some sort of encapsulation like vaapi or vdpau - maybe we should ask fernet about his thoughts?

[garbear]

Yea, let's include OpenGL support in this effort. Encapsulation is a must.

[a1rwulf]

I'm not 100% sure if we want to take the whole effort of OpenGL inclusion right now.
It requries shared GL contexts for every platform and we need to do it for OpenGL ES as well.
What's your focus currently and vision for the near future (despite input getting merged)?

[garbear]

The API comes first. Once OpenGL is properly abstracted, it doesn't matter if it's implemented for all platforms, or none.

I've read through VideoPlayer, and it's perfectly suitable for rendering single-frame and packet-based video. It polls for data, but this can be made event-based using some VFS synchronization trickery.

OpenGL rendering doesn't fit into the VideoPlayer architecture too well. This might warrant its own player.

[garbear]

Big update @acmiyaguchi, @a1rwulf - I finished adding game support to the new VideoPlayer. This was a complete failure, the multiple threads meant multiple buffers and multiple delays and memory management challenges.

So I started fresh, and wrote a new RetroPlayer using the successful parts of the old one and the videoplayer work. RetroPlayer has been stripped bare, it contains only video and audio processing (no threads) and delegates everything else to the game add-on.

The new RetroPlayer is now finished. It has stubs for H264 decoding. I'll start the work of integrating this PR into the stubs. I'll go through DVDFactoryCodec like @dead so we can support the rest of VideoPlayer's codecs.

RetroPlayer is ready for OpenGL support too. @a1rwulf - your patch needs updating. You can give it a shot, otherwise when H264 is done I'll port the patch to the new player.

[a1rwulf]

@garbear nice work!
I'll have a look at it ;-)

[garbear]

Check out my retroplayer-playback branch. I'll merge into the primary retroplayer-17alpha1 after some more testing

[garbear]

tested and merged. never had 1000 line addition with so little bugs :)

[garbear]

The final step of integrating this PR has been finished: adding support for encoded audio and video into RetroPlayer. See 2087c6b.

I'll leave this PR open to track progress on game.moonlight until it's finished.

[a1rwulf]

nice :)