input: remove seek postpone

This partially reverts 3d449085

There are no reasons (anymore) to wait for the decoder to finish its bufferisation before seeking (and there is no reason that 125ms is enough to wait for the bufferisation to finish).

Cc. @typx @fcartegnie @Courmisch, correct me if I'm wrong ^^

changed milestone to %4.0

added Component::Core: Input label

added MRStatus::Reviewable label

As long as there is an input control queue with input control reduction it somehow makes sense to give it the opportunity to compress the seek requests more instead of flushing/seeking/flushing/seeking/etc. in case of consecutive seeks especially if the seek is slow or CPU/network intensive.

Edit: The risk with remote contents is that numerous seeks opening and closing connections could be assimilated server-side as a denial of service attempt and end up blacklisting the client IP.

As long as there is an input control queue with input control reduction it somehow makes sense to give it the opportunity to compress the seek requests more instead of flushing/seeking/flushing/seeking/etc. in case of consecutive seeks especially if the seek is slow or CPU/network intensive.

Note that seek requests are reduced: if the user seeks 5 times in a row, the input will process the first seek request and the last one (skipping the ones in the middle).

Edit: The risk with remote contents is that numerous seeks opening and closing connections could be assimilated server-side as a denial of service attempt and end up blacklisting the client IP.

But this was not the initial reason of this commit.

Note that I can't see any difference with various content with or without this MR when spamming seek requests or seeking with 'start-time'.

All in all it's a "race" between the queuer and the input thread that you can't really reliably reproduce.

If several seek request are queued and not popped they are indeed reduced by ControlGetReducedIndexLocked so sure it's not an issue usually.

The issue I'm describing happens if the controls are popped faster than they are queued. If for example pf_demux is very fast then we end up seeking a lot with the side effects described above (which becomes more likely now that everything is fast).

3d449085 was ensuring compression of all the consecutive seek requests received in 125 ms (or less if the buffering was very fast). It only affects consecutive seek requests while not hindering single seeks since it's likely that we are not buffering.

Now that I've said that, that's not a hill that I would defend to the death if more people think that it's not needed anymore.

Now that I've said that, that's not a hill that I would defend to the death if more people think that it's not needed anymore.

Same, I can live with or without this MR, but I don't really like keeping historical hacks that we don't really understand. OK, as you said, it might avoid spamming a server (but I'm not sure if it was the first intent).

@Courmisch what do you think ?

Personnally, I prefer that we first understand the "hacks" and the problem space it was supposed to stem from before removing.

Well I'm not in Laurent's head but I'm pretty sure that the goal was to force jump request compression when people were holding a jump hotkey because seek can be expensive especially in remote cases.

The postpone only happens if we are buffering (so we just seeked) and limited to 125 ms to make sure that a seek occcurs at least every 125 ms. It doesn't hinder a simple seek aka most cases (unless we seek while we are buffering but it will at most delay it by an unperceptible delay). Using both the buffering and a fixed delay allow to wait even less if the access is very fast so the seek is very likely cheap.

Side note: this was designed at a time people were using HDDs and not SSDs, with slower seek times. And it's way worse when playing from a real DVD/CD.

Well people are still using gift USB keys with terrible performances. Also range HTTP requests are not free especially if they come with crypto handshakes :)

If seeking is slow then it shouldn't much matter. The second seek will be overridden by the third one while the first one is still pending.

AFAIU, Laurent's concern had more to do with computationally expensive or I/O-heavy seeking than slow.

So long as locking doesn't favour the input thread over the interface thread, this should work. But this might require a queued lock (which in turns requires CV with queued locks).

mentioned in merge request !4934 (merged)

added 1 commit

• 8b04278a - input: remove seek postpone

Compare with previous version

added MRStatus::InReview label and removed MRStatus::Reviewable label

added MRStatus::NotCompliant label and removed MRStatus::InReview label