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

fix(DASH): Fix MPD Patch when SegmentTemplate is shared between Representations #7218

Merged

Conversation

tykus160
Copy link
Member

Fixes #7214

@tykus160 tykus160 added type: bug Something isn't working correctly priority: P1 Big impact or workaround impractical; resolve before feature release component: DASH The issue involves the MPEG DASH manifest format labels Aug 27, 2024
@tykus160 tykus160 requested a review from avelad August 27, 2024 12:05
@avelad avelad added this to the v4.11 milestone Aug 27, 2024
@shaka-bot
Copy link
Collaborator

shaka-bot commented Aug 27, 2024

Incremental code coverage: 97.78%

@avelad
Copy link
Member

avelad commented Aug 27, 2024

After talking to @tykus160 , it still can't be merged because there is an error in the compiled build.

@wilsonluniz-harmonicinc

Just curious if I can get a live version of this PR for test? So that I don't need to setup my HTTPS server.

@wilsonluniz-harmonicinc

@tykus160 Seems only <add/> works. I tested neither <remove/> or <replace/> works.

In practice, MPD:

<S d="1" t="0" />
<S d="2" t="1" />
<S d="1" t="3" />

Remove patch:

<remove sel="/MPD/Period[@id='1']/AdaptationSet[1]/SegmentTemplate/SegmentTimeline/S[t='3']">
</remove>

Replace patch:

 <replace sel="/MPD/Period[@id='1']/AdaptationSet[1]/SegmentTemplate/SegmentTimeline/S[t='3']/@r">
    3
</replace>

Error message:

Expected _class2({ startTime: 3, endTime: 4, trueEndTime: 4, ... }) to equal _class2({ startTime: 3, endTime: 6, trueEndTime: 6, ... }).

@tykus160
Copy link
Member Author

@wilsonluniz-harmonicinc I think issues you've mentioned might happen also when SegmentTemplate is not shared between Representations. Could you confirm it? If so, they will be handled in separate PRs.

@wilsonluniz-harmonicinc

@tykus160 Oh yes you're right. Both <replace> and <remove> not works even I put <SegmentTemplate> into <Representation> and change @sel respectively. Should I open another issue for that?

@avelad
Copy link
Member

avelad commented Aug 28, 2024

@tykus160 Oh yes you're right. Both <replace> and <remove> not works even I put <SegmentTemplate> into <Representation> and change @sel respectively. Should I open another issue for that?

Yes please!

@tykus160
Copy link
Member Author

@wilsonluniz-harmonicinc you can, it will make tracking of that easier.

@avelad avelad merged commit b2502fd into shaka-project:main Aug 28, 2024
14 of 16 checks passed
@tykus160 tykus160 deleted the wt-mpd-patch-shared-fix-upstream branch August 28, 2024 16:10
avelad pushed a commit that referenced this pull request Aug 29, 2024
avelad pushed a commit that referenced this pull request Aug 29, 2024
@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Oct 27, 2024
@shaka-project shaka-project locked as resolved and limited conversation to collaborators Oct 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
component: DASH The issue involves the MPEG DASH manifest format priority: P1 Big impact or workaround impractical; resolve before feature release status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MPD Patch: Incorrectly apply patch multiple times?
4 participants