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

Add frame step buttons to internal video player #1207

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -806,6 +806,11 @@ public static boolean pref_behaviour_video_playback_controls() {
false);
}

public static boolean pref_behaviour_video_frame_step() {
return getBoolean(R.string.pref_behaviour_video_frame_step_key,
false);
}

public static boolean pref_behaviour_video_mute_default() {
return getBoolean(
R.string.pref_behaviour_video_mute_default_key,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ public ExoPlayerWrapperView(
addButton(createButton(
context,
mControlView,
R.drawable.icon_previous,
R.drawable.icon_restart,
R.string.video_restart,
view -> {
mVideoPlayer.seekTo(0);
Expand Down Expand Up @@ -188,7 +188,70 @@ public ExoPlayerWrapperView(
updateProgress();
}));

addButton(playButton.get(), buttons);
if (PrefsUtility.pref_behaviour_video_frame_step()) {
final AtomicReference<ImageButton> stepBackButton = new AtomicReference<>();
final AtomicReference<ImageButton> stepForwardButton = new AtomicReference<>();
final long frameRate = mVideoPlayer.getVideoFormat() != null
? 1000 / (long) mVideoPlayer.getVideoFormat().frameRate
: 33;

stepBackButton.set(createButton(
context,
mControlView,
R.drawable.icon_step_back,
R.string.video_step_back,
view -> {
mVideoPlayer.seekTo(mVideoPlayer.getCurrentPosition() - frameRate);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is the correct way to do it? Frame rate is a number of frames in a second so you need to calculate 1000 / frameRate to get duration of a frame in milliseconds

Copy link
Author

@ecawthorne ecawthorne Sep 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're absolutely right, I'm not sure what I was thinking there. I also unintentionally inverted the null check which hid a different issue: apparently getVideoFormat().framerate is always null in this context. After looking into it a bit, it seems like it's due to the format being H.264/AVC which ExoPlayer doesn't supply a frame rate for.

The condition is fixed in 8e0efb3. If you have any suggestions on how to proceed for calculating the frame rate just let me know. I'm working on the assumption that there may be a format change at some point in the future so I kept the frame rate check there for now.

updateProgress();
}
));

stepForwardButton.set(createButton(
context,
mControlView,
R.drawable.icon_step_forward,
R.string.video_step_forward,
view -> {
mVideoPlayer.seekTo(mVideoPlayer.getCurrentPosition() + frameRate);
updateProgress();
}
));

mVideoPlayer.addListener(new Player.Listener() {
@Override
public void onIsPlayingChanged(final boolean isPlaying) {
if (isPlaying) {
stepBackButton.get().setImageAlpha(0x3F);
stepBackButton.get().setContentDescription(
context.getString(R.string.video_step_back_disabled));
stepBackButton.get().setEnabled(false);

stepForwardButton.get().setImageAlpha(0x3F);
stepForwardButton.get().setContentDescription(
context.getString(R.string.video_step_forward_disabled));
stepForwardButton.get().setEnabled(false);

} else {
stepBackButton.get().setImageAlpha(0xFF);
stepBackButton.get().setContentDescription(
context.getString(R.string.video_step_back));
stepBackButton.get().setEnabled(true);

stepForwardButton.get().setImageAlpha(0xFF);
stepForwardButton.get().setContentDescription(
context.getString(R.string.video_step_forward));
stepForwardButton.get().setEnabled(true);
}
}
});

addButton(stepBackButton.get(), buttons);
addButton(playButton.get(), buttons);
addButton(stepForwardButton.get(), buttons);

} else {
addButton(playButton.get(), buttons);
}
}

addButton(createButton(
Expand Down
26 changes: 26 additions & 0 deletions src/main/res/drawable/icon_restart.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<!-- Copyright (C) 2024 The Android Open Source Project

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
-->

<vector xmlns:android="http://schemas.android.com/apk/res/android"
android:width="32dp"
android:height="32dp"
android:viewportHeight="24.0"
android:viewportWidth="24.0">

<path
android:fillColor="#FFFFFFFF"
android:pathData="M12,4C14.1,4 16.1,4.8 17.6,6.3C20.7,9.4 20.7,14.5 17.6,17.6C15.8,19.5 13.3,20.2 10.9,19.9L11.4,17.9C13.1,18.1 14.9,17.5 16.2,16.2C18.5,13.9 18.5,10.1 16.2,7.7C15.1,6.6 13.5,6 12,6V10.6L7,5.6L12,0.6V4M6.3,17.6C3.7,15 3.3,11 5.1,7.9L6.6,9.4C5.5,11.6 5.9,14.4 7.8,16.2C8.3,16.7 8.9,17.1 9.6,17.4L9,19.4C8,19 7.1,18.4 6.3,17.6Z"/>

</vector>
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<!-- Copyright (C) 2017 The Android Open Source Project
<!-- Copyright (C) 2024 The Android Open Source Project

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand All @@ -12,14 +12,17 @@
See the License for the specific language governing permissions and
limitations under the License.
-->

<vector xmlns:android="http://schemas.android.com/apk/res/android"
android:width="32dp"
android:height="32dp"
android:viewportHeight="24.0"
android:viewportWidth="24.0">
android:height="32dp"
android:viewportHeight="24.0"
android:viewportWidth="24.0">

<path
android:fillColor="#FFFFFFFF"
android:pathData="M6,6h2v12L6,18zM9.5,12l8.5,6L18,6z"/>
<path
android:fillColor="#FFFFFFFF"
android:pathData="M6,18V6H8V18H6M9.5,12L18,6V18L9.5,12Z"/>

</vector>


26 changes: 26 additions & 0 deletions src/main/res/drawable/icon_step_forward.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<!-- Copyright (C) 2024 The Android Open Source Project

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
-->

<vector xmlns:android="http://schemas.android.com/apk/res/android"
android:width="32dp"
android:height="32dp"
android:viewportHeight="24.0"
android:viewportWidth="24.0">

<path
android:fillColor="#FFFFFFFF"
android:pathData="M16,18H18V6H16M6,18L14.5,12L6,6V18Z"/>

</vector>
9 changes: 9 additions & 0 deletions src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1490,9 +1490,13 @@
<string name="video_unmute">Unmute</string>
<string name="video_restart">Restart video</string>
<string name="video_rewind">Rewind</string>
<string name="video_step_back">Step back</string>
<string name="video_step_back_disabled">Step back disabled</string>
<string name="video_play">Play</string>
<string name="video_pause">Pause</string>
<string name="video_fast_forward">Fast forward</string>
<string name="video_step_forward">Step forward</string>
<string name="video_step_forward_disabled">Step forward disabled</string>
<string name="video_zoom_in">Zoom in</string>
<string name="video_zoom_out">Zoom out</string>

Expand Down Expand Up @@ -1894,4 +1898,9 @@

<string name="error_invalid_account_title">Invalid account</string>
<string name="error_invalid_account_message">Selected account is not currently logged in</string>

<!-- 2024-07-11 -->
<string name="pref_behaviour_video_frame_step_title">Enable stepping frame by frame</string>
<string name="pref_behaviour_video_frame_step_key" translatable="false">pref_behaviour_video_frame_step</string>

</resources>
4 changes: 4 additions & 0 deletions src/main/res/xml/prefs_images_video.xml
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@
android:key="@string/pref_behaviour_video_playback_controls_key"
android:defaultValue="false"/>

<CheckBoxPreference android:title="@string/pref_behaviour_video_frame_step_title"
android:key="@string/pref_behaviour_video_frame_step_key"
android:defaultValue="false"/>

<CheckBoxPreference android:title="@string/pref_behaviour_video_mute_default_title"
android:key="@string/pref_behaviour_video_mute_default_key"
android:defaultValue="true"/>
Expand Down