search

abrenoch / hyperion-android-grabber

Screen grabber for hyperion
MIT License
207 stars 36 forks source link

flatbuffers instead of protocol buffers #186

Closed flos06 closed 2 years ago

flos06 commented 2 years ago

Hi. Would it be possible for the app to use flatbuffers instead of protocol buffers?

abrenoch commented 2 years ago

duplicate of #139

Unless protobuffs are being dropped or there is a substantial advantage to using flatbuffers, I don't really plan on adding it. If someone else wants to put in the work I would welcome a PR.

flos06 commented 2 years ago

I understand. To specify the advantage for me would be that HyperHDR supports HDR to SDR tone mapping via flatbuffers but not via proto buffers. I tried fiddling around myself but converting appears to be to difficult for me unfortunately

abrenoch commented 2 years ago

Interesting... I'm not familiar with HyperHDR. This might warrant some more research!

flos06 commented 2 years ago

Thanks for looking into it. There was a pull request here: https://github.com/awawa-dev/HyperHDR/pull/215 and it has been merged. It has not made it into the latest release I don't believe so I compiled it myself. Alas it is only for flatbuffers

flos06 commented 2 years ago

I'm really trying to make this work, however I'm stuck. I've been trying to simply replace the gradle plugins etc however obviously the HyperionProto package is not being generated since I'm trying to use flatbuffers. To generate the proper package I need to rewrite the schema I believe?

Unfortunately I am unable to find documentation that explains how to rewrite protobuffers to flatbuffers via relatively simple steps. So I am afraid I will be unable to do this myself for the time being. If anyone else would be willing to give it a shot I would be incredibly grateful.

abrenoch commented 2 years ago

It has been a while since I got that working, but I recall it wasn't very intuitive. The .proto (for protobuf) or .fbs (for flatbuffer) acts as a template to generate the protocol by. I cannot recall at the moment if that happens as part of the build process in android studio or if it was something I had to run separately.

Luckily the flatbuffer schema for HyperHDR and hyperion appear to be identical, but that then brings another question...

Looking over the PR you linked to, It seems the HDR mapping is happening in the client itself (sources/flatbufserver/FlatBufferClient.cpp:180), and it doesn't appear any special data is being sent using the flatbuffer protocol. That behavior appears to be baked into their client, so that is doing the tone mapping before sending the data over to the server. I could be wrong but that is the impression I'm getting.

Unfortunately there is more going on with that than just a different protocol... However the tone mapping behavior might be something that could happen in this app. That is new to me!

flos06 commented 2 years ago

I'm honestly not sure how exactly it works. I am very much a beginner regarding programming. The way I thought it worked was that the data gets send over the flatbuffers protocol and then HyperHDR does the tone mapping, but I am sure you are more knowledgable than me in that regard.

Regarding the proto template thingy. I am pretty sure the protocol gets generated as part of the build process using the .proto template. Seems the same with flatbuffers.

Edit: For reference and transparancy I have also opened up an issue over at HyperHDR to ask if they could maybe implement the same for protobuf but no dice unfortunately. ( https://github.com/awawa-dev/HyperHDR/issues/263 ) He does make it seem as though swapping protobuf for flatbuffers would be enough unless I am misunderstanding him.

flos06 commented 2 years ago

The problem is solved. HyperHDR has added support for protobuffers HDR tone mapping.

abrenoch commented 2 years ago

Neat, able to try it out yet? I'm curious if it works out well.

flos06 commented 2 years ago

Yes it works perfect for HDR10. Colors are accurate. No support for DV unfortunately, but I think that has to do with the captured image.

abrenoch commented 2 years ago

Well thanks for bringing this to my attention... I find it really interesting, I wasn't aware HDR tone mapping was a thing (granted I havent looked into it in a while now). I wonder if that is something that can happen client-side in this application.