client: handle FLV onMetaData script tags from direct publishers - #26
Merged
Merged
Conversation
* ret now starts as PEX_RTMP_SERVER_STATUS_OK; the genuine parse failure explicitly sets PEX_RTMP_SERVER_STATUS_PARSE_FAILED before goto done, and the loop bails out early if a message handler fails. * Added client_handle_flv_script_data() which decodes the onMetaData AMF payload of a MSG_NOTIFY FLV tag into client->metadata and sets new_metadata, so the updated framerate is actually propagated to subscribers via client_maybe_update_metadata() instead of being silently dropped (previously subscribers always got the hardcoded framerate = 30.0 from client_set_default_metadata).
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the direct-FLV ingestion path for publishers so that FLV MSG_NOTIFY script-data (onMetaData) is decoded and applied to the publisher’s metadata, ensuring updated values (e.g., framerate) are propagated to subscribers. It also tightens error handling in the FLV tag parsing loop by explicitly reporting parse failures and aborting early when message handling fails.
Changes:
- Initialize
client_handle_flv_buffer()status asPEX_RTMP_SERVER_STATUS_OK, explicitly setPARSE_FAILEDon tag-header parse failure, and stop processing further tags if a handler returns an error. - Add
client_handle_flv_script_data()to decode FLVonMetaDataAMF payload intoclient->metadataand marknew_metadatafor propagation. - Handle FLV
MSG_NOTIFYtags in the FLV parsing loop by invoking the new metadata decoding path.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
camilo-celis
left a comment
camilo-celis
left a comment
Contributor
There was a problem hiding this comment.
Amazing analysis on the MCU issue ... Not sure how u digged that one out 😆
Not too familiar with all various use cases, so just left question assuming we want to be as defensive as possible.
…but we have to verify it has be decoded correctly.
havardgraff
reviewed
Aug 27, 2026
Co-authored-by: havardgraff <1926313+havardgraff@users.noreply.github.com>
Contributor
|
@camilo-celis I've opened a new pull request, #27, to work on those changes. Once the pull request is ready, I'll request review from you. |
camilo-celis
changed the title
camilo-celis
pushed a commit
that referenced
this pull request
Sep 7, 2026
* Work for Issue pexip/mcu#50793 * `client_handle_flv_buffer()` handler failures are intentionally ignored now, while parse failures are still surfaced via the parse status. * Added client_handle_flv_script_data() which decodes the onMetaData AMF payload of a MSG_NOTIFY FLV tag into client->metadata and sets new_metadata, so the updated framerate is actually propagated to subscribers via client_maybe_update_metadata() instead of being silently dropped (previously subscribers always got the hardcoded framerate = 30.0 from client_set_default_metadata). * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Remove erroneous bracket. * Fixup handling of amf_dec_load_object call as it never returns NULL, but we have to verify it has be decoded correctly. * Fixup amf_enc_write_int to write a double if AMF0 as it has no integer type. * Parse status and per-subscriber send status tracked separately. * Now merge updates into client->metadata instead of replacing and loosing old values. * Move AMF payload validation helper to amf.c Co-authored-by: havardgraff <1926313+havardgraff@users.noreply.github.com> * Fixup windows static builds now the define is being passed down correctly. --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: havardgraff <1926313+havardgraff@users.noreply.github.com> (cherry picked from commit 62058dd)
This file contains hidden or 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
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.
client_handle_flv_buffer()handler failures are intentionally ignored now, while parse failures are still surfaced via the parse status.Work for pexip/mcu#50793