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

[Nikolay Sivov]

On 5/28/21 12:28 PM, Giovanni Mascellani wrote:
> 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.

I don't necessarily mean tests integrated in wine tests. Just something
that I can run to see what happens.

>
> 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.
>