Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix buffering output delay #67

Merged

Conversation

Aerion
Copy link
Contributor

@Aerion Aerion commented Jul 2, 2022

Events always come with multiple lines to process.
In order to process it once per event, do the following:

  • Call output to display the changed state
  • Wait 100ms to discard all the lines of the event
  • Call output again to display the state (which could have changed if
    the user did an action within the 100ms window)

Before, we were always waiting 100ms after an event to display the
output. There was only one call to output, but late.

Without waiting for 100ms, we would be calling output too many times
for a single event.

Having two calls to output per 100ms is the best of the two solutions:
there is responsiveness, and the number of calls is limited.

Events always come with multiple lines to process.
In order to process it once per event, do the following:

- Call output to display the changed state
- Wait 100ms to discard all the lines of the event
- Call output again to display the state (which could have changed if
  the user did an action within the 100ms window)

Before, we were always waiting 100ms after an event to display the
output. There was only one call to output, but late.

Without waiting for 100ms, we would be calling output too many times
for a single event.

Having two calls to output per 100ms is the best of the two solutions:
 there is responsiveness, and the number of calls is limited.
@marioortizmanero marioortizmanero merged commit 166a778 into marioortizmanero:master Jul 2, 2022
@marioortizmanero
Copy link
Owner

marioortizmanero commented Jul 5, 2022

@Aerion, we currently have this:

        while read -r; do
            # Output the new state
            output

            # Read all stdin to flush unwanted pending events, i.e. if there are
            # 15 events at the same time (100ms window), output is only called
            # twice.
            read -r -d '' -t 0.1 -n 10000

            # After the 100ms waiting time, output again the state, as it may
            # have changed if the user did an action during the 100ms window.
            output
        done

Why not just this instead?

        while read -r -d '' -t 0.1 -n 10000; do
            output
        done

@Aerion
Copy link
Contributor Author

Aerion commented Jul 5, 2022

It may be erronous, but here is what I had in mind:

Having while read -r will enter the loop as soon as we have a new line from pactl, thus calling output as soon as possible.
Moving read -d -t 0.1 to the while condition would mean that output would be called in the worst case after 100ms (as -d ' ' would try to read every line).

It seems that the pactl events are not sent all at the same time, there may be some small delay between them (at least, when we still checked the client events). So my goal was to call output asap, discard the buffer after 100ms, and call output again.

Though in its current state, the downside is that if you do multiple actions, you'll only see the updates every 100ms.

@marioortizmanero
Copy link
Owner

marioortizmanero commented Jul 5, 2022

Ah yeah, makes sense, thanks for the re-explanation. Otherwise it will always hit the timeout and refresh the output, even when there aren't any new events.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants