Skip to content

Add basic support for PotPlayer, MPC-HC, MPC-BE#3798

Merged
FreeTubeBot merged 5 commits intoFreeTubeApp:developmentfrom
trostboot:ext-mpchc-mpcbe
Jul 29, 2023
Merged

Add basic support for PotPlayer, MPC-HC, MPC-BE#3798
FreeTubeBot merged 5 commits intoFreeTubeApp:developmentfrom
trostboot:ext-mpchc-mpcbe

Conversation

@trostboot
Copy link
Contributor

Add basic support for PotPlayer, MPC-HC, MPC-BE

Pull Request Type

  • Bugfix
  • Feature Implementation
  • Documentation
  • Other

Related issue

closes #3614

Description

This PR adds basic external player functionality for PotPlayer, MPC-HC and MPC-BE.
It also sorts the list alphabetically, because UX.

Note that MPC-HC requires yt-dlp or similar to be present.
It will also fail to properly play back videos with a startOffset if the resolution is > 1080p (audio will play, video will be black). Unfortunately the default limit (View > Options > Advanced > YDLMaxHeight) MPC-HC ships with is 1440p.
Feedback desired on whether startOffset should stay enabled given the current state.

Note that MPC-BE requires yt-dlp or similar to be present and enabled in order to parse playlists.

Note that both MPC-HC and MPC-BE require #3797 to fix playback from within playlists.

Screenshots

Testing

Desktop

  • OS: Windows
  • OS Version: 11
  • FreeTube version: v0.18.0-nightly-3148

Additional context

Copy link
Member

@PikachuEXE PikachuEXE left a comment

Choose a reason for hiding this comment

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

e9fae08
Moves entries around

Also the indent alignment is inconsistent with other entries

auto-merge was automatically disabled July 22, 2023 06:33

Head branch was pushed to by a user without write access

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) July 22, 2023 06:33
@trostboot
Copy link
Contributor Author

Thanks, fixed the indents.

As for the reordering, that was intentional so the list would be alphabetical. It just makes more sense to me. Do you have any objections to that?

@PikachuEXE
Copy link
Member

It's harder to review the diff

If we want the display order to be updated it should be done in code not in data (with limited no. of entries)

auto-merge was automatically disabled July 22, 2023 07:30

Head branch was pushed to by a user without write access

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) July 22, 2023 07:30
@trostboot
Copy link
Contributor Author

Well, I've had a look around the relevant code, but I'm afraid that's above my pay grade. Until someone else feels the need to tackle that, here's the unsorted list back :)

@trostboot trostboot requested a review from PikachuEXE July 22, 2023 07:32
Copy link
Member

@PikachuEXE PikachuEXE left a comment

Choose a reason for hiding this comment

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

Code looks fine but I will wait for #3797 to be merged first

@FreeTubeBot FreeTubeBot merged commit 01930e4 into FreeTubeApp:development Jul 29, 2023
@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Jul 29, 2023
@efb4f5ff-1298-471a-8973-3d47447115dc

Thanks for your contribution @trostboot. If u want to address more external player issues u could take a look at #1975

@trostboot trostboot deleted the ext-mpchc-mpcbe branch July 29, 2023 11:50
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.

[Feature Request]: Open videos in MPC-BE, MPC-HC, or PotPlayer

6 participants