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

Media elements improvements #8203

Merged
merged 16 commits into from Mar 13, 2024
9 changes: 7 additions & 2 deletions e2e_playwright/st_audio.py
Expand Up @@ -16,6 +16,11 @@

import streamlit as st

url = "https://www.w3schools.com/html/horse.ogg"
file = requests.get(url).content
url1 = "https://www.w3schools.com/html/horse.ogg"
file = requests.get(url1).content
st.audio(file)

url2 = "https://mdn.github.io/learning-area/html/multimedia-and-embedding/video-and-audio-content/viper.mp3"
st.audio(url2, start_time=10, end_time=13)

st.audio(url2, start_time=15, end_time=19, loop=True)
45 changes: 41 additions & 4 deletions e2e_playwright/st_audio_test.py
Expand Up @@ -15,9 +15,46 @@

from playwright.sync_api import Page, expect

from e2e_playwright.conftest import wait_until


def test_audio_has_correct_properties(app: Page):
audio = app.get_by_test_id("stAudio")
expect(audio).to_be_visible()
expect(audio).to_have_attribute("controls", "")
expect(audio).to_have_attribute("src", re.compile(r".*media.*wav"))
"""Test that `st.audio` renders correct properties."""
audio_elements = app.get_by_test_id("stAudio")
expect(audio_elements).to_have_count(3)

expect(audio_elements.nth(0)).to_be_visible()
expect(audio_elements.nth(0)).to_have_attribute("controls", "")
expect(audio_elements.nth(0)).to_have_attribute("src", re.compile(r".*media.*wav"))


def test_audio_end_time(app: Page):
"""Test that `st.audio` end_time property works correctly."""
audio_elements = app.get_by_test_id("stAudio")
expect(audio_elements).to_have_count(3)

expect(audio_elements.nth(1)).to_be_visible()

audio_element = audio_elements.nth(1)
audio_element.evaluate("el => el.play()")
app.wait_for_timeout(5000)
expect(audio_element).to_have_js_property("paused", True)
wait_until(app, lambda: int(audio_element.evaluate("el => el.currentTime")) == 13)


def test_audio_end_time_loop(app: Page):
"""Test that `st.audio` end_time and loop properties work correctly."""
audio_elements = app.get_by_test_id("stAudio")
expect(audio_elements).to_have_count(3)

expect(audio_elements.nth(2)).to_be_visible()

audio_element = audio_elements.nth(2)
audio_element.evaluate("el => el.play()")
# The corresponding element definition looks like this:
# st.audio(url2, start_time=15, end_time=19, loop=True)
# We wait for 6 seconds, which mean the current time should be
# approximately 17 (4 seconds until end_time and 2 seconds starting from start time)
app.wait_for_timeout(6000)
expect(audio_element).to_have_js_property("paused", False)
wait_until(app, lambda: 16 < audio_element.evaluate("el => el.currentTime") < 18)
21 changes: 21 additions & 0 deletions e2e_playwright/st_video.py
Expand Up @@ -46,3 +46,24 @@
"Deutsch": "test_assets/sintel-de.vtt",
},
)


# Test end time webm video
st.video(
"test_assets/sintel-short.webm",
start_time=31,
end_time=33,
)

# Test end time mp4 video
st.video(
"test_assets/sintel-short.mp4",
start_time=31,
end_time=33,
)

# Test end time and loop webm video
st.video("test_assets/sintel-short.webm", start_time=35, end_time=39, loop=True)

# Test end time and loop mp4 video
st.video("test_assets/sintel-short.mp4", start_time=35, end_time=39, loop=True)
56 changes: 53 additions & 3 deletions e2e_playwright/st_video_test.py
Expand Up @@ -16,16 +16,17 @@
import pytest
from playwright.sync_api import Page, expect

from e2e_playwright.conftest import ImageCompareFunction
from e2e_playwright.conftest import ImageCompareFunction, wait_until

VIDEO_ELEMENTS_COUNT = 9

# Chromium miss codecs required to play that mp3 videos
# https://www.howtogeek.com/202825/what%E2%80%99s-the-difference-between-chromium-and-chrome/
@pytest.mark.skip_browser("chromium")
def test_video_rendering(app: Page, assert_snapshot: ImageCompareFunction):
"""Test that `st.video` renders correctly via screenshots matching."""
video_elements = app.get_by_test_id("stVideo")
expect(video_elements).to_have_count(5)
expect(video_elements).to_have_count(VIDEO_ELEMENTS_COUNT)

# Wait for the video to load
app.wait_for_timeout(2000)
Expand Down Expand Up @@ -53,7 +54,7 @@ def test_video_rendering(app: Page, assert_snapshot: ImageCompareFunction):
def test_video_rendering_webp(app: Page, assert_snapshot: ImageCompareFunction):
"""Test that `st.video` renders correctly webm video via screenshots matching."""
video_elements = app.get_by_test_id("stVideo")
expect(video_elements).to_have_count(5)
expect(video_elements).to_have_count(VIDEO_ELEMENTS_COUNT)

# Wait for the video to load
app.wait_for_timeout(2000)
Expand Down Expand Up @@ -95,3 +96,52 @@ def test_handles_changes_in_start_time(

video_elements = app.get_by_test_id("stVideo")
assert_snapshot(video_elements.nth(1), name="video-updated-start")


@pytest.mark.parametrize(
"nth_element",
[
pytest.param(5, marks=pytest.mark.skip_browser("webkit")),
Copy link
Collaborator

Choose a reason for hiding this comment

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

interesting :) didn't know this trick 👍

pytest.param(6, marks=pytest.mark.skip_browser("chromium")),
],
)
def test_video_end_time(app: Page, nth_element: int):
"""Test that `st.video` with end_time works correctly."""
video_elements = app.get_by_test_id("stVideo")
expect(video_elements).to_have_count(VIDEO_ELEMENTS_COUNT)

expect(video_elements.nth(nth_element)).to_be_visible()

video_element = video_elements.nth(nth_element)
video_element.scroll_into_view_if_needed()
video_element.evaluate("el => el.play()")
# Wait until video will reach end_time
app.wait_for_timeout(3000)
expect(video_element).to_have_js_property("paused", True)
wait_until(app, lambda: int(video_element.evaluate("el => el.currentTime")) == 33)


@pytest.mark.parametrize(
"nth_element",
[
pytest.param(7, marks=pytest.mark.skip_browser("webkit")),
pytest.param(8, marks=pytest.mark.skip_browser("chromium")),
],
)
def test_video_end_time_loop(app: Page, nth_element: int):
"""Test that `st.video` with end_time and loop works correctly."""
video_elements = app.get_by_test_id("stVideo")
expect(video_elements).to_have_count(VIDEO_ELEMENTS_COUNT)

expect(video_elements.nth(nth_element)).to_be_visible()

video_element = video_elements.nth(nth_element)
video_element.scroll_into_view_if_needed()
video_element.evaluate("el => el.play()")
# According to the element definition looks like this:
# start_time=35, end_time=39, loop=True
# We wait for 6 seconds, which mean the current time should be approximately 37:
# 4 seconds until end_time and 2 seconds starting from start time
app.wait_for_timeout(6000)
expect(video_element).to_have_js_property("paused", False)
wait_until(app, lambda: 36 < video_element.evaluate("el => el.currentTime") < 38)
82 changes: 80 additions & 2 deletions frontend/lib/src/components/elements/Audio/Audio.tsx
Expand Up @@ -31,11 +31,89 @@ export default function Audio({
}: AudioProps): ReactElement {
const audioRef = useRef<HTMLAudioElement>(null)

const { startTime, endTime, loop } = element

// Handle startTime changes
useEffect(() => {
if (audioRef.current) {
audioRef.current.currentTime = element.startTime
audioRef.current.currentTime = startTime
}
}, [startTime])

useEffect(() => {
const audioNode = audioRef.current

const setStartTime: () => void = () => {
if (audioNode) {
// setStartTime
audioNode.currentTime = element.startTime
}
}

if (audioNode) {
audioNode.addEventListener("loadedmetadata", setStartTime)
}

return () => {
if (audioNode) {
audioNode.removeEventListener("loadedmetadata", setStartTime)
}
}
}, [element])

// Stop the audio at 'endTime' and handle loop
useEffect(() => {
const audioNode = audioRef.current
if (!audioNode) return

// Flag to avoid calling 'audioNode.pause()' multiple times
let stoppedByEndTime = false

const handleTimeUpdate = (): void => {
if (endTime > 0 && audioNode.currentTime >= endTime) {
if (loop) {
// If loop is true and we reached 'endTime', reset to 'startTime'
audioNode.currentTime = startTime || 0
audioNode.play()
} else if (!stoppedByEndTime) {
stoppedByEndTime = true
audioNode.pause()
}
}
}

if (endTime > 0) {
audioNode.addEventListener("timeupdate", handleTimeUpdate)
}

return () => {
if (audioNode && endTime > 0) {
audioNode.removeEventListener("timeupdate", handleTimeUpdate)
}
}
}, [endTime, loop, startTime])

// Handle looping the audio
useEffect(() => {
const audioNode = audioRef.current
if (!audioNode) return

// Loop the audio when it has ended
const handleAudioEnd = (): void => {
if (loop) {
audioNode.currentTime = startTime || 0 // Reset to startTime or to the start if not specified
audioNode.play()
}
}

audioNode.addEventListener("ended", handleAudioEnd)

return () => {
if (audioNode) {
audioNode.removeEventListener("ended", handleAudioEnd)
}
}
}, [element.startTime])
}, [loop, startTime])

const uri = endpoints.buildMediaURL(element.url)
return (
Expand Down
81 changes: 75 additions & 6 deletions frontend/lib/src/components/elements/Video/Video.tsx
Expand Up @@ -41,7 +41,7 @@ export default function Video({

/* Element may contain "url" or "data" property. */

const { type, url, startTime, subtitles } = element
const { type, url, startTime, subtitles, endTime, loop } = element

// Handle startTime changes
useEffect(() => {
Expand All @@ -55,7 +55,6 @@ export default function Video({

const setStartTime: () => void = () => {
if (videoNode) {
// setStartTime
videoNode.currentTime = element.startTime
}
}
Expand All @@ -71,12 +70,82 @@ export default function Video({
}
}, [element])

// Stop the video at 'endTime' and handle loop
useEffect(() => {
const videoNode = videoRef.current
if (!videoNode) return

// Flag to avoid calling 'videoNode.pause()' multiple times
let stoppedByEndTime = false

const handleTimeUpdate = (): void => {
if (endTime > 0 && videoNode.currentTime >= endTime) {
if (loop) {
// If loop is true and we reached 'endTime', reset to 'startTime'
videoNode.currentTime = startTime || 0
videoNode.play()
} else if (!stoppedByEndTime) {
stoppedByEndTime = true
videoNode.pause()
}
}
}

if (endTime > 0) {
videoNode.addEventListener("timeupdate", handleTimeUpdate)
}

return () => {
if (videoNode && endTime > 0) {
videoNode.removeEventListener("timeupdate", handleTimeUpdate)
}
}
}, [endTime, loop, startTime])

// Handle looping the video
useEffect(() => {
const videoNode = videoRef.current
if (!videoNode) return

// Loop the video when it has ended
const handleVideoEnd = (): void => {
if (loop) {
videoNode.currentTime = startTime || 0 // Reset to startTime or to the start if not specified
videoNode.play()
}
}

videoNode.addEventListener("ended", handleVideoEnd)

return () => {
if (videoNode) {
videoNode.removeEventListener("ended", handleVideoEnd)
}
}
}, [loop, startTime])

const getYoutubeSrc = (url: string): string => {
const { startTime } = element
if (startTime) {
return `${url}?start=${startTime}`
const { startTime, endTime, loop } = element
const youtubeUrl = new URL(url)

if (startTime && !isNaN(startTime)) {
youtubeUrl.searchParams.append("start", startTime.toString())
}

if (endTime && !isNaN(endTime)) {
youtubeUrl.searchParams.append("end", endTime.toString())
}

if (loop) {
youtubeUrl.searchParams.append("loop", "1")
// When using the loop parameter, YouTube requires the playlist parameter to be set to the same video ID
const videoId = youtubeUrl.pathname.split("/").pop()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does that work for all the different types of youtube URLs?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It should work with different youtube URLs, because we standardize URL on the backend

return f"https://www.youtube.com/embed/{code}"


if (videoId) {
youtubeUrl.searchParams.append("playlist", videoId)
}
}
return url
return youtubeUrl.toString()
}

/* Is this a YouTube link? If so we need a fancier tag.
Expand Down