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

7.4.1 result.firstKeyFrame is null #5888

Closed
wants to merge 12 commits into from
Closed

7.4.1 result.firstKeyFrame is null #5888

wants to merge 12 commits into from

Conversation

naiveliang
Copy link

Description

When I was using video.js with version of 7.4.1, I found a bug at the line 37806, like the following:
37804 if (probe$2.ts.videoPacketContainsKeyFrame(frame)) {
37805 result.firstKeyFrame = probe$2.ts.parsePesTime(frame);
37806 result.firstKeyFrame.type = 'video';
37807 }

The return value of 'probe$2.ts.parsePesTime(frame)' may be null, so 'result.firstKeyFrame.type' will throw an error, like this: Uncaught TypeError: Cannot set property 'type' of null.

Specific Changes proposed

To solve this error, I add a code before line 37806, like the following:
37804 if (probe$2.ts.videoPacketContainsKeyFrame(frame)) {
37805 result.firstKeyFrame = probe$2.ts.parsePesTime(frame);
37806 if(result.firstKeyFrame==null) result.firstKeyFrame = {};
37807 result.firstKeyFrame.type = 'video';
37808 }

This is able to solve my problem.

smbea and others added 12 commits January 8, 2019 15:10
Before the fullscreen API was un-prefixed, we listened to triggered a
`fullscreenchange` event on the player manually. This worked fine
because a prefixed `fullscreenchange` event was triggered on the player
element itself. But when it was unprefixed, we ended up with two events
now, the unprefixed event and the one we triggered.

Instead, we should listen the event the browser supports and re-trigger
it as the unprefixed event as necessary.

We also make sure that the handler gets calls when the document level
fullscreenchange handler gets called so that it is cancelled properly.

Fixes #5685.
Whenever we seek after the video has ended, we are no longer ended and
therefore we should remove the vjs-ended class.

Fixes #5654.
…5721)

This was done to make the behavior on Android with HLS live streams better but the it's opening us up to too many potential issues, like a user not being able to properly disable the control bar, that we should just back it out.
…ild (#5702)

A child component may have been assigned to another
parent before assigning that child component to the
new parent via "addChild" method. In this case, the
original parent should remove the child then it can
be safely added back to the new parent. This commit
will keep the parent's "children_" and its DOM
element's child nodes in the consistent state.
This reverts the previous fix (2bc90a1)
and also simplifies handling. Seems like this works a lot better while
also ensuring that the vjs-fullscreen class is still on the player.

Fixes #5685, fixes #5745
…ders (#5754)

The ResizeManager's iframe element is able to be focused via tabbing, which results in a bad user experience for users that rely on a screen reader to navigate the video and its sibling elements. The fix is to set the tabIndex to "-1", and the aria-hidden property to true for good measure.
@welcome
Copy link

welcome bot commented Mar 23, 2019

💖 Thanks for opening this pull request! 💖

Things that will help get your PR across the finish line:

  • Run npm run lint -- --errors locally to catch formatting errors earlier.
  • Include tests when adding/changing behavior.
  • Include screenshots and animated GIFs whenever possible.

We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can.

@brandonocasey
Copy link
Contributor

brandonocasey commented Mar 28, 2019

Thanks for bringing this issue to our attention. The code that you referenced does not actually live in this repo (even though we ship it with Video.js). It lives here (i think): https://github.com/videojs/mux.js/blob/master/lib/tools/ts-inspector.js#L222

in the https://github.com/videojs/mux.js repo

That would be were you want to submit your fix, and not here in the video.js repo.

EDIT: I looked through the issues. There does seem to be a pull request with a similar fix to your code: videojs/mux.js#251

@gkatsev
Copy link
Member

gkatsev commented Mar 28, 2019

This should've been an issue rather than PR. However, looks like an issue already exists (videojs/mux.js#250) and there's a pending PR for it as well: videojs/mux.js#251

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.

7 participants