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

Is the RTMP chunk type not fully supported? #65

Closed
udoless opened this issue Oct 24, 2023 · 3 comments · Fixed by #72
Closed

Is the RTMP chunk type not fully supported? #65

udoless opened this issue Oct 24, 2023 · 3 comments · Fixed by #72
Labels
enhancement New feature or request

Comments

@udoless
Copy link

udoless commented Oct 24, 2023

I was looking at the code of the zip_chunk_header method, and it seems like the 'if' logic inside should never be executed (because self.csid_2_chunk_header is always empty and there is no assignment to it).

fn zip_chunk_header(&mut self, chunk_info: &mut ChunkInfo) -> Result<PackResult, PackError> {
chunk_info.basic_header.format = 0;
let pre_header = self
.csid_2_chunk_header
.get_mut(&chunk_info.basic_header.chunk_stream_id);
if let Some(val) = pre_header {
let cur_msg_header = &mut chunk_info.message_header;
let pre_msg_header = &val.message_header;
if cur_msg_header.msg_streamd_id == pre_msg_header.msg_streamd_id {
chunk_info.basic_header.format = 1;
cur_msg_header.timestamp -= pre_msg_header.timestamp;
if cur_msg_header.msg_type_id == pre_msg_header.msg_type_id
&& cur_msg_header.msg_length == pre_msg_header.msg_length
{
chunk_info.basic_header.format = 2;
if chunk_info.message_header.timestamp == pre_msg_header.timestamp {
chunk_info.basic_header.format = 3;
}
}
}
}
Ok(PackResult::Success)
}

@harlanc
Copy link
Owner

harlanc commented Oct 24, 2023

Thanks for your report, you are right, some logics are missed and the Chunk header should be cached in the csid_2_chunk_header , could you do some fix and make a PR?

@harlanc harlanc added the enhancement New feature or request label Oct 24, 2023
@udoless
Copy link
Author

udoless commented Oct 24, 2023

Thank you for the response. I'm not very familiar with this project right now, but I can give it a try if there's an opportunity in the future.

@harlanc harlanc linked a pull request Oct 28, 2023 that will close this issue
@harlanc
Copy link
Owner

harlanc commented Nov 4, 2023

I added the missing logic and did some simple tests, now different format types can be set for RTMP chunks, any other questions please let me know.

@harlanc harlanc removed a link to a pull request Nov 26, 2023
@harlanc harlanc linked a pull request Nov 26, 2023 that will close this issue
@harlanc harlanc closed this as completed Nov 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants