Skip to content

Conversation

@FelonEkonom
Copy link
Member

@FelonEkonom FelonEkonom commented Oct 7, 2025

I got a playlist like this from the internet and IMO we should be able to handle it

#EXTM3U
#EXT-X-VERSION:3
#EXT-X-TARGETDURATION:7
#EXT-X-MEDIA-SEQUENCE:111174
#EXTINF:6,
l_3063418_667045738_111174.ts?nimblesessionid=614412691
#EXTINF:6.001,
l_3063418_667051738_111175.ts?nimblesessionid=614412691
#EXTINF:5.99,
l_3063418_667057739_111176.ts?nimblesessionid=614412691
#EXTINF:5.998,
l_3063418_667063729_111177.ts?nimblesessionid=614412691
#EXTINF:5.997,
l_3063418_667069727_111178.ts?nimblesessionid=614412691
#EXTINF:6.003,
l_3063418_667075724_111179.ts?nimblesessionid=614412691

@FelonEkonom FelonEkonom requested review from Copilot and varsill October 7, 2025 12:31
Copy link

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 segment URI extension handling logic to better support segment URLs with query parameters or other extensions beyond the basic file extension. The change centralizes the demuxing engine resolution logic and modifies the pattern matching to handle cases where extensions might be followed by additional content.

  • Extracts duplicated demuxing engine resolution logic into a shared utility function
  • Updates pattern matching to handle segment URIs with query parameters or additional content after the extension
  • Bumps version from 0.1.2 to 0.1.3

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
mix.exs Version bump to 0.1.3
lib/ex_hls/client/vod.ex Refactored to use centralized demuxing engine resolution utility
lib/ex_hls/client/utils.ex Added new utility function with updated pattern matching for extensions
lib/ex_hls/client/live/reader.ex Refactored to use centralized demuxing engine resolution utility
README.md Updated dependency version in documentation

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@FelonEkonom FelonEkonom self-assigned this Oct 7, 2025
@FelonEkonom FelonEkonom moved this to In Review in Smackore Oct 7, 2025

@spec resolve_demuxing_engine_impl(String.t()) :: atom()
def resolve_demuxing_engine_impl(segment_uri) do
case Path.extname(segment_uri) do
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the more proper way to do it would be to do:

URI.parse!(segment_uri).path 
|> Path.extname()
|> case do
         ".ts" -> DemuxingEngine.MPEGTS
        ".m4s" -> DemuxingEngine.CMAF
        ".mp4" -> DemuxingEngine.CMAF
        _other -> raise "Unsupported segment URI extension: #{segment_uri |> inspect()}"
end

because this way we let URI retrieve the proper "path" for us, which we can later use with Path.extname().

@FelonEkonom FelonEkonom requested a review from varsill October 14, 2025 10:19
Copy link
Collaborator

@varsill varsill left a comment

Choose a reason for hiding this comment

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

🥇

@FelonEkonom FelonEkonom merged commit 21684ff into master Oct 14, 2025
3 checks passed
@github-project-automation github-project-automation bot moved this from In Review to Done in Smackore Oct 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants