Skip to content
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

Workaround for negative timestamp misinterpretation #64

Merged
merged 1 commit into from
Oct 8, 2018

Conversation

PyvesB
Copy link
Contributor

@PyvesB PyvesB commented Oct 2, 2018

This pull request is related to disccusions on the red5-client repository, issue #44.

Unsigned timestamps between 231 and 232 -1, which Red5 misinterprets as negative signed integers between -231 and -1, are now rolled over to positive integers between 0 and 231 - 1. Instead of being discarded, RTMP messages with large timestamps are now sent downstream.

I have added some unit tests to cover this behaviour and have successfully tested the patch on our pre-production environment with a few different RTMP output destinations.

@mondain mondain merged commit 0f18376 into Red5:master Oct 8, 2018
@mondain
Copy link
Member

mondain commented Oct 8, 2018

Thank you, @PyvesB

@PyvesB
Copy link
Contributor Author

PyvesB commented Nov 13, 2018

@mondain do you have any timeline on when the next Red 5 milestone will be published? It would be nice to have this easily available as part of a Maven release. Thanks in advance!

@PyvesB
Copy link
Contributor Author

PyvesB commented Dec 17, 2018

@mondain I noticed your recent 1.0.10-M10 release, however, it does not seem to contain the changes from this pull request. Is this to be expected?

@mondain
Copy link
Member

mondain commented Dec 17, 2018

It wasn't intentionally skipped.

@mondain
Copy link
Member

mondain commented Dec 17, 2018

It doesn't appear to be skipped, its in the codebase

@PyvesB
Copy link
Contributor Author

PyvesB commented Dec 17, 2018

It is in the codebase, but I didn't see it there when I decompiled the 1.0.10-M10 source. Might this be related to merge commit 3faa0ae, which was pushed after the tagging done in ec51822? I'm wondering whether the release of red5-server-common was not performed from an outdated local copy, which then led to that merge commit.

@mondain
Copy link
Member

mondain commented Dec 17, 2018

If its not in the decompile than you are probably correct; I'll get it in the release if I don't see any issues posted before January 1st.

@PyvesB
Copy link
Contributor Author

PyvesB commented Jan 11, 2019

@mondain would it now be a good time to attempt a new release? 😃

@mondain
Copy link
Member

mondain commented Jan 11, 2019

I'll get one out in the next couple days; I have a SharedObject fix in as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants