Mir

Code review comment for lp:~raof/mir/1hz-rendering-always

Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

A first pass:

88 +class Alarm
124 +class Timer

Delete CopyAssign.

401 + std::unique_lock<decltype(data->m)> lock(data->m);
412 + std::unique_lock<decltype(data->m)> lock(data->m);

We could use the slightly more efficient lock_guard<> since we don't use any of unique_lock's extra capabilities.

523 + the_main_loop());

Do we want to use the same MainLoop object for both the buffer drop timeouts and the display server main loop? Although I don't see a particular functional problem with doing so, it indicates an intention to serialize events from both sources, which is not really true.

On a related note, perhaps we need a better abstraction for this, one that hides all the timing details from our buffer mechanism, e.g.:

class FrameDroppingPolicy
{
    virtual std::unique_ptr<Alarm> schedule_frame_drop(std::function<void()> do_frame_drop) = 0;
};

SwitchingBundle(..., std::shared_ptr<FrameDroppingPolicy>, ...)

Needs Fixing (Needs discussion about the abstraction part)

review: Needs Fixing

« Back to merge proposal