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

H.264 decoder support #1194

Merged
merged 2 commits into from
Jan 21, 2022
Merged

H.264 decoder support #1194

merged 2 commits into from
Jan 21, 2022

Conversation

xornet-sl
Copy link
Contributor

This decoder supports H264 ustreamer from pikvm.org project.
I think we shoud also change rfb standard and add h264 there. Also I will work on server-side implementation in tigervnc.
H.264 streaming allows reduce network utilization, sometimes dramatically. So I beleive it should be useful.
This PR contains ffmpeg libav implementation for linux and media foundation for windows.
I added a cmake option ENABLE_H264, so it can be built without H.264 support if any problems occurs
Any comments are welcome!

@mdevaev
Copy link

mdevaev commented Feb 4, 2021

See detailed description: #1187 (comment)

@mdevaev mdevaev mentioned this pull request Feb 4, 2021
@CendioOssman
Copy link
Member

Nice. :)

I think it is best if we nail down the behaviour of this encoding before we start getting in to the details of the code though. So could you get a first draft going for @rfbproto? Once we've nailed the details of how this should behave, we can start looking at if the code actually follows that behaviour. :)

@mdevaev
Copy link

mdevaev commented Feb 4, 2021

I'll take care of it immediately ;)

mdevaev added a commit to mdevaev/rfbproto that referenced this pull request Feb 4, 2021
The story:
- TigerVNC/tigervnc#1187

The reference implementation:
- TigerVNC/tigervnc#1194
@mdevaev
Copy link

mdevaev commented Feb 4, 2021

@CendioOssman here the draft: rfbproto/rfbproto#39

@xornet-sl
Copy link
Contributor Author

About package building: Deb-based systems have libav* and ffmpeg in their repos but rpm-based systems don't. I don't have any experiense with building packages under rpm-based systems. Also I have no macOS nearby to debug building proccess there too. It would be great if anyone could help me with these systems!

@CendioOssman
Copy link
Member

Red Hat derived things do not include ffmpeg and such because of patent threats. They have some form of OpenH264 though.

Red Hat users generally use rpmfusion to get things that Red Hat refuses to package.

@xornet-sl
Copy link
Contributor Author

xornet-sl commented Feb 4, 2021

Red Hat users generally use rpmfusion to get things that Red Hat refuses to package.

Yes, I know that, but how can I reference this repo as a dependency? Is there some right way to do so?

@xornet-sl
Copy link
Contributor Author

arghhh.. I found some issues with versions of ffmpeg. older versions have a different API and older systems won't work. It is time to do more #ifdef logic 🤬

@CendioOssman
Copy link
Member

Red Hat users generally use rpmfusion to get things that Red Hat refuses to package.

Yes, I know that, but how can I reference this repo as a dependency? Is there some right way to do so?

If you're talking about our Travis builds, then you probably shouldn't. Those builds should target a standard Red Hat. So this feature will have to be disabled there.

@xornet-sl
Copy link
Contributor Author

xornet-sl commented Feb 4, 2021

travis is useless....

 library/ubuntu
toomanyrequests: You have reached your pull rate limit. You may increase the limit by authenticating and upgrading: https://www.docker.com/increase-rate-limit
The command "docker build -t tigervnc/$DOCKER .travis/$DOCKER" failed and exited with 1 during .
Your build has been stopped.

Can we restart a single job instead of queueing a full rebuild again?

@CendioOssman
Copy link
Member

Yes, at least I can. But it might be because I'm an admin of the project.

I've poked Travis support about this. We'll see what they say about getting those caches working properly.

@QuattroCS
Copy link

QuattroCS commented Mar 15, 2021

@xornet-sl got past the cmake, got 3 errors on make (mac os x 10.15.7)

/Users/QuattroCS/TigerVNC/tigervnc/common/rfb/CConnection.cxx:51:5: error: multiple initializations given for non-static member 'supportsLocalCursor'
    supportsLocalCursor(false), supportsCursorPosition(false),
    ^~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/QuattroCS/TigerVNC/tigervnc/common/rfb/CConnection.cxx:46:5: note: previous initialization is here
    supportsLocalCursor(false), supportsDesktopResize(false),
    ^~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/QuattroCS/TigerVNC/tigervnc/common/rfb/CConnection.cxx:52:5: error: multiple initializations given for non-static member 'supportsDesktopResize'
    supportsDesktopResize(false), supportsLEDState(false),
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/QuattroCS/TigerVNC/tigervnc/common/rfb/CConnection.cxx:46:33: note: previous initialization is here
    supportsLocalCursor(false), supportsDesktopResize(false),
                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/QuattroCS/TigerVNC/tigervnc/common/rfb/CConnection.cxx:52:35: error: multiple initializations given for non-static member 'supportsLEDState'
    supportsDesktopResize(false), supportsLEDState(false),
                                  ^~~~~~~~~~~~~~~~~~~~~~~
/Users/QuattroCS/TigerVNC/tigervnc/common/rfb/CConnection.cxx:47:5: note: previous initialization is here
    supportsLEDState(false),
    ^~~~~~~~~~~~~~~~~~~~~~~
3 errors generated.
make[2]: *** [common/rfb/CMakeFiles/rfb.dir/CConnection.cxx.o] Error 1
make[1]: *** [common/rfb/CMakeFiles/rfb.dir/all] Error 2
make: *** [all] Error 2

@QuattroCS
Copy link

QuattroCS commented Mar 15, 2021

In file included from /Users/QuattroCS/TigerVNC/tigervnc/common/rfb/Decoder.cxx:30:
/Users/QuattroCS/TigerVNC/tigervnc/common/rfb/H264Decoder.h:78:23: error: in-class initialization of non-static data member is a C++11 extension [-Werror,-Wc++11-extensions]
      SwsContext* sws = NULL;
                      ^
/Users/QuattroCS/TigerVNC/tigervnc/common/rfb/H264Decoder.h:79:26: error: in-class initialization of non-static data member is a C++11 extension [-Werror,-Wc++11-extensions]
      uint8_t* swsBuffer = NULL;
                         ^
/Users/QuattroCS/TigerVNC/tigervnc/common/rfb/H264Decoder.h:80:34: error: in-class initialization of non-static data member is a C++11 extension [-Werror,-Wc++11-extensions]
      rdr::U8* h264AlignedBuffer = NULL;
                                 ^
3 errors generated.
make[2]: *** [common/rfb/CMakeFiles/rfb.dir/Decoder.cxx.o] Error 1
make[1]: *** [common/rfb/CMakeFiles/rfb.dir/all] Error 2
make: *** [all] Error 2

@xornet-sl
Copy link
Contributor Author

@xornet-sl got past the cmake, got 3 errors on make (mac os x 10.15.7)

/Users/QuattroCS/TigerVNC/tigervnc/common/rfb/CConnection.cxx:51:5: error: multiple initializations given for non-static member 'supportsLocalCursor'
    supportsLocalCursor(false), supportsCursorPosition(false),
    ^~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/QuattroCS/TigerVNC/tigervnc/common/rfb/CConnection.cxx:46:5: note: previous initialization is here
    supportsLocalCursor(false), supportsDesktopResize(false),
    ^~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/QuattroCS/TigerVNC/tigervnc/common/rfb/CConnection.cxx:52:5: error: multiple initializations given for non-static member 'supportsDesktopResize'
    supportsDesktopResize(false), supportsLEDState(false),
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/QuattroCS/TigerVNC/tigervnc/common/rfb/CConnection.cxx:46:33: note: previous initialization is here
    supportsLocalCursor(false), supportsDesktopResize(false),
                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/QuattroCS/TigerVNC/tigervnc/common/rfb/CConnection.cxx:52:35: error: multiple initializations given for non-static member 'supportsLEDState'
    supportsDesktopResize(false), supportsLEDState(false),
                                  ^~~~~~~~~~~~~~~~~~~~~~~
/Users/QuattroCS/TigerVNC/tigervnc/common/rfb/CConnection.cxx:47:5: note: previous initialization is here
    supportsLEDState(false),
    ^~~~~~~~~~~~~~~~~~~~~~~
3 errors generated.
make[2]: *** [common/rfb/CMakeFiles/rfb.dir/CConnection.cxx.o] Error 1
make[1]: *** [common/rfb/CMakeFiles/rfb.dir/all] Error 2
make: *** [all] Error 2

Ooops. My fault, wrong merge, sorry. Fixed

@QuattroCS
Copy link

@xornet-sl thank you sir

got another error elsewhere

@xornet-sl
Copy link
Contributor Author

In file included from /Users/QuattroCS/TigerVNC/tigervnc/common/rfb/Decoder.cxx:30:
/Users/QuattroCS/TigerVNC/tigervnc/common/rfb/H264Decoder.h:78:23: error: in-class initialization of non-static data member is a C++11 extension [-Werror,-Wc++11-extensions]
      SwsContext* sws = NULL;
                      ^
/Users/QuattroCS/TigerVNC/tigervnc/common/rfb/H264Decoder.h:79:26: error: in-class initialization of non-static data member is a C++11 extension [-Werror,-Wc++11-extensions]
      uint8_t* swsBuffer = NULL;
                         ^
/Users/QuattroCS/TigerVNC/tigervnc/common/rfb/H264Decoder.h:80:34: error: in-class initialization of non-static data member is a C++11 extension [-Werror,-Wc++11-extensions]
      rdr::U8* h264AlignedBuffer = NULL;
                                 ^
3 errors generated.
make[2]: *** [common/rfb/CMakeFiles/rfb.dir/Decoder.cxx.o] Error 1
make[1]: *** [common/rfb/CMakeFiles/rfb.dir/all] Error 2
make: *** [all] Error 2

fixed

@QuattroCS
Copy link

@xornet-sl make and make dmg passed woohoo thank you.

now i need to figure out how to include dependencies in the dmg.

@xornet-sl
Copy link
Contributor Author

xornet-sl commented Mar 15, 2021

@xornet-sl make and make dmg passed woohoo thank you.

now i need to figure out how to include dependencies in the dmg.

Thank you very much for helping us!

@QuattroCS
Copy link

@xornet-sl make and make dmg passed woohoo thank you.
now i need to figure out how to include dependencies in the dmg.

Thank you very much for helping us!

Absolutely, thank you for the quick responses.

can you point me in the right direction on the dependencies being included in the dmg?

@xornet-sl
Copy link
Contributor Author

@xornet-sl make and make dmg passed woohoo thank you.
now i need to figure out how to include dependencies in the dmg.

Thank you very much for helping us!

Absolutely, thank you for the quick responses.

can you point me in the right direction on the dependencies being included in the dmg?

Unfortunately I have no experience with OSX at all :(
What changes did you do in cmake to build this pull request on OSX?

@QuattroCS
Copy link

QuattroCS commented Mar 15, 2021

Unfortunately I have no experience with OSX at all :(
What changes did you do in cmake to build this pull request on OSX?

ok no worries

no changes - specifically - I had to brew install
libav - found that in your doc in travis
ffmpeg - found that in the comments
gettext - by checking the first cmake i did
gnutls - from the build doc
jpeg - from cmake error
fltk - i dont remember

i'm in mac os x 10.15.7

i dont know if other things i have installed were used in this build.

@xornet-sl
Copy link
Contributor Author

xornet-sl commented Mar 15, 2021

i dont know if other things i have installed were used in this build.

I think ffmpeg/libav should be just enough because this PR adds includes only for ffmpeg/libav header files. I just have no mac to build it by my own hands :) So I think we just need somehow to add ffmpeg/libav deps and it should be nice.

@QuattroCS
Copy link

i dont know if other things i have installed were used in this build.

I think ffmpeg/libav should be just enough because this PR adds includes only for ffmpeg/libav header files. I just have no mac to build it by my own hands :) So I think we just need somehow to add ffmpeg/libav deps and it should be nice.

i edited a couple more, jpeg was def stuck on cmake and it got past with it.

@xornet-sl
Copy link
Contributor Author

gettext - by checking the first cmake i did
gnutls - from the build doc
jpeg - from cmake error
fltk - i dont remember

All these deps should be already at their place. They have no relation to this particular PR

@QuattroCS
Copy link

All these deps should be already at their place. They have no relation to this particular PR

understood, i didn't want to leave any info behind.

@xornet-sl
Copy link
Contributor Author

@CendioOssman Hi, I see that travis has gone somewhere. Something went wrong with CI?

@xornet-sl
Copy link
Contributor Author

Rebased and squashed everything on top of current master. @CendioOssman please check it

@xornet-sl
Copy link
Contributor Author

xornet-sl commented Jan 21, 2022

@zhangyoufu Thanks. fixed.
Updated: found these libavdevice & libavformat in debian control files as build dependencies. Fixed too.

@xornet-sl
Copy link
Contributor Author

xornet-sl commented Jan 21, 2022

@CendioOssman All checks have passed! So, could we merge it now?

@CendioOssman
Copy link
Member

Looks good. Thanks for all the hard work! :)

@xornet-sl
Copy link
Contributor Author

Looks good. Thanks for all the hard work! :)

It's not over. I'm starting to work on server-side implementation

@CendioOssman CendioOssman merged commit ec54fc8 into TigerVNC:master Jan 21, 2022
@xornet-sl
Copy link
Contributor Author

Hooray! Thank you!

@xornet-sl xornet-sl deleted the h264dec branch January 21, 2022 15:49
@bphinz
Copy link
Member

bphinz commented Jan 25, 2022 via email

@CendioOssman
Copy link
Member

I guess you also have a rather old version of gcc? :)

Normally we avoid C+11 since stuff like RHEL 7 doesn't support that fully. I didn't notice it here since we don't build these parts for RHEL, and once I did I hoped other platforms would have more updated gcc.

@bphinz
Copy link
Member

bphinz commented Jan 25, 2022 via email

@xornet-sl
Copy link
Contributor Author

@bphinz @CendioOssman
Opened #1419 to fix this
@bphinz you can try to compile this branch to test if everything is ok now

@xornet-sl
Copy link
Contributor Author

@CendioOssman could you please answer to me via e-mail? I wrote on e-mail that is in your commits a few days ago. I have some questions to brainstorm with you about Encoder support. please answer me to [email protected] if you have some time

@bphinz
Copy link
Member

bphinz commented Jan 26, 2022 via email

@CendioOssman
Copy link
Member

I sent you a reply the other day. gmail has been acting up lately though, so check if it ended up in a spam folder or such.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants