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

[ts-sdk] Add offline processing #880

Merged
merged 2 commits into from
Dec 12, 2024

Conversation

wkozyra95
Copy link
Member

@wkozyra95 wkozyra95 commented Nov 22, 2024

Api changes

  • Add Slides/Slide components
  • Add Show component
  • Add the useAfterTimestamp hook (internally used by Show)
  • Add useCurrentTimestamp returns the current timestamp (does not trigger re-render for every time change)
    • Alternative 1: Add a hook that returns a function that returns a timestamp
    • Alternative 2: Add a hook that returns some context object that has a method that returns a timestamp
    • Alterantive 3: Do not expose it, but instead make useAfterTimestamp return the current timestamp (and maybe change name of that hook)
  • Add useBlockingTask hook
    • For live cases run the async function on mount. Hook returns result if promise resolved or undefined otherwise
    • For offline cases it blocks rendering until promise resolves and returns the result after.
  • Keep newBlockingTask function prrivate (internal implementation of useBlockingTask)
    • When it would be usfull as public API?
      • It could be useful if you want to implement something more advanced than use useBlockingTask, e.g. instead of running on mount it would rerun on specific props changes, or it should trigger after some PTS
    • Alternative 1: Make that function public and add some hook that returns TimeContext object. (newBlockingTask needs the context as argument)
    • Alternative 2: Add hook e.g. useBlockingTaskBuilder that returns a function to start a new blocking task.
  • Temporarily make public useTimeLimitedComponent.
    • That hook is intended to be private after we start using it in InputStream or Mp4 components. It is public for now, so we can write some examples and test internalls.
    • Alternative 1: Make that hook public so users can write their own components with a duration.

TODO in follow up

  • useInputStream will return correct states for offline processing based on offset and calculated duration.
function ExampleAppWithMultipleScreens() {
  return (
    <>
      <Show timestampMS={{ end: 3000 }}>
        <SomeComponent description="show from the beginning until 3 second" />
      </Show>
      <Show timestampMs={{ start: 3000, end: 6000 }}>
        <SomeComponent description="show between 3s and 6s />
      </Show>
      <Show delayMs={3000}>
        <SomeComponent description="3 seconds after mount" />
      </Show>
    </>
  );
}
function ExampleApp() {
  return (
    <View>
      <Slides>
        <Slide>
          <SomeComponent description="duration based on components inside Input" />
        </Slide>
        <Slide>
          <SomeComponent description="duration based on components inside Input" />
        </Slide>
        <Slide durationMs={3_000}>
          <SomeComponent  description="duration 3 seconds, ignore components" />
        </Slide>
      </Slides>
    </View>
  );
}
function ExampleAppWithBlockingTask() {
  const text = useBlockingTask(async function() {
     const response = await fetch("https://some/data")
     return await response.text()
  });
  return (
    <View>
      <Text>{text}</Text>
    </View>
  );
}

Currently, there are no components that support automatic slide change. I'm planning to add

  • Support for InputStream if it is mp4 to set it based on it's duration
  • New Mp4 component that is basically InputStream but does not require registration
  • (maybe) some fake component that does not render to anything but if it is inserted in scene it is forcing some duration

@wkozyra95 wkozyra95 linked an issue Nov 22, 2024 that may be closed by this pull request
@wkozyra95 wkozyra95 force-pushed the @wkozyra95/ts-sdk-offline-processing branch 10 times, most recently from 993b69e to 91a2cd5 Compare December 3, 2024 12:26
@wkozyra95 wkozyra95 force-pushed the @wkozyra95/ts-sdk-offline-processing branch from 91a2cd5 to 92478b7 Compare December 6, 2024 11:42
@wkozyra95 wkozyra95 marked this pull request as ready for review December 6, 2024 12:06
@wkozyra95 wkozyra95 requested review from BrtqKr and noituri December 6, 2024 12:06
@wkozyra95 wkozyra95 self-assigned this Dec 6, 2024
<Show delayMs={2000}>
<InputTile inputId="input_2" />
</Show>
<Show timestampMs={{ start: 5000, end: 8000 }}>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

either <Show timeRange={{ start: 5000, end: 8000 }}> or <Show timeRangeMs={{ start: 5000, end: 8000 }}>, but I prefer the first one - I'm not sure if that many libraries specify the unit in the props in this manner. You can usually safely assume that it's ms.

return (
<Tiles transition={{ durationMs: 200 }}>
<InputTile inputId="input_1" />
<Show delayMs={2000}>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd only remove the unit to unify it with the timestamp, if necessary


function ExampleApp() {
return (
<Tiles transition={{ durationMs: 200 }}>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd only remove the unit to unify it with the timestamp, if necessary


export type ShowProps = {
timestampMs?: { start?: number; end?: number };
delayMs?: number;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same with delay

Comment on lines +110 to +145
class UpdateTracker {
private promise: Promise<void> = new Promise(() => {});
private promiseRes: () => void = () => {};
private updateTimeout: number = -1;
private renderTimeout: number = -1;

constructor() {
this.promise = new Promise((res, _rej) => {
this.promiseRes = res;
});
}

public onUpdate() {
clearTimeout(this.updateTimeout);
this.updateTimeout = setTimeout(() => {
this.promiseRes();
}, MAX_NO_UPDATE_TIMEOUT_MS);
}

public async waitForRenderEnd(): Promise<void> {
this.promise = new Promise((res, _rej) => {
this.promiseRes = res;
});
clearTimeout(this.renderTimeout);
this.renderTimeout = setTimeout(() => {
console.warn(
"Render for a specific timestamp took too long, make sure you don't have infinite update loop."
);
this.promiseRes();
}, MAX_RENDER_TIMEOUT_MS);
await this.promise;
clearTimeout(this.renderTimeout);
clearTimeout(this.updateTimeout);
}
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if I understand the flow here.

  • waitForRenderEnd() waits for this.promiseRes() to be called. Which is called in 2 cases:
    • updateTimeout is executed first, or
    • renderTimeout is executed first

I don't see any situation where this.promiseRes is called outside of those timeouts. Is that how it's supposed to work?

Copy link
Member Author

@wkozyra95 wkozyra95 Dec 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this mechanism is similar in the idea to throttling. We want to send an update when we know that onUpdate was not called for 20ms. The second time limit (renderTimeout) is to just protect against some infinite loop

Copy link
Member

@noituri noituri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@wkozyra95 wkozyra95 force-pushed the @wkozyra95/ts-sdk-offline-processing branch from 92478b7 to 19706fd Compare December 12, 2024 10:11
@wkozyra95 wkozyra95 merged commit 0b74cc9 into master Dec 12, 2024
5 checks passed
@wkozyra95 wkozyra95 deleted the @wkozyra95/ts-sdk-offline-processing branch December 12, 2024 10:26
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.

[ts-sdk] Add offline processing support
3 participants