[AV] Correct Playback interface so that it contains all common method… #6213

mczernek · 2019-11-08T09:35:15Z

Why

Documentation includes method for Video reference, which is not available since at least sdk32. Some users reports this inconsistence.

How

Updating documentation and updating Playback interface for Audio and Video objects.

…s. Update documentation to include differences between videoRef and Sound objects.

…:expo/expo into @mczernek/av-playback-interface-and-docs

sjchmiela

On the other hand, maybe we should just expose a setOnPlaybackUpdateStatus method from Video.tsx (which shouldn't be too difficult, we would just save the callback as an instance variable and call in _handleNewStatus?

sjchmiela · 2019-11-08T09:48:40Z

packages/expo-av/src/Video.tsx

@@ -314,6 +314,7 @@ export default class Video extends React.Component<VideoProps, VideoState> imple
  setProgressUpdateIntervalAsync!: (
    progressUpdateIntervalMillis: number
  ) => Promise<PlaybackStatus>;
+  setOnPlaybackUpdateStatus!: (progressUpdateIntervalMillis: number) => Promise<PlaybackStatus>;


There's no other setOnPlaybackUpdateStatus anywhere in the repo, are you sure this is the name you wanted to use?

Should there even be this method here? It isn't set by PlaybackMixin… 🤔

No, is should not. Will delete.

sjchmiela · 2019-11-08T09:55:44Z

packages/expo-av/src/AV.ts

@@ -250,14 +254,14 @@ export interface Playback extends AV {
 */
 export const PlaybackMixin = {
  async playAsync(): Promise<PlaybackStatus> {
-    return ((this as any) as AV).setStatusAsync({ shouldPlay: true });
+    return ((this as any) as Playback).setStatusAsync({ shouldPlay: true });


Why would we change AV to Playback, so make the required interface more concrete? (While AV would suffice?)

My idea was, that we really require this to implement Playback interface, and PlaybackMixin is designed to add implementation to some methods.
So it is sufficient, but would be logically wrong to pass here AV implementation. I know that this does not guard us anyhow, but I thought that this way our intentions are expressed more clearly.

docs/pages/versions/v35.0.0/sdk/video.md

sjchmiela · 2019-11-08T10:00:26Z

docs/pages/versions/v35.0.0/sdk/av.md

 ...
 ```

-#### Example: Loop media exactly 20 times
+#### Example: Loop media exactly 20 times for Sound


Suggested change

#### Example: Loop media exactly 20 times for Sound

#### Example: Loop sound exactly 20 times

sjchmiela · 2019-11-08T10:00:52Z

docs/pages/versions/v35.0.0/sdk/av.md

@@ -351,11 +355,11 @@ _onPlaybackStatusUpdate = playbackStatus => {
 };

 ... // Load the playbackObject and obtain the reference.


Suggested change

... // Load the playbackObject and obtain the reference.

... // Load the sound and obtain the reference.

Co-Authored-By: Stanisław Chmiela <sjchmiela@users.noreply.github.com>

brentvatne · 2019-11-14T22:58:51Z

docs/pages/versions/v35.0.0/sdk/video.md

@@ -184,16 +184,14 @@ This dismisses the fullscreen video view.

 A `Promise` that is fulfilled with the `PlaybackStatus` of the video once the fullscreen player has finished dismissing, or rejects if there was an error, or if this was called on an Android device.

-The rest of the API on the `Video` component ref is the same as the API for `Audio.Sound`-- see the [AV documentation](../av/) for further information:
+The rest of the API on the `Video` component ref is almost the same as the API for `Audio.Sound`. Only exception is - `setOnPlaybackStatusUpdate(onPlaybackStatusUpdate)` method, which is not available for `videoRef` -- see the [AV documentation](../av/) for further information:


Suggested change

The rest of the API on the `Video` component ref is almost the same as the API for `Audio.Sound`. Only exception is - `setOnPlaybackStatusUpdate(onPlaybackStatusUpdate)` method, which is not available for `videoRef` -- see the [AV documentation](../av/) for further information:

The rest of the API on the `Video` component ref is almost the same as the API for `Audio.Sound`. The only exception is the `setOnPlaybackStatusUpdate(onPlaybackStatusUpdate)` method, which is not available for `videoRef` -- see the [AV documentation](../av/) for further information:

brentvatne · 2019-11-14T23:00:46Z

docs/pages/versions/unversioned/sdk/av.md

@@ -64,7 +64,7 @@ render() {

 ## Playback API

-On the `playbackObject` reference, the following API is provided:
+On the `playbackObject` reference, the following API is provided for both Audio and Video:


Suggested change

On the `playbackObject` reference, the following API is provided for both Audio and Video:

On the `playbackObject` reference, the following API is provided for both `Audio` and `Video`:

mczernek · 2019-11-27T12:53:41Z

On the other hand, maybe we should just expose a setOnPlaybackUpdateStatus method from Video.tsx (which shouldn't be too difficult, we would just save the callback as an instance variable and call in _handleNewStatus?

I guess we could do that, but I do not think we want to do it backward. What about just updating docs for now, and creating another issue for adding this method?

sjchmiela

Thanks! 👏

rusakovic · 2020-04-11T14:04:10Z

@mczernek
Helo Michał!
I tried to solve video issues several months. Maybe you can suggest correct manual for AV.
How to loadAsync, unloadAsync, and pause/play correctly with useEffect, useState, useRef hooks. Because currently official manual is super outdated.

Currently, I opened this ticket. #7446

I'm ready to help!
Thank you!

expo#6213) * [AV] Correct Playback interface so that it contains all common methods. Update documentation to include differences between videoRef and Sound objects. * [AV] Correct Playback interface so that it contains all common methods. Update documentation to include differences between videoRef and Sound objects. * Build files. * Update docs/pages/versions/v35.0.0/sdk/video.md Co-Authored-By: Stanisław Chmiela <sjchmiela@users.noreply.github.com> * Correct docs. Reove unavailable method from Video.tsx. * [AV] Add on update callback to video ref object. * Changelog.

[AV] Correct Playback interface so that it contains all common method…

4a24fef

…s. Update documentation to include differences between videoRef and Sound objects.

mczernek requested review from brentvatne and tsapeta November 8, 2019 09:35

mczernek requested a review from sjchmiela as a code owner November 8, 2019 09:35

mczernek added 3 commits November 8, 2019 10:38

[AV] Correct Playback interface so that it contains all common method…

41136e3

…s. Update documentation to include differences between videoRef and Sound objects.

Merge branch '@mczernek/av-playback-interface-and-docs' of github.com…

92da418

…:expo/expo into @mczernek/av-playback-interface-and-docs

Build files.

3ae3979

sjchmiela reviewed Nov 8, 2019

View reviewed changes

Update docs/pages/versions/v35.0.0/sdk/video.md

c3473fc

Co-Authored-By: Stanisław Chmiela <sjchmiela@users.noreply.github.com>

brentvatne reviewed Nov 14, 2019

View reviewed changes

sjchmiela added the Launch-blocking label Nov 22, 2019

tsapeta removed the Launch-blocking label Nov 25, 2019

Correct docs. Reove unavailable method from Video.tsx.

71993af

[AV] Add on update callback to video ref object.

a7cd4f5

mczernek requested a review from bbarthec as a code owner December 12, 2019 17:02

mczernek added 2 commits December 12, 2019 18:02

Merge branch 'master' into @mczernek/av-playback-interface-and-docs

1c621d1

Changelog.

08f65f9

mczernek requested a review from sjchmiela December 16, 2019 14:23

sjchmiela approved these changes Dec 16, 2019

View reviewed changes

Merge branch 'master' into @mczernek/av-playback-interface-and-docs

dfc17a1

mczernek merged commit 76ddcce into master Dec 17, 2019

mczernek deleted the @mczernek/av-playback-interface-and-docs branch December 17, 2019 10:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[AV] Correct Playback interface so that it contains all common method… #6213

[AV] Correct Playback interface so that it contains all common method… #6213

mczernek commented Nov 8, 2019

sjchmiela left a comment

sjchmiela Nov 8, 2019

sjchmiela Nov 8, 2019

mczernek Nov 27, 2019

sjchmiela Nov 8, 2019

mczernek Nov 27, 2019

sjchmiela Nov 8, 2019

sjchmiela Nov 8, 2019

brentvatne Nov 14, 2019

brentvatne Nov 14, 2019

mczernek commented Nov 27, 2019

sjchmiela left a comment

rusakovic commented Apr 11, 2020 •

edited

Loading

	#### Example: Loop media exactly 20 times for Sound
	#### Example: Loop sound exactly 20 times

		@@ -351,11 +355,11 @@ _onPlaybackStatusUpdate = playbackStatus => {
		};

		... // Load the playbackObject and obtain the reference.

	... // Load the playbackObject and obtain the reference.
	... // Load the sound and obtain the reference.

	The rest of the API on the `Video` component ref is almost the same as the API for `Audio.Sound`. Only exception is - `setOnPlaybackStatusUpdate(onPlaybackStatusUpdate)` method, which is not available for `videoRef` -- see the [AV documentation](../av/) for further information:
	The rest of the API on the `Video` component ref is almost the same as the API for `Audio.Sound`. The only exception is the `setOnPlaybackStatusUpdate(onPlaybackStatusUpdate)` method, which is not available for `videoRef` -- see the [AV documentation](../av/) for further information:

	On the `playbackObject` reference, the following API is provided for both Audio and Video:
	On the `playbackObject` reference, the following API is provided for both `Audio` and `Video`:

[AV] Correct Playback interface so that it contains all common method… #6213

[AV] Correct Playback interface so that it contains all common method… #6213

Conversation

mczernek commented Nov 8, 2019

Why

How

sjchmiela left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mczernek commented Nov 27, 2019

sjchmiela left a comment

Choose a reason for hiding this comment

rusakovic commented Apr 11, 2020 • edited Loading

rusakovic commented Apr 11, 2020 •

edited

Loading