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

yt-dlp: Add mutagen variant #20465

Closed

Conversation

TheRealKeto
Copy link
Contributor

@TheRealKeto TheRealKeto commented Sep 17, 2023

Description

Add mutagen variant to yt-dlp subport. This allows for mutagen to be used over AtomicParsley for metadata manipulation (e.g thumbnail embedding)

Closes: https://trac.macports.org/ticket/66945

Type(s)
  • bugfix
  • enhancement
  • security fix
Tested on

macOS 12.6 21G115 arm64
Xcode 14.2 14C18

Verification

Have you

  • followed our Commit Message Guidelines?
  • squashed and minimized your commits?
  • checked that there aren't other open pull requests for the same change?
  • referenced existing tickets on Trac with full URL?
  • checked your Portfile with port lint --nitpick?
  • tried existing tests with sudo port test?
  • tried a full install with sudo port -vst install?
  • tested basic functionality of all binary files?

@macportsbot
Copy link

Notifying maintainers:
@ryandesign for port youtube-dl.

@ryandesign
Copy link
Contributor

All you've done here is add a dependency. You haven't told yt-dlp that it should use AtomicParsley or py-mutagen.

@ryandesign
Copy link
Contributor

Also, you haven't removed the AtomicParsley dependency when the mutagen variant is used.

@TheRealKeto
Copy link
Contributor Author

TheRealKeto commented Sep 17, 2023

All you've done here is add a dependency. You haven't told yt-dlp that it should use AtomicParsley or py-mutagen.

yt-dlp automatically prioritizes mutagen and AtomicParsley, depending on which one is found first. Simply adding the dependency is sufficient.

Also, you haven't removed the AtomicParsley dependency when the mutagen variant is used.

I can't remove it even when removing it with depends_run-delete, as it's part of the ffmpeg default variant. Would it be acceptable to make two variants (e.g mutagen and atomic) within the yt-dlp subport? Alternatively, both dependencies can co-exist, as mutagen is mostly used to embed thumbnails.

@ryandesign
Copy link
Contributor

All you've done here is add a dependency. You haven't told yt-dlp that it should use AtomicParsley or py-mutagen.

yt-dlp automatically prioritizes mutagen and AtomicParsley, depending on which one is found first. Simply adding the dependency is sufficient.

Yes, I'm sure that is what will happen but it is not sufficient for MacPorts. Consider the opposite situation: The user does not select the mutagen variant, but the user happens to have mutagen installed. The user expects yt-dlp to use AtomicParsley, but it uses mutagen, despite not declaring a dependency on it. We don't want that to happen.

Also, you haven't removed the AtomicParsley dependency when the mutagen variant is used.

I can't remove it even when removing it with depends_run-delete, as it's part of the ffmpeg default variant. Would it be acceptable to make two variants (e.g mutagen and atomic) within the yt-dlp subport? Alternatively, both dependencies can co-exist, as mutagen is mostly used to embed thumbnails.

I understand that simply adding depends_run-delete port:AtomicParsley within the variant will not accomplish the goal of removing the dependency; some restructuring of the port would be needed.

Yes, when there are two conflicting options, like AtomicParsley and mutagen, having two separate variants, which are marked as conflicting with one another and where the port enforces that one of the two variants must be selected and one of them is selected by default, is optimal in my opinion. It is complicated by the fact that yt-dlp is a subport of youtube-dl and youtube-dl presumably does not have the option of using mutagen.

What is the relationship between ffmpeg and AtomicParsley and mutagen? Currently the ffmpeg variant depends on both ffmpeg and AtomicParsley. Was that the right thing to do? Is the answer the same for youtube-dl and yt-dlp? Can ffmpeg be used without AtomicParsley? Can AtomicParsley be used without ffmpeg? Does mutagen use ffmpeg—mandatorily, optionally, or not at all? The answers to these questions might tell us whether we should have three variants, one for adding each dependency (and whatever mechanism might be needed to inform the software that it should or should not use that dependency), and which of them should requires the other.

The reason the ticket has remained unsolved for so many months is my lack of motivation to research the answers to these questions.

@TheRealKeto
Copy link
Contributor Author

The hierarchy of dependencies which are of concern in this PR (AtomicParsley, mutagen, ffmpeg) is the following:

  • yt-dlp prioritizes mutagen, but will use AtomicParsley if mutagen is not installed.
  • If AtomicParsley and mutagen are missing, ffmpeg will be used.
  • Neither project can't work without ffmpeg.

This can also be said about youtube-dl, although without mutagen in the mix. Given the concerns previously raised, I don't think this can be resolved.

Even if both youtube-dl and yt-dlp were to be separate port files, neither project has some kind of configuration system when building, making it impossible for either project to prioritize a dependency over another. As it stands, both projects have logic to detect and fallback on dependencies it's missing, but there's no way to enforce one dependency over another without heavy patching.

@TheRealKeto TheRealKeto deleted the yt-dlp/variant/mutagen branch September 18, 2023 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants