-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
add basic HLS fMP4 support #3403
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
| total_duration = 0 | ||
| while not self.closed: | ||
| for sequence in filter(self.valid_sequence, self.playlist_sequences): | ||
| if self.init_map != sequence.segment.map: |
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 think a bit of a comment describing the mechanism might be useful here.
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.
The map segment has to be put in the queue only once, and is then valid for all following segments until an ext_x_discontinuity tag arrives (in this case, a possibly new map segment follows) or the stream ends.
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 want to save the init maps in a separate list and not in each Segment as before, because I want only the index of the init. map store, so less memory is used, especially with very long playlist of vod’s - do you have any objections, @beardypig, @bastimeyer?
store init maps in a separate list
bastimeyer
left a comment
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.
HLSStreamWorker.iter_segments() should not create new segments and yield them with the same num. This is bad, because first, it's not the job of the worker to create segments, it's the job of the parser, and second, you're re-downloading the same media initialization over and over for each segment that comes after the EXT-X-MAP tag, which is wasteful.
Also not sure what the purpose of the init_map and maps list is. Each Segment's map attribute gets set on all segments following the EXT-X-MAP, as defined by the HLS spec:
- https://tools.ietf.org/html/rfc8216#section-4.3.2.5
map_ = self.state.get("map") streamlink/src/streamlink/stream/hls_playlist.py
Lines 256 to 259 in 83fb57c
def parse_tag_ext_x_map(self, value): attr = self.parse_attributes(value) byterange = self.parse_byterange(attr.get("BYTERANGE", "")) self.state["map"] = Map(attr.get("URI"), byterange) streamlink/src/streamlink/stream/hls_playlist.py
Lines 246 to 248 in 83fb57c
def parse_tag_ext_x_discontinuity(self, value): self.state["discontinuity"] = True self.state["map"] = None
A much better implementation would be fetching the media initialization once and writing it to the buffer in the HLSStreamWriter.write method before it writes the fetched segment data. The map-fetching would require some modifications of the writer's fetch method and making it so that the media initialization data of the map attribute gets stored somewhere and shared between the segments with the same map.
Proper tests are also a requirement before any feature extensions to the streaming classes can get merged, ideally with full coverage.
Maybe you missed my comments:
|
|
With this apple demo it comes to many errors in the output stream:
|
That was my thought at first, but I’ve been dealing with it for days and my current solution is already quite optimal and you will probably find that too, if you take it seriously. The handling of the |
Wrong. The media initialization data of the See here:
The Sorry, but I don't think that your proposed solution is optimal. You are abusing the current implementation of the Worker/Writer and how the Adding hack after hack without any tests will make this an unmaintainable and inextensible mess. |
If that were the case, all my swiss wilmaa links would not run.
You are partly right, but an
I think the best tests are real life tests. The python tests only check the rudimentary functions, but that does not mean that those tests are useless. |
|
Well, in case of MPEG-TS segments, section 3.2 of the RFC 8216 says this:
Which means that when an For fMP4 segments in section 3.3, it's more complicated. The MIS via
The same instance of the
No. Unit and integration tests ensure that the implementation is correct and that future changes to any part of the project won't break anything. |
|
No response in ~3 months. Closing. |
you can test it with a swiss ip and this link.
closes #3328