Demuxer: emit an error in case of unknown data packets and EOF #32
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
aler9
changed the title
aler9
changed the title
aler9
changed the title
asticode
requested changes
Oct 1, 2021
| // We need to silence this error as there may be some incomplete data here | ||
| // We still want to try to parse all packets, in case final data is complete | ||
| continue | ||
| } | ||
|
|
||
| // Update data | ||
| if d = dmx.updateData(ds); d != nil { | ||
| err = nil |
There was a problem hiding this comment.
Why are you resetting err here? 🤔
imho we still want to return an ErrNoMorePackets here so that the developer knows the end of stream has been reached.
There was a problem hiding this comment.
i'm not sure about this, in my opinion a read function should return either (data, nil) or (nil, error) - in this case, even if we reset the error, it reappears at the next function call and makes the function return (nil, error)
At the moment, the demuxer returns (nil, nil) when the stream is finished, there are TS packets in the queue but they're unknown data packets. This patch makes the demuxer emit (nil, ErrNoMorePackets).
|
FYI I've created a |
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.
Fixes #26
Fixes bluenviron/mediamtx#558
At the moment, demuxer's
NextData()returns(nil, nil)when the stream is finished (EOF), there are TS packets in the queue but they're unknown data packets (not PSI or PES).This is due to the fact that
erris overridden by a call toparseData(), that sets it tonilunless there's a parse error.In my opinion, the function:
(nil, ErrNoMorePackets)in case there are TS packets with payload in the queue but they can't be parsed.(data, nil)in case the TS packets in the queue are known data packetsThis patch fixes the issue.