Skip to content

RTMP: Refactor RTMP extended timestamp. #4365

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

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

winlinvip
Copy link
Member

@winlinvip winlinvip commented May 30, 2025

Refactor the extended timestamp code for #4356, and follow the below steps:

  1. Read the timestamp from the buffer.
  2. Read the extended timestamp if the timestamp indicates that an extended timestamp is present.
  3. Skip back 4 bytes if following the RTMP v1 2009 version, which specifies that there is no extended timestamp for Type 3 chunks.
  4. Generate the chunk timestamp, treating it as an absolute timestamp for Type 0, or as a delta timestamp for Type 1 or 2.

This improvement will be merged to develop branch only.

@winlinvip winlinvip added the EnglishNative This issue is conveyed exclusively in English. label May 30, 2025
@winlinvip winlinvip changed the title RTMP: Refactor for extended timestamp. RTMP: Refactor RTMP extended timestamp. May 30, 2025
@duiniuluantanqin duiniuluantanqin requested a review from Copilot June 12, 2025 00:56
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the RTMP extended timestamp handling to align with updated RTMP specifications and improve clarity in the timestamp update logic. Key changes include:

  • Removing the obsolete branch for updating the timestamp when no extended timestamp is present.
  • Updating comments to clearly differentiate between RTMP v1 2009 and RTMP v1 2012 behaviors.
  • Adjusting the logic for reading and processing extended timestamps, including conditionally skipping 4 bytes when necessary.
Comments suppressed due to low confidence (1)

trunk/src/protocol/srs_protocol_rtmp_stack.cpp:1125

  • The variable 'timestamp' is referenced before it is declared. Consider computing and declaring 'timestamp' prior to its usage in this condition to ensure proper behavior.
if (!is_first_chunk_of_msg && chunk_extended_timestamp > 0 && chunk_extended_timestamp != timestamp) {

@winlinvip winlinvip force-pushed the develop branch 2 times, most recently from 195435d to 97e2b64 Compare July 1, 2025 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
EnglishNative This issue is conveyed exclusively in English.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant