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

Failing Transmuxer test on Safari #7462

Open
joeyparrish opened this issue Oct 21, 2024 · 6 comments
Open

Failing Transmuxer test on Safari #7462

joeyparrish opened this issue Oct 21, 2024 · 6 comments
Labels
browser: Safari Issues affecting Safari or WebKit derivatives component: transmuxer The issue involves our built-in transmuxer flag: seeking PR We are actively seeking PRs for this; we do not currently expect the core team will resolve this priority: P1 Big impact or workaround impractical; resolve before feature release type: bug Something isn't working correctly
Milestone

Comments

@joeyparrish
Copy link
Member

Have you read the FAQ and checked for duplicate open issues?
Yes

If the problem is related to FairPlay, have you read the tutorial?

N/A

What version of Shaka Player are you using?

main

Can you reproduce the issue with our latest release version?
Yes

Can you reproduce the issue with the latest code from main?
Yes

Are you using the demo app or your own custom app?
N/A: Unit & integration tests

If custom app, can you reproduce the issue using our demo app?
N/A: Unit & integration tests

What browser and OS are you using?
Safari (currently 18.0.1) on macos-latest VMs on GitHub (currently 10.15.7, Arm)

For embedded devices (smart TVs, etc.), what model and firmware version are you using?
N/A

What are the manifest and license server URIs?

N/A: Unit & integration tests

What configuration are you using? What is the output of player.getNonDefaultConfiguration()?

N/A: Unit & integration tests

What did you do?

Run tests

What did you expect to happen?
Old transmuxer test passes

What actually happened?

Fails with:

Error: Timeout waiting for movement from 1.197910122000073 to 6

current time: 2.32
duration: 18
ready state: 3
playback rate: 3
paused: false
ended: false

buffered: {
  "total": [
    {
      "start": 0,
      "end": 2.08
    },
    {
      "start": 2.32,
      "end": 4.88
    },
    {
      "start": 5.12,
      "end": 11.04
    },
    {
      "start": 11.28,
      "end": 13.52
    }
  ],
  "audio": [
    {
      "start": 0,
      "end": 14.272
    }
  ],
  "video": [
    {
      "start": 0,
      "end": 2.08
    },
    {
      "start": 2.32,
      "end": 4.88
    },
    {
      "start": 5.12,
      "end": 11.04
    },
    {
      "start": 11.28,
      "end": 13.52
    }
  ],
  "text": []
}

As you can see from the buffered ranges, there is a gap of 0.24 seconds between each segment.

I need to dig deeper, but any of these could be true:

  1. Safari interprets the transmuxer output differently, seeing gaps that other browsers don't
  2. There are gaps in the output no matter what, but only Safari refuses to play through them
  3. Probably other possibilities I haven't thought of yet...

I think it's possible that this is a legitimate failure that indicates a real bug, and not just a flaky test. I haven't proven that yet, it's just a gut feeling.

Are you planning send a PR to fix it?
Yes, if I can find the root cause. I would welcome additional help in investigating and in fixing!

@joeyparrish joeyparrish added type: bug Something isn't working correctly flag: seeking PR We are actively seeking PRs for this; we do not currently expect the core team will resolve this priority: P1 Big impact or workaround impractical; resolve before feature release browser: Safari Issues affecting Safari or WebKit derivatives component: transmuxer The issue involves our built-in transmuxer labels Oct 21, 2024
@shaka-bot shaka-bot added this to the v4.12 milestone Oct 21, 2024
@avelad
Copy link
Member

avelad commented Oct 22, 2024

imagen

@joeyparrish
Copy link
Member Author

@avelad sent me this additional information:

With https://cdn.theoplayer.com/video/adultswim2/clip.m3u8 you can reproduce the same error. It seems that when content is muxed we somehow generate the PTS wrong, but only in the video! In hljs.js it works fine but fails in Shaka. I have compared the segments and the mdat is exactly the same, and the sample data too. The only thing that changes is the baseMediaDecodeTime.

@joeyparrish
Copy link
Member Author

joeyparrish commented Oct 31, 2024

The first 46 frames in the first segment are non-keyframes. That's the only peculiarity I've found so far. The gap at the beginning of the buffered range corresponds to the PTS of the first keyframe. (These data are from the theoplayer URL in the comment above, not the canned data in the failing test case.)

@joeyparrish
Copy link
Member Author

joeyparrish commented Oct 31, 2024

Just realized why the test case that's failing on Safari isn't failing everywhere.

2) H.264+AC3 in TS
     Transmuxer Player for muxed content

We don't have native AC3 support in most browsers. So the tests gets skipped on other browsers.

@joeyparrish
Copy link
Member Author

In the content used in our test case, the first segment starts with a keyframe, but the second does not, and many others do not as well. DTS==PTS in every single frame, so there are no b-frames in that content, nor any chance for us to mistakenly use DTS where we should use PTS or vice-versa.

The first gap position (2.08) aligns perfectly with the first frame of the second segment, which is not a keyframe.

So the two clips (our test case and the Theoplayer clip) would seem to have some things in common (not every segment starts with a keyframe) and others things differ (Theo's has b-frames).

joeyparrish added a commit to joeyparrish/shaka-player that referenced this issue Oct 31, 2024
This is generating too much noise, and we do not yet know how to fix it.

Issue shaka-project#7462
@joeyparrish
Copy link
Member Author

I'm giving up for now. I don't know what to do with this information. I'm hopeful that someone else can make progress with this as a starting place.

joeyparrish added a commit that referenced this issue Nov 1, 2024
This is generating too much noise, and we do not yet know how to fix it.

Issue #7462
avelad pushed a commit that referenced this issue Nov 12, 2024
This is generating too much noise, and we do not yet know how to fix it.

Issue #7462
avelad pushed a commit that referenced this issue Nov 12, 2024
This is generating too much noise, and we do not yet know how to fix it.

Issue #7462
joeyparrish added a commit that referenced this issue Nov 12, 2024
This is generating too much noise, and we do not yet know how to fix it.

Issue #7462
@avelad avelad modified the milestones: v4.12, v4.13 Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
browser: Safari Issues affecting Safari or WebKit derivatives component: transmuxer The issue involves our built-in transmuxer flag: seeking PR We are actively seeking PRs for this; we do not currently expect the core team will resolve this priority: P1 Big impact or workaround impractical; resolve before feature release type: bug Something isn't working correctly
Projects
None yet
Development

No branches or pull requests

3 participants