Move Stream's DownloadingQueue into fetchers' SegmentQueue #1467
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
peaBerberian
added
proposal
peaBerberian
force-pushed
the
misc/segment-queue-fetcher2
branch
3 times, most recently
from
b0a289a to
fdadc57
Compare
peaBerberian
force-pushed
the
misc/segment-queue-fetcher2
branch
from
fdadc57 to
cbc263d
Compare
peaBerberian
force-pushed
the
misc/segment-queue-fetcher2
branch
from
cbc263d to
61d25b9
Compare
peaBerberian
added a commit
that referenced
this pull request
Aug 27, 2024
Based on #1467 We're planning to add CMCD (Common Media Client Data) v1 capabilities to our next release of the RxPlayer v4. This features adds multiple key-value pairs with most request (either from the query string, either from headers) to inform the server-side of the current playback condition from the player which made the request. We were previously missing two keys used for a specific use-case: `nor` (Next Object Request) and `nrr` (Next Range Request). Those two allow the player to anounce which segment it will probably load next so that the server could theoretically prefetch it in advance. Implementing those two means that our CMCD logic needs to be aware of the next segment that will probably be requested after a current one. This is made much simpler by #1467, as now our fetcher logic is aware of the whole queue of segments for a given type (audio/video etc.) in most cases. The main meat of this PR is just computing a relative URL, because that's what CMCD wants (in the name of DoS mitigation, I did not dive into why). I thought we already had that logic somewhere but if we have, I did not find it, so I re-coded and tested it.
peaBerberian
force-pushed
the
misc/segment-queue-fetcher2
branch
2 times, most recently
from
a42ea2c to
823a48b
Compare
peaBerberian
added a commit
that referenced
this pull request
Aug 27, 2024
Based on #1467 We're planning to add CMCD (Common Media Client Data) v1 capabilities to our next release of the RxPlayer v4. This features adds multiple key-value pairs with most request (either from the query string, either from headers) to inform the server-side of the current playback condition from the player which made the request. We were previously missing two keys used for a specific use-case: `nor` (Next Object Request) and `nrr` (Next Range Request). Those two allow the player to anounce which segment it will probably load next so that the server could theoretically prefetch it in advance. Implementing those two means that our CMCD logic needs to be aware of the next segment that will probably be requested after a current one. This is made much simpler by #1467, as now our fetcher logic is aware of the whole queue of segments for a given type (audio/video etc.) in most cases. The main meat of this PR is just computing a relative URL, because that's what CMCD wants (in the name of DoS mitigation, I did not dive into why). I thought we already had that logic somewhere but if we have, I did not find it, so I re-coded and tested it.
peaBerberian
added a commit
that referenced
this pull request
Aug 27, 2024
Based on #1467 We're planning to add CMCD (Common Media Client Data) v1 capabilities to our next release of the RxPlayer v4. This features adds multiple key-value pairs with most request (either from the query string, either from headers) to inform the server-side of the current playback condition from the player which made the request. We were previously missing two keys used for a specific use-case: `nor` (Next Object Request) and `nrr` (Next Range Request). Those two allow the player to anounce which segment it will probably load next so that the server could theoretically prefetch it in advance. Implementing those two means that our CMCD logic needs to be aware of the next segment that will probably be requested after a current one. This is made much simpler by #1467, as now our fetcher logic is aware of the whole queue of segments for a given type (audio/video etc.) in most cases. The main meat of this PR is just computing a relative URL, because that's what CMCD wants (in the name of DoS mitigation, I did not dive into why). I thought we already had that logic somewhere but if we have, I did not find it, so I re-coded and tested it.
peaBerberian
added a commit
that referenced
this pull request
Aug 27, 2024
Based on #1467 We're planning to add CMCD (Common Media Client Data) v1 capabilities to our next release of the RxPlayer v4. This features adds multiple key-value pairs with most request (either from the query string, either from headers) to inform the server-side of the current playback condition from the player which made the request. We were previously missing two keys used for a specific use-case: `nor` (Next Object Request) and `nrr` (Next Range Request). Those two allow the player to anounce which segment it will probably load next so that the server could theoretically prefetch it in advance. Implementing those two means that our CMCD logic needs to be aware of the next segment that will probably be requested after a current one. This is made much simpler by #1467, as now our fetcher logic is aware of the whole queue of segments for a given type (audio/video etc.) in most cases. The main meat of this PR is just computing a relative URL, because that's what CMCD wants (in the name of DoS mitigation, I did not dive into why). I thought we already had that logic somewhere but if we have, I did not find it, so I re-coded and tested it.
While working on CMCD (#1461), I saw an opportunity for doing a refactoring that I initially postponed: moving what was called the `DownloadingQueue` from the `RepresentationStream` to the `fetchers` code. The modules involved -------------------- The `DownloadingQueue` is a class allowing to put the current list of wanted segments for a given media type (audio, video, text) into a FIFO queue so they can be requested from the most urgent (usually the one coming right next in the corresponding media buffer) to the least urgent (usually the ones consecutive to that segment, in chronological order) - through a simple interface. The `DownloadingQueue` does both the queueing (the list of wanted segments is given to it as input) and the requesting (by relying on an instanciated `SegmentFetcher` - defined in the `fetchers` code) as well as some optimizations such as requesting the initialization segment at the same time than the initially-wanted media segment when possible. Previously, that `DownloadingQueue` was part of the `RepresentationStream` (in the `src/core/stream/representation` directory). The `RepresentationStream` is documented as the module whose role is to find the right segments to load, to orchestrate their requests and then to push them into the right `SegmentSink` (`src/core/segment_sinks`). In terms of responsibility and readability of the code it can make sense to put the `DownloadingQueue` in the `RepresentationStream`, but I always felt that it would be more at its place in the `fetchers` code for segments instead (in `src/core/fetchers/segment). The `fetchers` code for segments's role is to actually perform the segment requests, and do so in a transport-agnostic way (the transport-specific part being defined in `src/transports` which is directly used by the `fetchers` code). What I did here --------------- Basically, I moved the `DownloadingQueue` file into the `src/core/fetchers/segment` directory and renamed it `SegmentQueue` as it made more sense to me to remove the `Downloading` part from the name now that it is implied from its new location, and add the `Segment` part, which is sort of a convention in the `fetchers` to separate manifest-related code to segment-related code. I then made the `SegmentQueue` one of the main modules to be used by code external to the `fetchers` (here principally the `RepresentationStream`) instead of the `PrioritizedSegmentFetcher` like before. I also tried to simplify the whole `fetchers` API for segments so it can be straightforward to use from other modules without having to understand logic specific to that part. What's the relation with CMCD ----------------------------- As written at the start, this was done in relation with my work on CMCD, because that scheme can optionally provide to the CDN hints about what segment it is going to load next (with the previous segment's request). Previously, this would have meant an akward supplementary parameter somewhere asking for information on the next wanted segment for what's in the end very specific. By instead updating the `fetchers` (which is the module directly exploiting CMCD) so its API is already aware of the current queue of wanted segments (and not just the most urgent one like before), implementing "next segment hints" can be done without having to involve any other module.
peaBerberian
force-pushed
the
misc/segment-queue-fetcher2
branch
from
823a48b to
2733245
Compare
Florent-Bouisset
approved these changes
Sep 3, 2024
peaBerberian
added a commit
that referenced
this pull request
Sep 3, 2024
Based on #1467 We're planning to add CMCD (Common Media Client Data) v1 capabilities to our next release of the RxPlayer v4. This features adds multiple key-value pairs with most request (either from the query string, either from headers) to inform the server-side of the current playback condition from the player which made the request. We were previously missing two keys used for a specific use-case: `nor` (Next Object Request) and `nrr` (Next Range Request). Those two allow the player to anounce which segment it will probably load next so that the server could theoretically prefetch it in advance. Implementing those two means that our CMCD logic needs to be aware of the next segment that will probably be requested after a current one. This is made much simpler by #1467, as now our fetcher logic is aware of the whole queue of segments for a given type (audio/video etc.) in most cases. The main meat of this PR is just computing a relative URL, because that's what CMCD wants (in the name of DoS mitigation, I did not dive into why). I thought we already had that logic somewhere but if we have, I did not find it, so I re-coded and tested it.
peaBerberian
added a commit
that referenced
this pull request
Sep 4, 2024
Based on #1467 We're planning to add CMCD (Common Media Client Data) v1 capabilities to our next release of the RxPlayer v4. This features adds multiple key-value pairs with most request (either from the query string, either from headers) to inform the server-side of the current playback condition from the player which made the request. We were previously missing two keys used for a specific use-case: `nor` (Next Object Request) and `nrr` (Next Range Request). Those two allow the player to anounce which segment it will probably load next so that the server could theoretically prefetch it in advance. Implementing those two means that our CMCD logic needs to be aware of the next segment that will probably be requested after a current one. This is made much simpler by #1467, as now our fetcher logic is aware of the whole queue of segments for a given type (audio/video etc.) in most cases. The main meat of this PR is just computing a relative URL, because that's what CMCD wants (in the name of DoS mitigation, I did not dive into why). I thought we already had that logic somewhere but if we have, I did not find it, so I re-coded and tested it.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While working on CMCD (#1461), I saw an opportunity for doing a refactoring that I initially postponed: moving what was called the
DownloadingQueuefrom theRepresentationStreamto thefetcherscode.The modules involved
The
DownloadingQueueis a class allowing to put the current list of wanted segments for a given media type (audio, video, text) into a FIFO queue so they can be requested from the most urgent (usually the one coming right next in the corresponding media buffer) to the least urgent (usually the ones consecutive to that segment, in chronological order) - through a simple interface.The
DownloadingQueuedoes both the queueing (the list of wanted segments is given to it as input) and the requesting (by relying on an instanciatedSegmentFetcher- defined in thefetcherscode) as well as some optimizations such as requesting the initialization segment at the same time than the initially-wanted media segment when possible.Previously, that
DownloadingQueuewas part of theRepresentationStream(in thesrc/core/stream/representationdirectory).The
RepresentationStreamis documented as the module whose role is to find the right segments to load, to orchestrate their requests and then to push them into the rightSegmentSink(src/core/segment_sinks).In terms of responsibility and readability of the code it can make sense to put the
DownloadingQueuein theRepresentationStream, but I always felt that it would be more at its place in thefetcherscode for segments instead (in `src/core/fetchers/segment).The
fetcherscode for segments's role is to actually perform the segment requests, and do so in a transport-agnostic way (the transport-specific part being defined insrc/transportswhich is directly used by thefetcherscode).What I did here
Basically, I moved the
DownloadingQueuefile into thesrc/core/fetchers/segmentdirectory and renamed itSegmentQueueas it made more sense to me to remove theDownloadingpart from the name now that it is implied from its new location, and add theSegmentpart, which is sort of a convention in thefetchersto separate manifest-related code to segment-related code.I then made the
SegmentQueueone of the main modules to be used by code external to thefetchers(here principally theRepresentationStream) instead of thePrioritizedSegmentFetcherlike before.I also tried to simplify the whole
fetchersAPI for segments so it can be straightforward to use from other modules without having to understand logic specific to that part.What's the relation with CMCD
As written at the start, this was done in relation with my work on CMCD, because that scheme can optionally provide to the CDN hints about what segment it is going to load next (with the previous segment's request).
Previously, this would have meant an akward supplementary parameter somewhere asking for information on the next wanted segment for what's in the end very specific.
By instead updating the
fetchers(which is the module directly exploiting CMCD) so its API is already aware of the current queue of wanted segments (and not just the most urgent one like before), implementing "next segment hints" can be done without having to involve any other module.