-
-
Notifications
You must be signed in to change notification settings - Fork 54
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
dash: don't use profiles to determine if a video is a live stream #1231
Conversation
WalkthroughThe update to Changes
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- server/services/dash.ts (1 hunks)
- server/tests/unit/services/dash.spec.ts (1 hunks)
Additional comments: 2
server/tests/unit/services/dash.spec.ts (1)
- 30-31: The addition of
mediaPresentationDuration
to themockMPDData
andmockManifest
is consistent with the changes in theDashVideoAdapter
to use this attribute for determining if a video is a live stream. This ensures that the unit tests will accurately test the new functionality.server/services/dash.ts (1)
- 68-69: The logic change to check for
mediaPresentationDuration
instead of profiles to determine if a video is a live stream aligns with the PR objectives. This should correctly handle the case where VOD streams are marked with a live profile. Ensure that this change is thoroughly tested, especially for edge cases where themediaPresentationDuration
might be missing or incorrectly formatted.Verification successful
The
parseIso8601Duration
function inserver/services/parsing/iso8601.ts
includes error handling for malformed inputs. If the input string does not match the expected ISO 8601 duration format, the function throws an error with the messageFailed to parse duration: ${duration}
. This indicates that the function is designed to handle cases where themediaPresentationDuration
might be missing or incorrectly formatted by throwing an error, which should be caught and handled appropriately where the function is called.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that all cases where mediaPresentationDuration might be missing or malformed are handled. rg --type js 'mediaPresentationDuration' --context 5Length of output: 52
Script:
#!/bin/bash # Let's try to find the file by name first to ensure it exists and is indexed. fd 'dash.ts' # If the file is found, we will then search for 'mediaPresentationDuration' within that file. # We will use 'rg' without specifying the file type to avoid any issues with file type detection. fd 'dash.ts' --exec rg 'mediaPresentationDuration' --context 5Length of output: 501
Script:
#!/bin/bash # Let's find the definition and usage of parseIso8601Duration to see how it handles malformed inputs. rg 'parseIso8601Duration' --context 5Length of output: 5914
Script:
#!/bin/bash # Examine the error handling within the parseIso8601Duration function to see how it deals with malformed inputs. rg 'parseIso8601Duration' server/services/parsing/iso8601.ts -A 10 -B 2Length of output: 370
Script:
#!/bin/bash # The previous output was incomplete. Let's get the full body of the parseIso8601Duration function to see the complete error handling. rg 'parseIso8601Duration' server/services/parsing/iso8601.ts -A 20 -B 2Length of output: 712
Passing run #831 ↗︎
Details:
Review all test suite changes for PR #1231 ↗︎ |
closes #1224