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

flatbuffers instead of protocol buffers #186

Closed
flos06 opened this issue Apr 28, 2022 · 11 comments
Closed

flatbuffers instead of protocol buffers #186

flos06 opened this issue Apr 28, 2022 · 11 comments

Comments

@flos06
Copy link

flos06 commented Apr 28, 2022

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

@flos06 flos06 changed the title HDR to SDR tone mapping protocol buffers flatbuffers instead of protocol buffers Apr 28, 2022
@abrenoch
Copy link
Owner

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
Copy link
Author

flos06 commented Apr 29, 2022

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
Copy link
Owner

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

@flos06
Copy link
Author

flos06 commented Apr 29, 2022

Thanks for looking into it. There was a pull request here: awawa-dev/HyperHDR#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
Copy link
Author

flos06 commented Apr 30, 2022

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
Copy link
Owner

abrenoch commented Apr 30, 2022

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
Copy link
Author

flos06 commented Apr 30, 2022

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. ( awawa-dev/HyperHDR#263 ) He does make it seem as though swapping protobuf for flatbuffers would be enough unless I am misunderstanding him.

@flos06
Copy link
Author

flos06 commented May 11, 2022

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

@abrenoch
Copy link
Owner

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

@flos06
Copy link
Author

flos06 commented May 11, 2022

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
Copy link
Owner

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.

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

No branches or pull requests

2 participants