-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
feat: implement startPosition #3355
feat: implement startPosition #3355
Conversation
### `contentStartTime` | ||
The start time in ms for SSAI content. This determines at what time to load the video info like resolutions. Use this only when you have SSAI stream where ads resolution is not the same as content resolution. | ||
|
||
Platforms: Android, iOS | ||
Platforms: Android |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch !
Point 1: this change should test with source.startTime this change should test with SSAI: -> I agree Point 2: Can I propose you to move this property inside the source object ?
the background of this request is also the real time issue your are pointing in the media3 PR. the postDelayed is necessary to ensure all props are well applied. If we correctly want to remove this delay when starting the stream, all props necessary to start the playback shall be stored in the same property. Let me know if this is not clear ! (else the code looks good to me!) |
I agree (I'll make sure to include that in the documentation as well)
Unfortunately, I don't have any sample to test
I deeply agree (I think DRM should be included for the same reason (all props necessary to start the playback shall be stored in the same property.)) I agree with all your points, but I think So, while it might be a breaking change, I was wondering what you think of the following modification. source={{
uri: "..."
startPosition: 0,
crop: {
startTime: 0,
endTime: 0
}
}} |
You are right...
Yes ! to be follow up in: #2693 |
- put startPosition inside source prop - rename existing prop (startTime, endTime)
docs/pages/component/props.md
Outdated
#### Start playback at a specific point in time | ||
|
||
Provide an optional `startPosition` for video. Value is in milliseconds. If the `cropStart` prop is applied, it will be applied from that point forward. | ||
(If it is zero or negative, it is ignored) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should not be ignored if set to 0 !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right! I'll fix it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to handle 0 as a seek position.
Some video (like live streams) which starts to the end of playback by default.
I think this prop should be also applied in that case.
To be exact, on my product there is a "restart" option on live stream.
It take the EPG and create a kind of VOD with it. This VOD has a close start (always start at the same point), but an open end (video chuck are added according to live updates).
In taht case, exoplayer consider this video as a live stream, thus it starts the video at the end, where, when customer client on restart, he wants video to start from the beggining.
(And thank you for the rework of the prop)
"time": NSNumber(value: _startPosition), | ||
"tolerance": NSNumber(value: 100) | ||
]) | ||
_startPosition = -1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it really need to force _startPosition to -1 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was assuming that the playback would work fine, then an error would occur, and then it would work again.
When the source is unchanged and the lifecycle is as follows,
(1) readyToPlay → (2) failed → (3) readyToPlay
I forced it to -1
because I would expect startPosition
to not work in state (3).
But I'm not very experienced with iOS, and I don't know much about it. If my assumptions are wrong, feel free to let me know and I'll correct above code.
android/src/main/java/com/brentvatne/exoplayer/ReactExoplayerView.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
I am wondering what if we do videofile imports through require, and still need to use cropstart/end feature, in that case we cannot directly get access to uri of that file. Maybe it will be better if you consider making it more flexible? |
You are right! I think we should allow (or move ) the require usage inside uri prop of the source! |
Update the changelog
startPosition
prop (defined within source prop)source.startTime
have been renamed tosource.cropStart
source.endTime
have been renamed tosource.cropEnd
Provide an example of how to test the change
source.cropStart
source.cropStart
prop is applied, it will be applied from that point forward.Describe the changes
startPosition
prop (ignore zero or negative numbers)