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

Multi-period with presentationTimeOffset can overwrite previous period #1098

Closed
tjber opened this issue Oct 31, 2017 · 20 comments
Closed

Multi-period with presentationTimeOffset can overwrite previous period #1098

tjber opened this issue Oct 31, 2017 · 20 comments
Assignees
Labels
status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Milestone

Comments

@tjber
Copy link

tjber commented Oct 31, 2017

Hi,

Continued with [https://groups.google.com/forum/#!topic/shaka-player-users/2RK662rDMk8]

I didn't try to play it with the master.
I have used the demo application of version 2.2.4 in chrome and firefox

I have a system in which I will receive a series of video files containing a number of black frames at the beginning.
Currently the process to play those files is to transcode them to remove those frames and start playing from 0, but I'd like to skip this step.
So, I tried to use the presentationTimeOffset attribute in the mpd file.
<SegmentBase indexRangeExact="true" indexRange="" presentationTimeOffset="" >

The problem is that I need frame accuracy and it seems that this attribute only has second precision.
I have tried several combinations with this attribute and the timeScale without success. Dash especifications says :

presentationTimeOffset : The value of the presentation time offset in seconds is the division of the value of this attribute and the value of the timescale attribute.

but I get the feeling that this attribute is not being taken into account.

I attach the mpd file I'm testing with.
dash.mpd.txt

I have updated in dropbox the files with which the issue can be played back, and I have added a file in which you can see the expected result (expected_file.mp4) https://www.dropbox.com/sh/zfbynu6n9bxhd9q/AAB4TEvA9g71UADgne0pqwKia?dl=0

What I would like to do is to skip the initial frames (marked with a white counter) and that the playback starts in frame marked with the TC 00:00:00:00 of the file with the green TC, after that, in the second period skip the initial frames again and that the playback starts in frame marked with the TC 00:00:00:00 of the file with the red TC.

Currently if I try to jump 3.12 seconds actually a jump of 4 seconds is made, so for the first period the playback starts at TC 00:00:00:22 instead of 00:00:00:00 and the second period does not jump at all.

If I can change some of the manifest file to make this work, any ideas will be welcome.

Thank you in advance.
TJ

@TheModMaker
Copy link
Contributor

First, presentationTimeOffset must be an integer, so the value 3.12 is invalid. You can use the timescale attribute to get decimals, but it should match the timescale in the media. So if the timescale is 100, then the presentationTimeOffset would be 312.

It also looks like we aren't handling the presentationTimeoffset value correctly with timescale. I'm marking this a duplicate of #1099.

@TheModMaker TheModMaker added the status: duplicate A duplicate of another issue; should be closed after linking to the original issue label Oct 31, 2017
@joeyparrish
Copy link
Member

@tjber, when #1099 is fixed (soon), you should try again with the presentationTimeOffset field set with respect to timescale. Please follow along on #1099. Sorry for the confusion.

@joeyparrish
Copy link
Member

If you are still having issues after #1099 is fixed, we can reopen this issue and try again. Thanks!

@joeyparrish
Copy link
Member

The fix for #1099 has been pushed to the master branch.

@tjber
Copy link
Author

tjber commented Nov 2, 2017

I'll test it and come back with the results.
Thank you very much!

@tjber
Copy link
Author

tjber commented Nov 6, 2017

Hi!
I've tried again, both with the Demo app and the master.

The timescale attribute is already taken into account in the SegmentBase, but frame accuracy is not yet possible, it only jumps from second to second.
I mean, with these values:
<SegmentBase indexRangeExact="true" indexRange="915-1138" timescale = "100" presentationTimeOffset="212" >
Playback starts at second 3, not 2,120.

Also, If the manifest has more than 1 period, the presentationTimeOffset is only taken into account in the first period, playback of the following periods starts in the second 0.

This is the mpd file i used for testing.

dash.mpd.txt

Thanks!

@TheModMaker
Copy link
Contributor

Without a playable URL to test with, it will be hard to diagnose the problem. You can send it to [email protected] if you can't post it publically.

My guess (without being able to look at the content) is that there is a keyframe at 3 seconds. When we set the presentationTimeOffset, this changes where the media time 0 is. The browser will drop any frames that appear before time 0. If the first frame after 0 is not a keyframe, the browser won't be able to decode it so it will discard those frames until it hits a keyframe.

@TheModMaker TheModMaker reopened this Nov 6, 2017
@TheModMaker TheModMaker added needs triage and removed status: duplicate A duplicate of another issue; should be closed after linking to the original issue labels Nov 6, 2017
@joeyparrish joeyparrish added the status: waiting on response Waiting on a response from the reporter(s) of the issue label Nov 7, 2017
@tjber
Copy link
Author

tjber commented Nov 7, 2017

Hi,

@TheModMaker , I've tried your guess and it's good. I have generated a file in which all the frames are keyframes and so it is positioned correctly.
Unfortunately, in my case it's not a valid solution because I won't be able to regenerate the files, so I'll need to find a workaround.

The playback of the second period still does not take into account the presentationTimeOffset, even with these new files.

I've added the regenerated materials and the two manifests I've tested with to the dropbox folder:
https://www.dropbox.com/sh/zfbynu6n9bxhd9q/AAB4TEvA9g71UADgne0pqwKia?dl=0
dash. mpd --> manifest that I've been doing the tests
dash_allkeyframes. mpd --> manifest that refers to regenerated files in which all frames are keyframes
(you can download and use them freely, they are a replica of my original files that I created to document the issue)

@joeyparrish joeyparrish removed the status: waiting on response Waiting on a response from the reporter(s) of the issue label Nov 7, 2017
@joeyparrish
Copy link
Member

If your goal is to limit what's played by skipping the first, say, 2.1 seconds, you should try the playRangeStart configuration instead of modifying the manifest:

https://shaka-player-demo.appspot.com/docs/api/shakaExtern.html#PlayerConfiguration

player.configure({
  playRangeStart: 2.1
});

Does this help?

@joeyparrish joeyparrish added type: question A question from the community and removed needs triage labels Nov 7, 2017
@joeyparrish joeyparrish changed the title Problem with presentationTimeOffset Using presentationTimeOffset to limit display or edit content Nov 7, 2017
@joeyparrish
Copy link
Member

Okay, I think I see what's going on now. We are using presentationTimeOffset in the second Period, but the content is being offset back into the previous Period's time range and overwriting content starting at time 11. Period 1's duration is 13.12, PTO for period 2 is -2.12, so the content starts appearing at 11.

@joeyparrish joeyparrish changed the title Using presentationTimeOffset to limit display or edit content Multi-period with presentationTimeOffset can overwrite previous period Nov 7, 2017
@joeyparrish joeyparrish added type: bug Something isn't working correctly and removed type: question A question from the community labels Nov 7, 2017
@joeyparrish joeyparrish added this to the v2.3.0 milestone Nov 7, 2017
@joeyparrish joeyparrish self-assigned this Nov 7, 2017
@tjber
Copy link
Author

tjber commented Nov 8, 2017

Hi @joeyparrish thank you! I´ll try with playRangeStart/playRangeEnd and splitting each period in one manifest, not ideal but quite better than transcode :)

@joeyparrish
Copy link
Member

I hope that unblocks you.

In the mean time, though, there does seem to be a bug on our side. We should be instructing MediaSource to throw out any content that hangs off the beginning of the Period, and we are not.

When I start adding that to our code, I still see some strange behavior, and I'm not sure if it's a Chrome bug or a Shaka bug. Still working to track that down.

@joeyparrish
Copy link
Member

I've put together a non-Shaka demo that shows MediaSource doing the right thing with this keyframe-only content, so any issues I'm having are issues with Shaka Player.

1098.zip

@joeyparrish
Copy link
Member

I've noticed two things that are wrong with your demo content:

  1. I think the second period presentationTimeOffset should be 312, not 212
  2. The timescale in the manifest does not match the timescale in the media, which is throwing off our SIDX parser

I will need to consult with the DASH spec and IOP to understand whether or not the timescales are supposed to match. If so, the content is wrong. If not, the player is wrong.

@joeyparrish
Copy link
Member

The DASH spec says of timescale:

NOTE: This may be any frequency but typically is the media clock frequency of one of the media streams (or a positive integer multiple thereof).

So we should not assume that it matches the timescale of the container.

@joeyparrish
Copy link
Member

With the following changes, I have your all-keyframes content working as I think it is intended:

  1. Edit the manifest to set presentationTimeOffset in second period to 312 instead of 212
  2. Use appendWindowStart in MediaSource to avoid replacing period 1 media with period 2 media
  3. Fix segment index parsers such that they do not assume a consistent timescale between media & manifest

You can expect fixes for 2 & 3 to be published in the near future.

shaka-bot pushed a commit that referenced this issue Nov 9, 2017
According to the DASH spec, the timescale in the manifest need not match the
timescale in the media.  Therefore, we should be applying scaled
presentationTimeOffsets in segment index parsers, since the two scales might
differ.

Issue #1098

Change-Id: Ic191d1bba399b30a656ab5060d7bb226e659b79a
shaka-bot pushed a commit that referenced this issue Nov 10, 2017
When shifting content using presentationTimeOffset, especially in
combination with SegmentBase and media segment indexes, there can
legitimately be segments which are outside the period bounds.

Instead of failing an assertion, throw out the unneeded segments.

This also drops some largely unnecessary and confusing warnings.

Issue #1098

Change-Id: I2addd6d45f1aaf95a1b981cd9373dd24163c13a9
@joeyparrish
Copy link
Member

I updated the nightly build with the fixes for this. You can now see a demo of your content here:

https://nightly-dot-shaka-player-demo.appspot.com/demo/#asset=//storage.googleapis.com/shaka-demo-assets/_bugs/1098-fixed/dash_allkeyframes.mpd;lang=en-US;play

Is this what you were going for?

@tjber
Copy link
Author

tjber commented Nov 13, 2017

Hi @joeyparrish
I'm sorry for the delay in answering. I've been out for a few days.
Thank you very much! that was exactly the behavior I expected.
I'll try to get the "allkeyframe" files sent to me to avoid the initial jump.
Thank you so much.

@joeyparrish
Copy link
Member

We are always happy to help. It took some time to understand what was happening, but it turned out to be an important bug. So thank you for the report!

joeyparrish added a commit that referenced this issue Nov 13, 2017
According to the DASH spec, the timescale in the manifest need not match the
timescale in the media.  Therefore, we should be applying scaled
presentationTimeOffsets in segment index parsers, since the two scales might
differ.

Issue #1098

Change-Id: Ic191d1bba399b30a656ab5060d7bb226e659b79a
joeyparrish added a commit that referenced this issue Nov 13, 2017
When shifting content using presentationTimeOffset, especially in
combination with SegmentBase and media segment indexes, there can
legitimately be segments which are outside the period bounds.

Instead of failing an assertion, throw out the unneeded segments.

This also drops some largely unnecessary and confusing warnings.

Issue #1098

Change-Id: I2addd6d45f1aaf95a1b981cd9373dd24163c13a9
joeyparrish added a commit that referenced this issue Nov 13, 2017
This avoids having media from one period replaced by media from the
next period.  Instead, media that comes before the period start will
be chopped off by MediaSource.

Closes #1098

Change-Id: Idf6dc2ffafe78214e94bc75aca63920e153f1a2c
@joeyparrish
Copy link
Member

Fixes cherry-picked to v2.2.6.

@shaka-project shaka-project locked and limited conversation to collaborators Mar 22, 2018
@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Apr 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Projects
None yet
Development

No branches or pull requests

4 participants