shaka-project / shaka-player-embedded

Shaka Player in a C++ Framework
Apache License 2.0
239 stars 62 forks source link

Fix temporary ByteBuffer& constructor issue on some platforms #85

Closed nitro404 closed 4 years ago

TheModMaker commented 4 years ago

Can you explain more about what this is fixing? The ByteBuffer values are being moved around, which is the desired behavior. Also, it is usually against the Google style guide to use non-const references; it is usually better to use a pointer for mutable references. Why is this just for OnEncrypted and not for segment appends?

roman380 commented 4 years ago

The problem with OnEncrypted is that the handler takes ByteBuffer argument and there is an std::bind to such handler. It is the only case throughout the project of the kind. This combination does not pick move constructor on some platforms and hence the workaround.

nitro404 commented 4 years ago

If this change is not preferred, feel free to close the request. It is not beneficial for the currently targeted platforms, but could resolve issues in the future if other platforms are expanded to.

TheModMaker commented 4 years ago

I'm going to close this. I'm changing how the data is passed around for the encrypted events as part of #60, so this shouldn't be an issue for long. I'll try to avoid std::bind with ByteBuffer so it should work on your platform too.