VPAAMP-457 Cherry-pick middleware subtitle PTS offset changes from aa…#184
Open
psiva01 wants to merge 1 commit into
Open
VPAAMP-457 Cherry-pick middleware subtitle PTS offset changes from aa…#184psiva01 wants to merge 1 commit into
psiva01 wants to merge 1 commit into
Conversation
…mp commit 2da7d5d Instead of rewriting the MPEGTS field in the VTT byte stream (fragile header surgery), the PTS offset is pushed into the subtec channel as a signed time offset, matching the pattern already used on the DASH path. - subtitleParser.h: add virtual setPtsOffset(double) no-op to base class - WebVttSubtecParser.hpp: declare setPtsOffset override; change time_offset_ms_ to signed int64_t - WebVttSubtecParser.cpp: implement setPtsOffset; pass time_offset_ms_ to SendDataPacket Cherry-picked from rdkcentral/aamp@2da7d5d
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a per-fragment subtitle PTS offset API on SubtitleParser and wires that offset into the WebVTT subtec path by passing a signed millisecond offset into SubtecChannel::SendDataPacket, avoiding MPEGTS/VTT header rewriting.
Changes:
- Add
SubtitleParser::setPtsOffset(double)as a default no-op extension point. - Update
WebVTTSubtecParserto store a signedtime_offset_ms_and pass it intoSendDataPacket. - Implement
WebVTTSubtecParser::setPtsOffset()to convert seconds to a signed millisecond offset using the subtec subtraction convention.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| subtitle/subtitleParser.h | Adds a new virtual setPtsOffset API and documents intended usage. |
| subtec/subtecparser/WebVttSubtecParser.hpp | Declares setPtsOffset override and makes time_offset_ms_ signed. |
| subtec/subtecparser/WebVttSubtecParser.cpp | Implements setPtsOffset and forwards time_offset_ms_ into SendDataPacket. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+96
to
+104
| /** | ||
| * @brief Set a per-fragment PTS offset (seconds) that the parser | ||
| * applies when forwarding cue data to its subtitle sink. | ||
| * | ||
| * Used by the HLS PTS-restamp path to align subtitle display time | ||
| * with the restamped video PTS without rewriting MPEGTS in the | ||
| * VTT header. Default no-op; subtec-based parsers override. | ||
| */ | ||
| virtual void setPtsOffset(double ptsOffsetSec) {} |
| * with the restamped video PTS without rewriting MPEGTS in the | ||
| * VTT header. Default no-op; subtec-based parsers override. | ||
| */ | ||
| virtual void setPtsOffset(double ptsOffsetSec) {} |
Comment on lines
69
to
73
| bool WebVTTSubtecParser::processData(const char* buffer, size_t bufferLen, double position, double duration) | ||
| { | ||
| std::string str(const_cast<const char*>(buffer), bufferLen); | ||
| std::vector<uint8_t> data(str.begin(), str.end()); | ||
|
|
Comment on lines
+79
to
+83
| void WebVTTSubtecParser::setPtsOffset(double ptsOffsetSec) | ||
| { | ||
| // Subtec's display_time = media_PTS - time_offset_ms (subtraction | ||
| // convention shared with Rialto SetSubtitlePtsOffset). The HLS | ||
| // restamped video PTS is media_PTS + ptsOffsetSec, so we negate |
DomSyna
approved these changes
Jun 26, 2026
DomSyna
left a comment
DomSyna
left a comment
There was a problem hiding this comment.
So I'm assuming as its not clear from the PR description (to me anyway) this is porting the changes from the "amp middleware" to middleware develop as part of aligning the code.
The port looks identical to me
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.
…mp commit 2da7d5d
Instead of rewriting the MPEGTS field in the VTT byte stream (fragile header surgery), the PTS offset is pushed into the subtec channel as a signed time offset, matching the pattern already used on the DASH path.
Cherry-picked from rdkcentral/aamp@2da7d5d