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

[HLSTree] Fixes for parser regressions #1364

Merged
merged 3 commits into from
Aug 30, 2023

Conversation

CastagnaIT
Copy link
Collaborator

@CastagnaIT CastagnaIT commented Aug 25, 2023

Description

The current RemoveParameters method can be misleading and can delete required url parts
so cleaned up method by removing "filename" variable,
and then created an appropriate method to get url path

HLS D+ manifest case has shown another time some things that has not been taken in account after recent parser rework

First thing, multiple variants with same uri, was added more times as representation

#EXT-X-STREAM-INF:BANDWIDTH=4275411,AVERAGE-BANDWIDTH=2392560,CODECS="hvc1.2.4.H120.90,mp4a.40.2",RESOLUTION=1280x720,FRAME-RATE=24,VIDEO-RANGE=PQ,HDCP-LEVEL=TYPE-1,CHARACTERISTICS="com.dss.ctr.uhd",AUDIO="aac-128k",SUBTITLES="sub-main"
r/composite_2700k_CENC_CTR_UHD_HDR_HDR10_762ff663-9d08-497e-af6f-21855fff2eb5_b256a1e9-dd8d-4b19-b9cd-07b6a00b6567.m3u8

#EXT-X-STREAM-INF:BANDWIDTH=4404008,AVERAGE-BANDWIDTH=2521689,CODECS="hvc1.2.4.H120.90,ec-3,mp4a.40.2",RESOLUTION=1280x720,FRAME-RATE=24,VIDEO-RANGE=PQ,HDCP-LEVEL=TYPE-1,CHARACTERISTICS="com.dss.ctr.uhd",AUDIO="eac-3",SUBTITLES="sub-main"
r/composite_2700k_CENC_CTR_UHD_HDR_HDR10_762ff663-9d08-497e-af6f-21855fff2eb5_b256a1e9-dd8d-4b19-b9cd-07b6a00b6567.m3u8

Second thing, on renditions there may exists multile renditions with same identical properties but different GROUP-ID
and we should not add them more times

#EXT-X-MEDIA:TYPE=AUDIO,GROUP-ID="atmos",NAME="Deutsch",LANGUAGE="de",AUTOSELECT=YES,CHANNELS="6",URI="r/composite_256k_ec-3_de_PRIMARY_a39c570a-3c46-4db6-b42f-751c28f17566_10310d0e-3231-46ca-8ec4-822aec4d1f6b.m3u8"

#EXT-X-MEDIA:TYPE=AUDIO,GROUP-ID="eac-3",NAME="Deutsch",LANGUAGE="de",AUTOSELECT=YES,CHANNELS="6",URI="r/composite_256k_ec-3_de_PRIMARY_a39c570a-3c46-4db6-b42f-751c28f17566_10310d0e-3231-46ca-8ec4-822aec4d1f6b.m3u8"

The last one when we parse a child manifest that have multiple EXT-X-DISCONTINUITY was missing add base url to representations of switched period, and was causing bad download url address

Motivation and context

fix #1335 #1366

How has this been tested?

i wait user feedback as confirm
i tested some random dash/hls/ss

Screenshots (if appropriate):

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Clean up (non-breaking change which removes non-working, unmaintained functionality)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that will cause existing functionality to change)
  • Cosmetic change (non-breaking change that doesn't touch code)
  • None of the above (please explain below)

Checklist:

  • I have read the Contributing document
  • My code follows the Code Guidelines of this project
  • My change requires a change to the Wiki documentation
  • I have updated the documentation accordingly

@CastagnaIT CastagnaIT added Type: Fix non-breaking change which fixes an issue v21 Omega labels Aug 25, 2023
@CastagnaIT CastagnaIT added the Don't merge PR that should not be merged (yet) label Aug 25, 2023
The current RemoveParameters method can be misleading and can delete required url parts
Some manifests can have multiple audio rendition GROUPS with differents audio
codecs
means that multiple Variants with same uri but different AUDIO group may exists and we should not add them more times

Similar thing seem to happens for Renditions despite seem less common thing,
more Renditions with identical properties (and so same uri) but different GROUP-ID may exists, and we should not add them more times
@CastagnaIT CastagnaIT changed the title [AdaptiveTree] Removed some uses of RemoveParameters [HLSTree] Fixes for parser regressions Aug 26, 2023
@CastagnaIT CastagnaIT removed the Don't merge PR that should not be merged (yet) label Aug 27, 2023
@CastagnaIT CastagnaIT merged commit f4e47c6 into xbmc:Omega Aug 30, 2023
9 checks passed
@CastagnaIT CastagnaIT deleted the fix_url_methods branch August 30, 2023 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Fix non-breaking change which fixes an issue v21 Omega
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When starting an episode of series there are sound problems with the Disney+ addon
2 participants