feat: Cmcdv2 batching#9
Conversation
…ayer into cmcdv2/batching
There was a problem hiding this comment.
Summary of Changes
Hello @cotid-qualabs, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
I've implemented a new feature to enable batching of Common Media Client Data (CMCD) reports. This change introduces a dedicated batching manager that collects CMCD data and sends it in aggregated requests, either based on a configured batch size or a time interval. This enhancement aims to reduce the number of individual network requests for CMCD data, thereby improving overall network efficiency and potentially reducing server load. The existing CMCD management system has been updated to leverage this new batching capability, and the serialization logic for CMCD data has been refactored into a separate utility for better organization.
Highlights
- New CMCD Batching Manager: I've introduced a new
CmcdBatchingManagerutility. This manager is responsible for collecting CMCD reports and sending them in batches, either when a certain number of reports are accumulated (batchSize) or after a specified time interval (batchTimer). It also includes robust retry mechanisms for rate-limited responses (HTTP 429) and handles 'gone' endpoints (HTTP 410). - Centralized CMCD Serialization: The core logic for serializing CMCD data into a query string or header value has been extracted from
CmcdManagerinto a new, dedicatedCmcdUtilsclass. This promotes better modularity and reusability of the serialization function. - Configurable CMCD Batching Options: The
shaka.extern.CmcdTargetexterns have been updated to exposebatchSizeandbatchTimerproperties. This allows developers to configure CMCD batching behavior directly through the player's configuration. - Integration with CMCD Manager: The existing
CmcdManagerhas been updated to integrate with the newCmcdBatchingManager. When batching is configured,CmcdManagerwill now hand off CMCD reports to the batching manager instead of sending them individually. All internal serialization calls withinCmcdManagernow correctly reference the newCmcdUtils. - Comprehensive Unit Tests: New unit tests have been added for the
CmcdBatchingManagerto ensure the reliability and correctness of the batching, retry, and error handling functionalities.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces CMCDv2 batching capabilities, allowing CMCD reports to be collected and sent in batches to a collection endpoint. This is managed by a new CmcdBatchingManager. The implementation includes handling for batching by size or by a timer, as well as a retry mechanism with exponential backoff for rate-limited requests. The CMCD serialization logic has been refactored into a new CmcdUtils class for better code organization. My review found an issue with the batch timer logic where the time unit conversion was incorrect, which could lead to unexpected batching behavior.
Reverting commited package lock
…ayer into cmcdv2/batching
…mcdv2/batching
…mcdv2/batching
Fixes shaka-project#8795 This change is introduced to prevent releasing underlying Stream structures while they might still be needed. `MetaSegmentIndex` does not release child SegmentIndexes anymore during its release process. Output Stream's `closeSegmentIndex()` method is now responsible for this. It's also aware about the rest of output streams and it will prevent releasing a particular child stream, if this is used by other output stream. This scenario can easily happen if DASH manifest have different number of renditions in various periods. Consider following scenario (vx are renditions): | Period 1 | Period 2 | Periods combined | | -------- | -------- | ---------------- | | v1_1 | v2_1 | v1 = v1_1, v2_1 | | v1_2 | -- | v2 = v1_2, v2_1 | v2_1 is shared rendition between 2 output streams. If first stream is used but we close 2nd one, we will release shared data and thus affect playback. This bug was probably introduced by shaka-project#7217
No description provided.