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

[Roblox] Add individual extractor #5178

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

Conversation

MinePlayersPE
Copy link
Contributor

IMPORTANT: PRs without the template will be CLOSED

Description of your pull request and other information

Partially solves #5049 and #3667

Intended to always download audio with basic metadata (when actually available, via webpage), and download video (via assetdelivery API) alongside music metadata (via toolbox/extra metadata API) when logged in

Template

Before submitting a pull request make sure you have:

In order to be accepted and merged into yt-dlp 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?

Comment on lines +101 to +107
# Assets have no file extension and use binary/octet-stream as Content-Type
pp = FFmpegPostProcessor(self._downloader)
self.to_screen(f'{asset_id}: Checking file format with ffprobe')
try:
metadata = pp.get_metadata_object(asset_file_url)
except PostProcessingError as err:
raise ExtractorError(err.msg, expected=True)
Copy link
Member

Choose a reason for hiding this comment

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

There's actually a feature request to support this in core code #613

That aside, is there no way to avoid this in the extractor?

Copy link
Member

Choose a reason for hiding this comment

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

The most important metadata are setting acode/vcodec='none' and ext. I assume codec can be handled with asset_type

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can only narrow down the formats, e.g. audio can only be uploaded as ogg/vorbis or mp3, and while video is most likely webm/vp9 (since video can only be uploaded by select ppl, we can't even be sure about this), the metadata hints at mp4 muxing being possible

@pukkandan pukkandan added the site-request Request to support a new website label Dec 8, 2022
@pukkandan pukkandan added the stale-pr PR that has been pending fixes for a long time label Dec 27, 2022
@pukkandan pukkandan force-pushed the master branch 2 times, most recently from ee280c7 to 7aeda6c Compare May 24, 2023 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
site-request Request to support a new website stale-pr PR that has been pending fixes for a long time
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants