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 CAN-SKIP-UNTIL and delta generation #88

Merged
merged 3 commits into from
Sep 6, 2023
Merged

Conversation

sgfn
Copy link
Member

@sgfn sgfn commented Sep 4, 2023

As per comments in tests, delta can be created whenever the sum of durations of full segments in playlist exceeds 6 * TARGET-DURATION

lib/membrane_http_adaptive_stream/hls.ex Outdated Show resolved Hide resolved
lib/membrane_http_adaptive_stream/hls.ex Outdated Show resolved Hide resolved
lib/membrane_http_adaptive_stream/hls.ex Outdated Show resolved Hide resolved
@sgfn sgfn requested a review from Karolk99 September 6, 2023 12:13
Comment on lines 148 to 161
latest_full_segments <-
track.segments
# For some reason, Dialyzer complains without the following line.
# See discussion in https://github.com/membraneframework/membrane_http_adaptive_stream_plugin/pull/88
|> Function.identity()
|> Qex.reverse()
|> Enum.drop_while(&(&1.type == :partial)),
Copy link
Member Author

Choose a reason for hiding this comment

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

This is really strange. Without the pipe to Function.identity or IO.inspect, Dialyzer gives the following errors:

lib/membrane_http_adaptive_stream/hls.ex:123:pattern_match
The pattern can never match the type.

Pattern:
{:create_delta, _delta_ctx}

Type:
:dont_create_delta

________________________________________________________________________________
lib/membrane_http_adaptive_stream/hls.ex:179:pattern_match
The pattern can never match the type.

Pattern:
_track = %Membrane.HTTPAdaptiveStream.Manifest.Track{}, [{:delta?, true}]

Type:
atom() | map(), [{:delta?, false}, ...]

________________________________________________________________________________
lib/membrane_http_adaptive_stream/hls.ex:303:guard_fail
The guard clause:

when _segments_to_skip_count :: 0 > 0

can never succeed.

________________________________________________________________________________
lib/membrane_http_adaptive_stream/hls.ex:405:guard_fail
The guard clause:

when _duration :: 0 > 0

can never succeed.

________________________________________________________________________________
done (warnings were emitted)
Halting VM with exit status 2

Seems to be a bug in the utility.

Copy link
Contributor

Choose a reason for hiding this comment

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

if we want to merge it before the dialyzer fix I would prefer to use the @dialyzer tag to skip checking this function and some comment then Function.identity()

Copy link
Contributor

@Karolk99 Karolk99 left a comment

Choose a reason for hiding this comment

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

if we want to merge it before the dialyzer fix I would prefer to use the @dialyzer tag to skip checking this function and some comment then Function.identity()

@sgfn sgfn requested a review from Karolk99 September 6, 2023 14:39
@sgfn sgfn merged commit 6f6b102 into master Sep 6, 2023
3 checks passed
@sgfn sgfn deleted the sgfn/can-skip-until branch September 6, 2023 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants