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

[embedthumbnail] use ffmpeg to m4a/mp4 if version >= 4.0 #29593

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[embedthumbnail] use ffmpeg to m4a/mp4 if version >= 4.0 #29593

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Jul 20, 2021

Please follow the guide below

  • You will be asked some questions, please read them carefully and answer honestly
  • Put an x into all the boxes [ ] relevant to your pull request (like that [x])
  • Use Preview tab to see how your pull request will actually look like

Before submitting a pull request make sure you have:

In order to be accepted and merged into youtube-dl each piece of code must be in public domain or released under Unlicense. Check one of the following options:

  • I am the original author of this code and I am willing to release it under Unlicense
  • I am not the original author of this code but it is in public domain or released under Unlicense (provide reliable evidence)

What is the purpose of your pull request?

  • Bug fix
  • Improvement
  • New extractor
  • New feature

Description of your pull request and other information

Resolves #24882, and maybe #29563
Needs my #29581

ffmpeg supports embedding thumbnails to m4a/mp4 from version 4.0 (has been supported in api since before).
This PR checks ffmpeg version (actually libavformat build version) and if newer enough use ffmpeg instead of not actively maintained atomicparsley.

@Emasoft
Copy link

Emasoft commented Jan 30, 2023

Why is this not being merged yet?

@nicolaasjan
Copy link

nicolaasjan commented Apr 11, 2024

Anybody home?

Since some time I'm not able to compile my custom youtube-dl with #29581 and #29593 integrated, due to conflicts that have emerged.
Under #29581:

screenshot1

User @ghost seems deleted from GitHub...
So, I guess this PR is stale now?

@dirkf, what are your thoughts?
Is would be nice if someone else revived this.
Unfortunately I don't have the needed knowledge.

@dirkf
Copy link
Contributor

dirkf commented Apr 11, 2024

For the record, there is a procedure to revive a PR where the original repo has gone.

Let's use 29593 as an example; origin will be the git remote name pointing to the PR's target; embedthumbnail will be the local branch name.

  1. Check out the GH internal copy of the PR github.com/{repo}/{project}/pull/{pr}:
git fetch origin pull/29593/head:embedthumbnail
git switch embedthumbnail
  1. Probably, merge the latest master with embedthumbnail, resolving any conflicts.
  2. Fix the local (merged) branch.
  3. Perhaps review related stale PRs to select any that are closely linked and apply the same process to those PRs and merge them into your local branch, returning to step 3.
  4. Upload the perfected branch to your fork of the original target and open a new PR, copying the original PR Description(s) with appropriate changes, additions and attributions, and appending a line like this (Replaces) closes #29593 for each constituent PR.

Eventually, I'll do that for this PR, but anyone is very welcome to accelerate the process.

@nicolaasjan
Copy link

I think I have working code right now (tested with one m4a download).
Applied all changes, including the ones from #29581, via GH's web interface.
They are in my embedthumbnail branch:
https://github.com/nicolaasjan/youtube-dl

I don't dare to make a pull request, because I have no idea to test properly, so I couldn't check these boxes:

screenshot1

@dirkf
Copy link
Contributor

dirkf commented Apr 22, 2024

I have a new change set WIP that supersedes #29581 and #29593. I'll try to get a PR up soon that you can test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants