[PATCH v2 4/4] mf: Fix computing fastest and slowest rates for the media session.

[Giovanni Mascellani]

Hi,

Il 28/05/21 10:37, Nikolay Sivov ha scritto:
> These two changes (or three) changes are separate. Second changes
> default even when none of components support rate change. How did you
> test this?

I don't understand why they should be separate. This piece of code is 
basically computing the minimum or the maximum between a bunch of values.

There were two bugs: first one is that 0.0 is a valid initialization 
value when you're computing a maximum, but not when you are computing a 
minimum (if you iteratively compute min() starting from 0.0, the result 
will always be 0.0).

Second bug is that you have compute the maximum and minimum in absolute 
value. For example, if a session has a source with fastest reverse rate 
-100.0 and another with -20.0, the global fastest should be the minimum 
in absolute value, which is -20.0, and not -100.0.

My patch fixes two bugs, but they are in the same algorithm, so it felt 
right to have them together.

As for the tests, again, it's never clear to me when it's appropriate to 
have tests and when not. I would be inclined to always add tests, the 
more the better, but given other reviews I received, this is not 
necessarily true. I can add tests, but they will require a few mock 
classes in order not to be trivial. Let me know if this is welcome or not.

Lacking tests, I fixed this algorithm based on what it could logically 
do. Since the point is finding the set of rates that are acceptable for 
all the components in the session, and that reverse rates are 
represented with negative numbers, it felt right to implement the 
algorithm that I described above (compute the maximum absolute value 
when dealing with slowest rates and the minimum absolute value when 
dealing with fastest rates). Also, this new algorithm makes Deep Rock 
Galactic working, which it didn't before.

Giovanni.