Handler: allow spatial layers from 0 to N#149
Conversation
| bool paused{ false }; | ||
| // Video Max spatial layer. | ||
| uint8_t maxSpatialLayer{ 0 }; | ||
| uint8_t maxSpatialLayer{ (uint8_t) -1 }; |
There was a problem hiding this comment.
This basically means 255. It is (or was) a common way to initialize an unsigned number to its maximum value to mainly indicate it's not initialized.
Previsouly it was 0 and the allowed values were 1, 2 and 3, so this check https://github.com/versatica/libmediasoupclient/blob/v3/src/Producer.cpp#L204 would pass. Now, since we allow 0 as a value, the check would not pass if we set spatialLayer to 0. Another option would be to have a boolean to indicate SetMaxSpatialLayer has been called a first time, but not sure if that's a good option. Or make it a int8_t and don't do this weird cast.
Right now, with 255 (I doubt anyone has 255 layers) we indicate that is not initialized, like in the unit test that I updated.
I'm happy to update the PR with any suggestions. Thanks!
There was a problem hiding this comment.
This change should not be needed. It just represents the maximum spatial layer which at least is 0.
There was a problem hiding this comment.
Thank you @jmillan. So, if we leave it to 0 and call Producer::setMaxSpatialLayer(0) it seems that the Producer.cpp line I was pointing at above, it would actually not enable the 0 spatialLayer (because maxSpatialLayer is already initialized to 0):
if (spatialLayer == this->maxSpatialLayer)
return;
Before, since values were 1, 2 and 3, it will always go through (the first time). Maybe removing that if condition?
|
Why is this closed? We do want to address all pending PRs and issues, it's just that we are very busy. |
|
Hola! Oh, shoot! I completely forgot I had this PR open. Argh... let me fix that. |
|
Sorry again. Moved to #174. |
|
Thanks a lot |
3 participants
This fixes an incompatibility with mediasoup server which in v3 spatial layers go from 0 to N, however libmediasoupclient is still using v2 spatial layers.