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: Video capability for MCMP and Lua API #6439

Draft
wants to merge 38 commits into
base: development
Choose a base branch
from
Draft

Add: Video capability for MCMP and Lua API #6439

wants to merge 38 commits into from

Conversation

mpconley
Copy link
Contributor

@mpconley mpconley commented Nov 23, 2022

Brief overview of PR changes/additions

Adding:

  • GMCP capability to play videos via the Mud Client Media Protocol
  • Lua Geyser.VideoPlayer with dockable window support
  • Lua API createVideoPlayer()
  • Lua API loadVideoFile()
  • Lua API playVideoFile()
  • Lua API stopVideos()

Future:

  • Buttons on the Geyser.VideoPlayer to allow for play/pause/stop
  • Streaming (e.g., support YouTube, podcasts, etc.)

Stream a video

lua playVideoFile({
    name = "TextInMotion-VideoSample-1080p.mp4"
    , url = "https://d2qguwbxlx1sbt.cloudfront.net/"
    , stream = true
})

Download/play a video

lua playVideoFile({
    name = "TextInMotion-VideoSample-1080p.mp4"
    , url = "https://d2qguwbxlx1sbt.cloudfront.net/"
})

OR

lua playVideoFile({
    name = "TextInMotion-VideoSample-1080p.mp4"
    , url = "https://d2qguwbxlx1sbt.cloudfront.net/"
    , stream = false
})

Stop playing the video

lua stopVideos({
    name = "TextInMotion-VideoSample-1080p.mp4"
})

Preload a video to play later

lua loadVideoFile({
    name = "TextInMotion-VideoSample-1080p.mp4"
    , url = "https://d2qguwbxlx1sbt.cloudfront.net/"
})

Motivation for adding to Mudlet

To simplify and enhance capability to attract and teach new users to play MUD games, video capability is now available for the MUD Client Media Protocol and Lua APIs of Mudlet.

Other info (issues closed, discussion etc)

@mudlet-machine-account mudlet-machine-account added this to the 4.17.0 milestone Nov 23, 2022
@add-deployment-links
Copy link

add-deployment-links bot commented Nov 23, 2022

Hey there! Thanks for helping Mudlet improve. 🌟

Test versions

You can directly test the changes here:

No need to install anything - just unzip and run.
Let us know if it works well, and if it doesn't, please give details.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 23, 2022

Warnings
⚠️ PR makes changes to 24 source files. Double check the scope hasn't gotten out of hand

Generated by 🚫 dangerJS against 53b3318

src/TMainConsole.cpp Outdated Show resolved Hide resolved
@mpconley
Copy link
Contributor Author

/refresh links

@vadi2
Copy link
Member

vadi2 commented Nov 23, 2022

mudlet.org was having difficulties and I suspect the snapshots might not have been uploaded.

@vadi2
Copy link
Member

vadi2 commented Nov 23, 2022

Really great stuff as usual!

Copy link
Member

@SlySven SlySven left a comment

Choose a reason for hiding this comment

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

Given that the Video player is associated with a particular profile you also need to add a single line of code to enable/disable the Video player main toolbar button in:

  • (void) mudlet::disableToolbarButtons()
  • (void) mudlet::enableToolbarButtons()

src/CMakeLists.txt Outdated Show resolved Hide resolved
src/Host.cpp Outdated Show resolved Hide resolved
src/Host.cpp Outdated Show resolved Hide resolved
src/Host.cpp Outdated Show resolved Hide resolved
}

if (!pM->isVisible()) {
return {false, qsl("video player widget already closed")};
Copy link
Member

Choose a reason for hiding this comment

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

Not sure this counts as a warning - unlike the mapper I do not know whether it is possible to completely remove an extant "video player" whereas we only ever hide (but do not destroy) the mapper (which causes other issues if the user wishes to change from one embedded in a console to a floating/dockable one - as it cannot be done without closing the profile - which I would not like to see replicated in this new feature...!)

src/TMainConsole.cpp Outdated Show resolved Hide resolved
@@ -0,0 +1,24 @@
/***************************************************************************
Copy link
Member

Choose a reason for hiding this comment

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

This file is currently redundant - the trivial constructor can be placed within the header file (src/TVideoPlayer.h) as is the case with some other classes...

src/TVideoPlayer.h Outdated Show resolved Hide resolved
src/XMLexport.cpp Show resolved Hide resolved
src/dlgVideoPlayer.cpp Outdated Show resolved Hide resolved
@mpconley
Copy link
Contributor Author

  • enableToolbarButtons

These exist I believe: mpActionVideoPlayer

@SlySven
Copy link
Member

SlySven commented Nov 24, 2022

  • enableToolbarButtons

These exist I believe: mpActionVideoPlayer

Yeah - I think I missed them when I was looking through the changes, sorry about that.

@vadi2
Copy link
Member

vadi2 commented Nov 24, 2022

It's not immediately obvious as to why the mac build failed. Rebuilding...

@vadi2
Copy link
Member

vadi2 commented Nov 24, 2022

Could you add some examples of how to use the APIs? We'll need those for documentation in the end too. I tried playing around with the PR but didn't have any luck, documenting my beginners journey -

  1. tried loadVideoFile() thinking it would load and play a video for me - it would always return true but nothing happened. Is the separation to ensure the file is available? In that case would playVideoFile() still play a video for me, just on a possible delay?
  2. did the same for playVideoFile(), the function is also happy but nothing happens. Perhaps it should complain if the video player is not created?
  3. created the video player, got a small bar on the screen, but no functions wanted to play a video still.

I noticed that if you use createVideoPlayer() with invalid arguments (say, none at all) then the 'Video' button breaks.

Need to also add the Video option to the menu bar, as some play with the toolbar off for bigger screen real estate:

Screenshot 2022-11-24 8 32 56 AM

@mpconley
Copy link
Contributor Author

  1. tried loadVideoFile() thinking it would load and play a video for me - it would always return true but nothing happened. Is the separation to ensure the file is available? In that case would playVideoFile() still play a video for me, just on a possible delay?

Like sound and music, the loadXYZFile() functions are for preloading media in the background so it is ready to use for a future playXYZFile() function later. It downloads the file and puts it in the cache.

@mpconley mpconley requested review from a team as code owners November 27, 2022 13:44
@vadi2
Copy link
Member

vadi2 commented Nov 27, 2022

Coming from a perspective of a non-techie, do you think the Video button sets out the right expectations? If someone is to press it, how would they make use of it?

I am wondering if we should be showing the button until there's been a video played or until there's more controls in the widget to load & play videos on your own.

@mpconley
Copy link
Contributor Author

mpconley commented Nov 27, 2022

Coming from a perspective of a non-techie, do you think the Video button sets out the right expectations? If someone is to press it, how would they make use of it?

I am wondering if we should be showing the button until there's been a video played or until there's more controls in the widget to load & play videos on your own.

OK, now the button and shortcuts will not be enabled until the video UI is generated by MCMP, API, or the user putting a Geyser.VideoPlayer into their UI.

src/mudlet.h Outdated Show resolved Hide resolved
@mpconley
Copy link
Contributor Author

Found that an undocked and floating video when playing and is closed before it quits playing will also cause the segmentation fault.

@mpconley
Copy link
Contributor Author

Aligned the syntax of playVideoFile() with playSoundFile() and playMusicFile(). These are documented on Area 51. I added a key/value pair of stream = true to indicate streaming versus downloading for both MCMP and the API. On the bright side, this extends for sound and music too, so people/game admins now have a choice on whether to download or stream. Will need to work this into an update for MCMP as an option too.

@vadi2
Copy link
Member

vadi2 commented Nov 28, 2022

Found that an undocked and floating video when playing and is closed before it quits playing will also cause the segmentation fault.

My suspicion is that something is wrong with widget parenting. On my machine, there was an infinite loop of window creation in the stacktrace.

@vadi2
Copy link
Member

vadi2 commented Dec 8, 2022

This code isn't playing the video and there's no obvious errors, what could be wrong?

videoPlayer = Geyser.VideoPlayer:new({
  name = "videoPlayer",
  x = "-50%", y = "-50%",
  width = "50%",
  height = "50%"
})
playVideoFile(({name = "https://d2qguwbxlx1sbt.cloudfront.net/TextInMotion-VideoSample-1080p.mp4"}))

@mpconley
Copy link
Contributor Author

mpconley commented Dec 8, 2022

lua loadVideoFile({
    name = "TextInMotion-VideoSample-1080p.mp4"
    , url = "https://d2qguwbxlx1sbt.cloudfront.net/"
})

Note some changes in the API mentioned top of the GitHub PR and you must use play to play (load is just to download).

lua playVideoFile({name = "TextInMotion-VideoSample-1080p.mp4", url = "https://d2qguwbxlx1sbt.cloudfront.net/"})

@mpconley
Copy link
Contributor Author

mpconley commented Dec 8, 2022

I am working on this by the way, coming soon - doing a refactor so that we could run a video in the Mudlet scope too, not just in the profile scope. Hopefully I will work my way out of the segment faults this way too. More to come...

@vadi2 vadi2 modified the milestones: 4.17.0, 4.18.0 Jan 27, 2023
@mpconley mpconley closed this by deleting the head repository Apr 8, 2023
@mpconley mpconley reopened this Apr 8, 2023
@mpconley mpconley closed this Apr 8, 2023
@mpconley mpconley reopened this Apr 8, 2023
@mpconley
Copy link
Contributor Author

mpconley commented Apr 9, 2023

This code will be refactored in with the Qt6 updates for Mudlet multimedia.

@vadi2
Copy link
Member

vadi2 commented Apr 11, 2023

Have you got the original code to push to a branch btw? It is not in https://github.com/mpconley/Mudlet/branches

@mpconley
Copy link
Contributor Author

mpconley commented Apr 11, 2023

No, it was on a computer that died and I blew away the old branches when I made the new fork. I'll just reference what is here. Now that QMediaPlaylist is gone, I will be referencing as much of what they've done in the QT6 example as possible so this code will only be a guide. No worries, just iterating :)

@mpconley mpconley marked this pull request as draft October 22, 2023 14:24
@mpconley
Copy link
Contributor Author

Note to self: When this is picked back up, determine if there is a concept (or reverse concept) here that will help resolve the sometimes crashes when Mudlet closes when video is still playing.

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.

None yet

4 participants