webos: Cleanup after #23092

[sundermann]

Description

Addresses the review feedback from @lrusak in #23092

Motivation and context

Citing @lrusak review comment here:

It seems logging could use some uniformity (this is lacking elsewhere in the codebase but for new code it would be nice if the log style was consistent).

I'm now using LogF everywhere to make logging more consistent

There seems to be an overuse (abuse) of auto, see the code guidelines for help here -> https://github.com/xbmc/xbmc/blob/master/docs/CODE_GUIDELINES.md#124-auto

I've removed auto in a couple of places to make typing more explicit

The use of json is interesting and I would recommend looking into a way to set the json structures in a more readable manner (using a json template possibly and just setting the specific key/values through helpers (future TODO).

I think this is something we can improve on in webos-userland

There seems to be a lot of ifdeffing being added. I hope in the future we can find better ways to abstract this otherwise it's going to become a plague.

Probably once we get some integration with the luna bus to query the webOS version at runtime properly.

Take a look at using std::map (or utils/Map.h) for 1:1 mapping. This IMO improve readability and keeps definitions and maps together.

I'm using constexpr maps all over now

The API uses a lot of strings. It might be worth adding all the required strings as constant defines to group them together for readability.

Probably also something that can be added in webos-userland. I was thinking about implementing a helper function to set json values like so very.nested.json.object. Those strings could then be defined in the headers.

Please avoid adding dead code. You can always factor that in later. It's easier to change new code rather than changing it later.

I've removed the whole PCM stuff from AESinkStarfis and also the buffer limits from the video codec.

Our API's aren't quite setup yet to leverage std::chrono but I would recommend using it when dealing with time and changing units. It increases readability and avoids the need for macros to change units.

We now use std::chrono everywhere which is actually a very reasonable change because time units in DVDPlayer are not very consistent (use of micro, milli and second units)

How has this been tested?

LG OLED77C28 (webOS 7)

What is the effect on users?

Screenshots (if appropriate):

Types of change

• Bug fix (non-breaking change which fixes an issue)
• Clean up (non-breaking change which removes non-working, unmaintained functionality)
• Improvement (non-breaking change which improves existing functionality)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that will cause existing functionality to change)
• Cosmetic change (non-breaking change that doesn't touch code)
• None of the above (please explain below)

Checklist:

• My code follows the Code Guidelines of this project
• My change requires a change to the documentation, either Doxygen or wiki
• I have updated the documentation accordingly
• I have read the Contributing document
• I have added tests to cover my change
• All new and existing tests passed

[yol]

Busy with infrastructure unfortunately, will have to excuse myself

@lrusak please take good care of the wayland code ;D

[csdougliss]

@lrusak Bump 👍

[lrusak]

Nice cleanup. I left a couple comments about some minors. In the future it would also be nice to use separate commits instead of one large commit.

[sundermann]

Thanks for the review feedback. I have applied the requested changes

[sundermann]

@lrusak anything else missing?

[lrusak]

Seems ok. If you want to do the change to use using namespace std::chrono_literals; that would be ok.

It would also be nice if you can push fixups (git commit --fixup=<hash>) when making review changes as it helps to show what was actually changed instead of just force pushing all the changes. You can then git rebase --autosquash after approval to squash in all the fixups.

[sundermann]

Thanks, I have added usage of std::chrono_literals again.

[csdougliss]

@lrusak @wsnipex can this be merged?

[lrusak]

Look fine, please squash.

[sundermann]

Thanks. Commits are now squashed