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

rtp: fix up handling of extension header #84

Merged
merged 2 commits into from
Nov 12, 2021
Merged

Conversation

derkolja
Copy link

@derkolja derkolja commented Nov 8, 2021

Hi,

I ran into a bit of a problem trying to use the extension header. I am reasonably confident this fix is correct for the one byte header case, however, the RFC also specifies a Two-Byte Header, which I'm not certain it is supported here at all.
https://datatracker.ietf.org/doc/html/rfc5285#section-4.2

I hope this helps, I'm afraid I don't have any experience contributing to open source projects.

:-) Kolja

  • When setting the len field of the header, a bad offset of the source
    pointer was used
  • The len field of the header is a uint16 specifying payload length in
    multiples of 32 bit
  • Also, subtract the extension header length from the payload length

Kolja Schneider added 2 commits November 8, 2021 15:51
 - When setting the len field of the header, a bad offset of the source
   pointer was used
 - The len field of the header is a uint16 specifying payload length in
   multiples of 32 bit
 - Also, subtract the extension header length from the payload length
@jrsnen
Copy link
Member

jrsnen commented Nov 12, 2021

Hi!

Thank you for this PR. I don't see any problems with it, and I'm just going to merge it as is. The extension headers don't seem to be used in uvgRTP at the moment, but it is good to fix bugs in them regardless.

uvgRTP is still very much in its infancy, so there are lots of small things that need improving and fixing. Thank you for helping in this endeavor.

@jrsnen jrsnen merged commit f950185 into ultravideo:master Nov 12, 2021
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