fix DASH VOD and Live manifest and segment pacing #21

gmarzot · 2021-01-30T23:18:34Z

This works for DASH Live and VOD. Manifest GET rate for Live will not exceed media segment duration. For DASH VOD segment GET rate is determined by segment duration.

I have not looked at HLS yet.

… more than segments

gmarzot · 2021-02-18T17:02:12Z

any interest?

sebastianlorenzo88 · 2021-02-22T16:46:51Z

Hello, thanks for your contribution, we are going to review and test the PR as soon as possible.

Regards.

sebastianlorenzo88

Hello,

I left you some comments, mostly regarding code style to follow the conventions of the project.

Thanks for your contribution.
Regards

sebastianlorenzo88 · 2021-04-06T21:21:10Z

src/main/java/com/blazemeter/jmeter/videostreaming/dash/DashSampler.java

@@ -67,7 +67,9 @@ public MediaRepresentation findMatchingVariant(List<MediaRepresentation> variant
      boolean initialLoop = true;
      while (!mediaPlayback.hasEnded()) {
        if (mediaPlayback.needsManifestUpdate() && !initialLoop) {
-          long awaitMillis = manifest.getReloadTimeMillis(timeMachine.now());
+          long segmentDurationMillis = (long) mediaPlayback.getLastSegment().getDurationSeconds() * 1000;


This line is too long, does not fit with code style

Suggested change

long segmentDurationMillis = (long) mediaPlayback.getLastSegment().getDurationSeconds() * 1000;

long segmentDurationMillis =

(long) mediaPlayback.getLastSegment().getDurationSeconds() * 1000;

sebastianlorenzo88 · 2021-04-06T21:22:34Z

src/main/java/com/blazemeter/jmeter/videostreaming/dash/DashSampler.java

@@ -219,11 +221,9 @@ private boolean hasContents() {
      return segmentBuilder != null;
    }

+    // dynamic manifests (live) or empty segment list require manifest get (timing determined elsewhere)


again this line is too long

Suggested change

// dynamic manifests (live) or empty segment list require manifest get (timing determined elsewhere)

// dynamic manifests (live) or empty segment list require manifest get (timing determined

// elsewhere)

sebastianlorenzo88 · 2021-04-06T22:46:46Z

src/main/java/com/blazemeter/jmeter/videostreaming/dash/Manifest.java

@@ -6,6 +6,9 @@
 import io.lindstrom.mpd.data.MPD;
 import io.lindstrom.mpd.data.Period;
 import io.lindstrom.mpd.data.PresentationType;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;


according to the project code style, the imports should be in alphabetical order

sebastianlorenzo88 · 2021-04-06T22:49:18Z

src/main/java/com/blazemeter/jmeter/videostreaming/dash/Manifest.java

  private final List<MediaPeriod> periods;
+  private Instant playbackStartTime;
+  private static final Logger LOG = LoggerFactory.getLogger(Manifest.class);


static variables should be defined at the beginning of variables definition

sebastianlorenzo88 · 2021-04-06T22:50:47Z

src/main/java/com/blazemeter/jmeter/videostreaming/dash/Manifest.java

@@ -97,7 +106,12 @@ public Duration getBufferStartTime() {

  public Instant getAvailabilityStartTime() {
    OffsetDateTime time = mpd.getAvailabilityStartTime();
-    return time != null ? time.toInstant() : Instant.MIN;
+    // simulating availabilityStartTime for VOD enables pacing of segment GETs according to segment duration


line too long

Suggested change

// simulating availabilityStartTime for VOD enables pacing of segment GETs according to segment duration

// simulating availabilityStartTime for VOD enables pacing of segment GETs according to

// segment duration

sebastianlorenzo88 · 2021-04-06T22:52:22Z

src/main/java/com/blazemeter/jmeter/hls/logic/VideoStreamingSamplerFactory.java

@@ -12,7 +12,7 @@
      VideoStreamingHttpClient httpClient,
      TimeMachine timeMachine, SampleResultProcessor sampleResultProcessor) {
    //HLS Master Playlist must contain this .m3u8 extension in their URLs
-    if (url.contains(".m3u8")) {
+    if (url.contains(".m3u8")||url.contains("format=m3u8")) {


Missing space between OR

Suggested change

if (url.contains(".m3u8")||url.contains("format=m3u8")) {

if (url.contains(".m3u8") || url.contains("format=m3u8")) {

sebastianlorenzo88 · 2021-04-07T20:39:55Z

src/main/java/com/blazemeter/jmeter/videostreaming/dash/Manifest.java

+  public long getReloadTimeMillis(long segmentDurationMillis) {
+    Duration minUpdatePeriod = mpd.getMinimumUpdatePeriod();
+    // wait at least segment duration if minUpdatePeriod is 0 (or very small)
+    long maxIntervalTime = Math.max(minUpdatePeriod.toMillis(), segmentDurationMillis);
+    Instant now = Instant.now();
+
+    return Math.max(maxIntervalTime - Duration.between(lastDownLoadTime, now).toMillis(), 0);


Both minUpdatePeriod & now could be inlined, due to them are only used once.

Suggested change

public long getReloadTimeMillis(long segmentDurationMillis) {

Duration minUpdatePeriod = mpd.getMinimumUpdatePeriod();

// wait at least segment duration if minUpdatePeriod is 0 (or very small)

long maxIntervalTime = Math.max(minUpdatePeriod.toMillis(), segmentDurationMillis);

Instant now = Instant.now();

return Math.max(maxIntervalTime - Duration.between(lastDownLoadTime, now).toMillis(), 0);

public long getReloadTimeMillis(long segmentDurationMillis) {

// wait at least segment duration if minUpdatePeriod is 0 (or very small)

long maxIntervalTime = Math.max(mpd.getMinimumUpdatePeriod().toMillis(), segmentDurationMillis);

return Math

.max(maxIntervalTime - Duration.between(lastDownLoadTime, Instant.now()).toMillis(), 0);

}

RicardoPoleo · 2021-12-06T17:32:00Z

Any updates on this PR? @gmarzot

gmarzot · 2021-12-06T22:16:54Z

oh boy - I am very sorry to have let this slide.. my patches did more or less what i wanted locally and even though all the requests made sense never got to it. I do continue to see cases where the pacing gets really extreme .. looking like a request storms. Hopefully i'll find time to get back to it in the new year

…

On Mon, Dec 6, 2021 at 12:32 PM Ricardo Poleo ***@***.***> wrote: Any updates on this PR? @gmarzot <https://github.com/gmarzot> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#21 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB7T3ID7PNHJM4Z272VPLGLUPTXRXANCNFSM4W2YFPEQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

G S Marzot added 2 commits January 30, 2021 17:27

fix calculation of VOD segment pacing, live DASH manifest GET rate no…

482602c

… more than segments

fix DASH VOD and Live manifest and segment pacing

d5f221f

gmarzot mentioned this pull request Feb 4, 2021

Query pacing higher than expected from playlist segment duration #20

Open

G S Marzot added 2 commits March 16, 2021 10:21

support other HLS URL formats

dc06fd3

Merge branch 'master' of https://github.com/gmarzot/HLSPlugin

42c1d70

sebastianlorenzo88 reviewed Apr 7, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix DASH VOD and Live manifest and segment pacing #21

fix DASH VOD and Live manifest and segment pacing #21

gmarzot commented Jan 30, 2021

gmarzot commented Feb 18, 2021

sebastianlorenzo88 commented Feb 22, 2021

sebastianlorenzo88 left a comment

sebastianlorenzo88 Apr 6, 2021

sebastianlorenzo88 Apr 6, 2021

sebastianlorenzo88 Apr 6, 2021

sebastianlorenzo88 Apr 6, 2021

sebastianlorenzo88 Apr 6, 2021

sebastianlorenzo88 Apr 6, 2021

sebastianlorenzo88 Apr 7, 2021

RicardoPoleo commented Dec 6, 2021

gmarzot commented Dec 6, 2021 via email

	long segmentDurationMillis = (long) mediaPlayback.getLastSegment().getDurationSeconds() * 1000;
	long segmentDurationMillis =
	(long) mediaPlayback.getLastSegment().getDurationSeconds() * 1000;

	// dynamic manifests (live) or empty segment list require manifest get (timing determined elsewhere)
	// dynamic manifests (live) or empty segment list require manifest get (timing determined
	// elsewhere)

	// simulating availabilityStartTime for VOD enables pacing of segment GETs according to segment duration
	// simulating availabilityStartTime for VOD enables pacing of segment GETs according to
	// segment duration

	if (url.contains(".m3u8")\|\|url.contains("format=m3u8")) {
	if (url.contains(".m3u8") \|\| url.contains("format=m3u8")) {

fix DASH VOD and Live manifest and segment pacing #21

Are you sure you want to change the base?

fix DASH VOD and Live manifest and segment pacing #21

Conversation

gmarzot commented Jan 30, 2021

gmarzot commented Feb 18, 2021

sebastianlorenzo88 commented Feb 22, 2021

sebastianlorenzo88 left a comment

Choose a reason for hiding this comment

sebastianlorenzo88 Apr 6, 2021

Choose a reason for hiding this comment

sebastianlorenzo88 Apr 6, 2021

Choose a reason for hiding this comment

sebastianlorenzo88 Apr 6, 2021

Choose a reason for hiding this comment

sebastianlorenzo88 Apr 6, 2021

Choose a reason for hiding this comment

sebastianlorenzo88 Apr 6, 2021

Choose a reason for hiding this comment

sebastianlorenzo88 Apr 6, 2021

Choose a reason for hiding this comment

sebastianlorenzo88 Apr 7, 2021

Choose a reason for hiding this comment

RicardoPoleo commented Dec 6, 2021

gmarzot commented Dec 6, 2021 via email