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

Subtitle file is not deleted after being embedded #630

Closed
35609902357 opened this issue Aug 5, 2021 · 14 comments
Closed

Subtitle file is not deleted after being embedded #630

35609902357 opened this issue Aug 5, 2021 · 14 comments
Labels
enhancement New feature or request

Comments

@35609902357
Copy link

Embedding subtitles in the video and also keeping the file is pointless and it results in 2 identical tracks for the same video.

The default behavior of youtube-dl is to keep the file if not embedded and delete it if embedded, this is smarter because the -k flag can be used if someone wants to embed and also keep the file, but there is no flag to automatically remove the file once embedded.

Maybe adding a flag like --keep-subs-file would solve the problem for those few users who might want both, but the default should be what youtube-dl does, keep the file if external subs or embed and delete.

@pukkandan
Copy link
Member

Embedding subtitles in the video and also keeping the file is pointless and it results in 2 identical tracks for the same video.

Just because you don't have a use-case for it doesn't mean it is pointless

but the default should be what youtube-dl does, keep the file if external subs or embed and delete.

If you are using --write-sub --embed-sub, it means you want to both keep the subs in disk and embed it. You may also seperately use --embed-subs to embed the subs or --write-subs to keep the file. This is consistent with how --write/embed-thumbnails work

The default behavior of youtube-dl is to keep the file if not embedded and delete it if embedded

While this is not the "right" behaviour imo, you are right that this is how upstream works. So I will make a comapt-option to go back to this. But the default will be kept as the current behaviour

this is smarter because the -k flag can be used if someone wants to embed and also keep the file, but there is no flag to automatically remove the file once embedded.

It is not the same. -k also keeps around many other files like the thumbnail, unmerged streams etc

@pukkandan pukkandan added the enhancement New feature or request label Aug 5, 2021
@pukkandan
Copy link
Member

pukkandan commented Aug 5, 2021

For reference, here's the differences:

youtube-dl

Option Behaviour
--write-subs Write normal sub to disk
--write-auto-subs Write auto sub to disk
--write-subs --write-auto-subs Write a normal/auto sub to disk
--embed-subs Do nothing
Nothing Embed normal sub and keep it in disk
Nothing Embed a normal/auto sub and keep it in disk
--write-subs --embed-subs Embed normal sub and delete
--write-auto-subs --embed-subs Embed auto sub and delete
--write-subs --write-auto-subs --embed-subs Embed a normal/auto sub and delete

yt-dlp

Option Behaviour
--write-subs Write normal sub to disk
--write-auto-subs Write auto sub to disk
--write-subs --write-auto-subs Write a normal/auto sub to disk
--embed-subs Embed normal sub and delete
--write-subs --embed-subs Embed normal sub and keep it in disk
--write-subs --write-auto-subs --embed-subs Embed a normal/auto sub and keep it in disk
--write-auto-subs --embed-subs Embed auto sub and delete
--write-subs --write-auto-subs --embed-subs
--compat-options no-keep-subs
Embed a normal/auto sub and delete
(#826 tracks adding a proper option for this)

Edited to be more comprehensive

@35609902357
Copy link
Author

35609902357 commented Aug 6, 2021

@pukkandan

Just because you don't have a use-case for it doesn't mean it is pointless

You arbitrarily decided to change a long standing default behavior because one user asked for this in an unrelated issue, no proper issue was ever open about this.

Changing that default behavior makes the transition from youtube-dl to yt-dlp and viceversa a pain because it breaks years old config files and requires to pay extra unneeded attention when using one project or the other.

Not only that, but those who embed --all-subs will have a hard time having left hundreds of files around.

Not only that, but it's also lacking. For example, I want to automatically embed subs if they are present, if there is no subtitles track I want to embed automatically generated subs. In youtube-dl I would go with --write-subs --write-auto-subs --embed-subs, yt-dlp leaves around files, even if there are only auto generated subs. Since the only option to differentiate between subs types are --write-subs and --write-auto-subs, yt-dlp makes it impossible to have the old behavior.

--embed-subs being a post-process option should naturally work on the file generated by the previous option, then all files are deleted after the post-process, for those who need one or many files to be kept there's the -k flag.

There is no point having internal and external tracks for the large majority of users. Even if someone might need both for some edge case, there's no point in making it the default instead of creating a --keep-subs option to integrate their need.

From the Readme (emphasis mine):

The main focus of this project is adding new features and patches while also keeping up to date with the original project

Changing defaults goes against the purpose of this project.

@pukkandan
Copy link
Member

pukkandan commented Aug 6, 2021

You arbitrarily decided to change a long standing default behavior because one user asked for this in an unrelated issue, no proper issue was ever open about this.

It is youtube-dl's handling of --embed-subs that is inconsistent with their own handling of the similar option --embed-thumb. When this disparity was pointed out to me, I changed this at my discretion. This should be considered this a bug in youtube-dl since this inconsistent behaviour is never documented anywhere and is therefore counter-intuitive and most probably unintentional.

Not only that, but those who embed --all-subs will have a hard time having left hundreds of files around.

--all-subs is deprecated. You should be using --sub-lang all. see this. While I keep most deprecated options still working, they will obviously be more limited in their capability than their recommended counterparts

Changing that default behavior makes the transition from youtube-dl to yt-dlp and viceversa a pain because it breaks years old config files and requires to pay extra unneeded attention when using one project or the other.

I have no intention of maintaining perfect compatibility with years old youtube-dl config files out of the box - especially if they are using such undocumented "features". If you really want yt-dlp to work as close to youtube-dl as possible, you should use --compat-options youtube-dl. There are also many other changes that have been made to the default behavior as documented here

It is true that I had failed to document how --embed-subs behaved differently than youtube-dl. But since b51d2ae, it is documented and I have also added the option --compat-options no-keep-subs to go back how youtube-dl handled it.

For example, I want to automatically embed subs if they are present, if there is no subtitles track I want to embed automatically generated subs. In youtube-dl I would go with --write-subs --write-auto-subs --embed-subs, yt-dlp leaves around files, even if there are only auto generated subs. Since the only option to differentiate between subs types are '--write-subsand--write-auto-subs`, yt-dlp makes it impossible to have the old behavior.

This is a fair point. Currently the only way to achive this is by using the compat-option. I will see what can be done to improve this

--embed-subs being a post-process option should naturally work on the file generated by the previous option, then all files are deleted after the post-process, for those who need one or many files to be kept there's the -k flag.

This is clearly not the intended use of -k

     -k, --keep-video                Keep the intermediate video file on disk after post-processing

Instead of changing how --embed-subs work, I could have instead changed how -k works with --embed-thumb, but that only serves to further limit the functionality

The main focus of this project is adding new features and patches while also keeping up to date with the original project
Changing defaults goes against the purpose of this project.

Keeping up-to-date means that upstream changes will be pulled to yt-dlp in a timely manner, not that I won't make any changes

@35609902357
Copy link
Author

I have also added the option --compat-options no-keep-subs to go back how youtube-dl handled it.

Thanks for the fix @pukkandan. For simplicity you should allow compatibility options as standalone flags too (for example allow both --compat-options no-keep-subs and --no-keep-subs to do the same thing, same with the other options).

This should be considered this a bug in youtube-dl

If you really think so you should open an issue in youtube-dl project to express your reasons.

@database64128
Copy link

Not only that, but it's also lacking. For example, I want to automatically embed subs if they are present, if there is no subtitles track I want to embed automatically generated subs. In youtube-dl I would go with --write-subs --write-auto-subs --embed-subs, yt-dlp leaves around files, even if there are only auto generated subs. Since the only option to differentiate between subs types are --write-subs and --write-auto-subs, yt-dlp makes it impossible to have the old behavior.

I think this issue should be kept open until this is resolved.

@pukkandan
Copy link
Member

It is now possible using the compat option. But I do agree that it should be possible without it. Please open a new issue

@database64128
Copy link

Opened #826 to track it.

@PacoH
Copy link

PacoH commented Oct 14, 2021

BTW using just --embed-subs left behind the vtt file in the folowing case. Adding --compat-options no-keep-subs fixed it. I don't know if this is case-specific or not. Version 2021.10.10. OS X installed with pip.

yt-dlp --no-check-certificate -v -i -c --no-mtime --no-playlist --sub-lang en,en-US,en_US,cs,zh-CN --embed-subs -o "%(title)s %(resolution)s %(format_id)s %(vcodec)s %(acodec)s.%(ext)s" <url>

@chapmanjacobd
Copy link

chapmanjacobd commented Jul 10, 2022

What about --embed-any-subs and --write-any-subs as a shortcut for the most verbose configurations

@pukkandan
Copy link
Member

You can create your own "shortcuts" with --alias

@chapmanjacobd
Copy link

chapmanjacobd commented Jul 10, 2022

ack. sorry I meant to post this on #2262. Having subs and autosub configuration completely separate but "shortcuts" like --embed-any-subs, --write-any-subs at the very outside of the code to combine some of the configuration

edit: or --embed-requested-subs --write-requested-subs to be consistent with the --match-filter requested_subtitles option

@AtomicNess123
Copy link

The point of the option:

--write-subs --write-auto-subs --embed-subs
--compat-options no-keep-subs

is to give priority to the subs, not the auto-subs, is this corrrect?

@pukkandan
Copy link
Member

Yes. It will download normal subs if found, and auto-subs otherwise

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Subtitle selection
Development

No branches or pull requests

6 participants