Skip to content

[Proposal] Add declareInitSegment method to the SegmentBuffer abstraction#1216

Merged
peaBerberian merged 7 commits intonextfrom
misc/declareInitSegment
Jun 15, 2023
Merged

[Proposal] Add declareInitSegment method to the SegmentBuffer abstraction#1216
peaBerberian merged 7 commits intonextfrom
misc/declareInitSegment

Conversation

@peaBerberian
Copy link
Collaborator

@peaBerberian peaBerberian commented Feb 8, 2023

This commit adds the declareInitSegment and freeInitSegment methods to the RxPlayer's SegmentBuffer abstractions (which is the part of the code handling the operations on SourceBuffers, such as pushing media and init segments).

The short term idea is to improve the handling of initialization segments in the SegmentBuffer.
Until now, each pushed media segment lead to a check that the initialization segment it relies on is the same than the last one pushed. To be able to perform that check, a caller need to communicate again the initialization segment's data each time a chunk is pushed to the buffer.

This check can be performed efficiently in most cases because we first check init segment's data equality by reference, which, by pure luck, should be equal in most cases in the current code. In cases where it isn't the same reference however, it can lead to a byte-per-byte check, which should not be an issue in terms of performance in most cases, but is still an ugly specificity which could be handled in a more optimal and understandable way.

This commit now allows the definition of a initSegmentUniqueId, an identifier for initialization segments, on SegmentBuffers. Any pushed segments can then refer to its associated init segment by indicating which initSegmentUniqueId it is linked to.

The SegmentBuffer will ensure behind the hood that the right initialization segment is pushed before pushing media segments, like before, excepted that this can now be done just by comparing this initSegmentUniqueId - it also means that the caller is no more required to keep in memory the data of the loaded initialization segment, the SegmentBuffer is already doing that.
Previously, the initialization segment's data was kept by the RepresentationStream, the abstraction choosing which segments to load (which is part of the reasons why the reference mostly never changed).

The declaration and "freeing" of init segment is done through a declareInitSegment/freeInitSegment pair of methods on a SegmentBuffer. This sadly means that memory freeing for the initialization segment is now manual, whereas we just relied on garbage collection when the initialization segment was directly used.
It isn't a little disadvantage, which is why it is kept as a proposal for now.


Though mostly, the long term benefit is to implement the hybrid-worker mode that we plan to have in the future, where buffering is performed in a WebWorker (thus improving concurrence with an application, with the goal of preventing both UI stuttering due to heavy player tasks and rebuffering due to heavy UI tasks).

In the currently-planned long term worker features we would have thus the following modes:

  • full worker: where both the rebuffering logic and MSE API are called in a WebWorker, allowing to avoid UI and media playback blocking each other to some extent

    This however requires the MSE-in-Worker feature to be available in the browser AND it also implies a more complex API, notably some callbacks (manifestLoader, segmentLoader and representationFilter) which will have to be updated.

  • hybrid mode: The buffering logic is mainly performed in a WebWorker but MSE API are still in the main thread. This allows e.g. to not fight for CPU with the UI when the RxPlayer chooses which segments to request (and request them) and to avoid blocking the UI when the Manifest is being parsed.

    Though the UI blocking could still mean that a loaded segment is waiting to be pushed in that mode.

    Because here MSE APIs may have to be called through postMessage-style message passing, the previous logic of communicating each time the same initialization segment each time a segment was pushed, with no mean to just move that data (in JavaScript linguo, to "transfer" it) was considerably worst than before.
    Relying on a short identifier instead seems a better solution here.

  • normal mode: The current mode where everything stays in main thread.

However it should be noted that all of this long term objective is still in an higly experimental phase, and the gains are only theoretical for now.

@peaBerberian peaBerberian added proposal This Pull Request or Issue is only a proposal for a change with the expectation of a debate on it Priority: 3 (Low) This issue or PR has a low priority. skip-performance-checks Performance tests are not run on this PR labels Feb 8, 2023
@peaBerberian peaBerberian force-pushed the misc/declareInitSegment branch 2 times, most recently from bd38221 to 2584cd3 Compare February 8, 2023 17:02
@peaBerberian peaBerberian force-pushed the misc/declareInitSegment branch from 2584cd3 to 3c70636 Compare February 9, 2023 15:00
@peaBerberian peaBerberian added skip-performance-checks Performance tests are not run on this PR and removed skip-performance-checks Performance tests are not run on this PR labels Mar 8, 2023
This commit adds the `declareInitSegment` and `freeInitSegment` methods
to the RxPlayer's SegmentBuffer abstractions (which is the part of the
code handling the operations on SourceBuffers, such as pushing media and
init segments).

The short term idea is to improve the handling of initialization
segments in the SegmentBuffer.
Until now, each pushed media segment lead to a check that the
initialization segment it relies on is the same than the last one
pushed. To be able to perform that check, a caller need to communicate
again the initialization segment's data each time a chunk is pushed to
the buffer.

This check can be performed efficiently in most cases because we first
check init segment's data equality by reference, which, by pure luck,
should be equal in most cases in the current code. In cases where it
isn't the same reference however, it can lead to a byte-per-byte check,
which should not be an issue in terms of performance in most cases, but
is still an ugly specificity which could be handled in a more optimal
and understandable way.

This commit now allows the definition of a `initSegmentUniqueId`, an
identifier for initialization segments, on SegmentBuffers.
Any pushed segments can then refer to its associated init segment by
indicating which `initSegmentUniqueId` it is linked to.

The SegmentBuffer will ensure behind the hood that the right
initialization segment is pushed before pushing media segments, like
before, excepted that this can now be done just by comparing this
`initSegmentUniqueId` - it also means that the caller is no more
required to keep in memory the data of the loaded initialization
segment, the `SegmentBuffer` is already doing that.
Previously, the initialization segment's data was kept by the
`RepresentationStream`, the abstraction choosing which segments
to load (which is part of the reasons why the reference mostly never
changed).

The declaration and "freeing" of init segment is done through a
`declareInitSegment`/`freeInitSegment` pair of methods on a
`SegmentBuffer`. This sadly means that memory freeing for the
initialization segment is now manual, whereas we just relied on garbage
collection when the initialization segment was directly used.

---

Though mostly, the long term benefit is to implement the hybrid-worker
mode that we plan to have in the future, where buffering is performed in
a WebWorker (thus improving concurrence with an application, with the
goal of preventing both UI stuttering due to heavy player tasks and
rebuffering due to heavy UI tasks).

In the currently-planned long term worker features we would have thus
the following modes:

  - full worker: where both the rebuffering logic and MSE API are called
    in a WebWorker, allowing to avoid UI and media playback blocking
    each other to some extent

    This however requires the
    [MSE-in-Worker](w3c/media-source#175)
    feature to be available in the browser AND it also implies a more
    complex API, notably some callbacks (`manifestLoader`,
    `segmentLoader` and `representationFilter`) which will have to be
    updated.

  - hybrid mode: The buffering logic is mainly performed in a WebWorker
    but MSE API are still in the main thread. This allows e.g. to not
    fight for CPU with the UI to know which segments to download and to
    avoid blocking the UI when the Manifest is being parsed.

    Though the UI blocking could still mean that a loaded segment is
    waiting to be pushed in that mode.

    Because here MSE APIs may have to be called through `postMessage`-style
    message passing, the previous logic of communicating each time the
    same initialization segment each time a segment was pushed, with no
    mean to just move that data (in JavaScript linguo, to "transfer" it)
    was considerably worst than before.
    Relying on a short identifier instead seems a better solution here.

  - normal mode: The current mode where everything stays in main thread.

However it should be noted that all of this long term objective is still
in an higly experimental phase, and the gains are only theoretical for
now.
@peaBerberian peaBerberian force-pushed the misc/declareInitSegment branch from 3c70636 to 81ac894 Compare May 17, 2023 16:25
@peaBerberian peaBerberian merged commit 40068f9 into next Jun 15, 2023
peaBerberian added a commit that referenced this pull request Jun 27, 2023
[Proposal] Add `declareInitSegment` method to the SegmentBuffer abstraction
peaBerberian added a commit that referenced this pull request Jul 4, 2023
[Proposal] Add `declareInitSegment` method to the SegmentBuffer abstraction
@peaBerberian peaBerberian deleted the misc/declareInitSegment branch July 6, 2023 12:02
peaBerberian added a commit that referenced this pull request Jul 18, 2023
[Proposal] Add `declareInitSegment` method to the SegmentBuffer abstraction
peaBerberian added a commit that referenced this pull request Aug 7, 2023
[Proposal] Add `declareInitSegment` method to the SegmentBuffer abstraction
peaBerberian added a commit that referenced this pull request Aug 22, 2023
[Proposal] Add `declareInitSegment` method to the SegmentBuffer abstraction
peaBerberian added a commit that referenced this pull request Aug 23, 2023
[Proposal] Add `declareInitSegment` method to the SegmentBuffer abstraction
peaBerberian added a commit that referenced this pull request Aug 31, 2023
[Proposal] Add `declareInitSegment` method to the SegmentBuffer abstraction
peaBerberian added a commit that referenced this pull request Aug 31, 2023
[Proposal] Add `declareInitSegment` method to the SegmentBuffer abstraction
peaBerberian added a commit that referenced this pull request Sep 15, 2023
[Proposal] Add `declareInitSegment` method to the SegmentBuffer abstraction
peaBerberian added a commit that referenced this pull request Sep 22, 2023
[Proposal] Add `declareInitSegment` method to the SegmentBuffer abstraction
peaBerberian added a commit that referenced this pull request Sep 22, 2023
[Proposal] Add `declareInitSegment` method to the SegmentBuffer abstraction
peaBerberian added a commit that referenced this pull request Sep 25, 2023
[Proposal] Add `declareInitSegment` method to the SegmentBuffer abstraction
peaBerberian added a commit that referenced this pull request Sep 26, 2023
[Proposal] Add `declareInitSegment` method to the SegmentBuffer abstraction
peaBerberian added a commit that referenced this pull request Sep 27, 2023
[Proposal] Add `declareInitSegment` method to the SegmentBuffer abstraction
peaBerberian added a commit that referenced this pull request Oct 13, 2023
[Proposal] Add `declareInitSegment` method to the SegmentBuffer abstraction
peaBerberian added a commit that referenced this pull request Oct 13, 2023
[Proposal] Add `declareInitSegment` method to the SegmentBuffer abstraction
peaBerberian added a commit that referenced this pull request Oct 13, 2023
[Proposal] Add `declareInitSegment` method to the SegmentBuffer abstraction
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Priority: 3 (Low) This issue or PR has a low priority. proposal This Pull Request or Issue is only a proposal for a change with the expectation of a debate on it skip-performance-checks Performance tests are not run on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant