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

[DASHTree] stop waiting when new period appears to fix stalls during playback #1541

Closed
wants to merge 1 commit into from

Conversation

tmm1
Copy link
Contributor

@tmm1 tmm1 commented Apr 29, 2024

Description

fixes #1540
cc #1175

Motivation and context

How has this been tested?

tested on stream and confirmed no more stalls

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

Comment on lines +1694 to +1698
if (repr->IsWaitForSegment() &&
period->GetId() != m_periods.back()->GetId())
{
repr->SetIsWaitForSegment(false);
}
Copy link
Collaborator

@CastagnaIT CastagnaIT Apr 29, 2024

Choose a reason for hiding this comment

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

you havent provide any explanation nor manifests that explain why this is needed
when there are no segments you must not remove the "wait for segments"
the demuxer callbacks must be kept stopped

if (repr->IsWaitForSegment() && (repr->get_next_segment(repr->current_segment_)))
if (repr->IsWaitForSegment() &&
(repr->get_next_segment(repr->current_segment_) ||
period->GetId() != m_periods.back()->GetId()))
Copy link
Collaborator

Choose a reason for hiding this comment

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

this addition is not clear
period->GetId() != m_periods.back()->GetId()

Copy link
Collaborator

@CastagnaIT CastagnaIT left a comment

Choose a reason for hiding this comment

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

i would have a way to replicate this, since the changes are not clear
and the root of cause may be elsewhere

if a new period appears, it should trigger the last segment check on AdaptiveStream when the current one is ending
if this dont happens can suggest some other problem, for example is missing to be updated the period duration

@CastagnaIT
Copy link
Collaborator

closed since atm these changes are not so clear
if you are willing to provide copies of the mpd's downloaded by ISA during playback with related full debug log,
will be possible for us to understand the situation and then make a fix or allow you to modify the code appropriately
i will keep open related issue for about a week, if no responses also the issue will be closed

@CastagnaIT CastagnaIT closed this May 21, 2024
@tmm1
Copy link
Contributor Author

tmm1 commented May 27, 2024

Was discussed in detail during initial implementation. https://teamkodi.slack.com/archives/C0U5RMJSD/p1677448422522129

@CastagnaIT
Copy link
Collaborator

on that discussion you are talking about test stream
https://livesim.dashif.org/livesim2/periods_20/testpic_2s/Manifest.mpd

this one has been already tested on Omega over last recent changes,
anyway testing it on current master, results with a correct playback behaviour:

https://paste.kodi.tv/uhejasiquq.kodi

manifest_1716875099.txt
manifest_1716875101.txt

Inserting new Period (id=P9538195, start=1716875100000)
...
Check for last segment (period end PTS: 1716875100000, segment end PTS: 1716875100000)
...
Switching to new Period (id=P9538195, start=1716875100000, seq=2)

and segment downloaded from switched period starts with the right numeration

[AS-2] Download finished: https://livesim.dashif.org/livesim2/periods_20/testpic_2s/A48/858437550.m4s (downloaded 13188 byte, speed 107627.00 byte/s)
[AS-3] Download finished: https://livesim.dashif.org/livesim2/periods_20/testpic_2s/V300/858437550.m4s (downloaded 38009 byte, speed 324261.00 byte/s)

so your live stream looks to be different than this test stream

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.

MPD with new period gets stuck during playback
2 participants