-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
streams.dash: Support manifest strings in addition to manifest urls #2285
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
Conversation
Addresses streamlink#2179. Allows parse_manifest() to accept a string containing a properly-formatted XML manifest instead of a manifest url.
Codecov Report
@@ Coverage Diff @@
## master #2285 +/- ##
==========================================
- Coverage 52.61% 52.57% -0.05%
==========================================
Files 237 237
Lines 14758 14815 +57
==========================================
+ Hits 7765 7789 +24
- Misses 6993 7026 +33 |
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.
if url_or_manifest is used and there is drm protection in the string
streamlink/src/streamlink/stream/dash.py
Line 192 in 054fb23
raise PluginError("{} is protected by DRM".format(url)) |
there will be an NameError: name 'url' is not defined
maybe not that important as this is not supported anyways
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.
Looks good, but I think the test could cover more :)
tests/streams/test_dash.py
Outdated
@@ -208,6 +208,26 @@ def test_parse_manifest_drm(self, mpdClass): | |||
self.session, self.test_url) | |||
mpdClass.assert_called_with(ANY, base_url="http://test.bar", url="http://test.bar/foo.mpd") | |||
|
|||
@patch('streamlink.stream.dash.MPD') |
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.
I don't think this test is quite valid, it doesn't test that the correct thing is passed to the MPD class. I think parsing one of the included resources might be a better test.
def test_parse_manifest_string(self):
with text("dash/test_9.mpd") as mpd_txt:
test_manifest = mpd_txt.read()
streams = DASHStream.parse_manifest(self.session, test_manifest)
self.assertSequenceEqual(
sorted(list(streams.keys())),
sorted(["2500k"])
)
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.
fixed in 6aea0f4
Thanks @pmrowla, and thanks for the reviews @back-to and @beardypig! |
…treamlink#2285) * stream.dash: Support manifest strings in addition to manifest urls Addresses streamlink#2179. Allows parse_manifest() to accept a string containing a properly-formatted XML manifest instead of a manifest url. * streams.dash: add unit test for manifest string * streams.dash: fix parse_manifest_string unit test
…treamlink#2285) * stream.dash: Support manifest strings in addition to manifest urls Addresses streamlink#2179. Allows parse_manifest() to accept a string containing a properly-formatted XML manifest instead of a manifest url. * streams.dash: add unit test for manifest string * streams.dash: fix parse_manifest_string unit test
…treamlink#2285) * stream.dash: Support manifest strings in addition to manifest urls Addresses streamlink#2179. Allows parse_manifest() to accept a string containing a properly-formatted XML manifest instead of a manifest url. * streams.dash: add unit test for manifest string * streams.dash: fix parse_manifest_string unit test
…treamlink#2285) * stream.dash: Support manifest strings in addition to manifest urls Addresses streamlink#2179. Allows parse_manifest() to accept a string containing a properly-formatted XML manifest instead of a manifest url. * streams.dash: add unit test for manifest string * streams.dash: fix parse_manifest_string unit test
Modifies
parse_manifest()
to accept aurl_or_manifest
parameter.See also: #2239
This does not include any of the changes for supporting DASH SegmentBase.