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

Do NOT put archive.txt and related entries into the download location paths, also allow output filename to be set #37

Closed
GlassedSilver opened this issue Mar 22, 2020 · 2 comments
Labels
is: bug Something isn't working is: enhancement New feature or extension of existing functionality

Comments

@GlassedSilver
Copy link
Collaborator

Personally, I'd favor putting the archive related files into a specified directory.

But whatever you do, if you put them into the path for downloaded files you'll have a bad time if you have a variable output directory.

Related here is another issue. If you wanna have an output directory with spaces you need quotation marks, those however escape the -o command arg if I'm not mistaken.

Even when using a directory with underscores I get download failed errors.

Snippet of error log when using video/%(uploader)s [%(channel_id)s]/%(upload_date)s - %(title)s [%(id)s] as output:

at Object.openSync (fs.js:440:3)
at /app/app.js:923:33
at Layer.handle [as handle_request] (/app/node_modules/express/lib/router/layer.js:95:5)
at next (/app/node_modules/express/lib/router/route.js:137:13)
at Route.dispatch (/app/node_modules/express/lib/router/route.js:112:3)
at Layer.handle [as handle_request] (/app/node_modules/express/lib/router/layer.js:95:5)
at /app/node_modules/express/lib/router/index.js:281:22
at Function.process_params (/app/node_modules/express/lib/router/index.js:335:12)
at next (/app/node_modules/express/lib/router/index.js:275:10)
at compression (/app/node_modules/compression/index.js:220:5)
(node:1) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 3)
(node:1) UnhandledPromiseRejectionWarning: Error: ENOENT: no such file or directory, open 'video/%(uploader)s [%(channel_id)s]/%(upload_date)s - %(title)s [%(id)s]archive.txt'

Snippet when using just video/ as output.

at Object.openSync (fs.js:440:3)
at /app/app.js:923:33
at Layer.handle [as handle_request] (/app/node_modules/express/lib/router/layer.js:95:5)
at next (/app/node_modules/express/lib/router/route.js:137:13)
at Route.dispatch (/app/node_modules/express/lib/router/route.js:112:3)
at Layer.handle [as handle_request] (/app/node_modules/express/lib/router/layer.js:95:5)
at /app/node_modules/express/lib/router/index.js:281:22
at Function.process_params (/app/node_modules/express/lib/router/index.js:335:12)
at next (/app/node_modules/express/lib/router/index.js:275:10)
at compression (/app/node_modules/compression/index.js:220:5)
(node:1) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 4)

Another thing I noticed. You have coded a way to set the output directory and that's sweet. However, as you can see there are definitely users who want to have a variable output directory, so expect that in your coding, but also a variable filename.

I expected to set the filename in the path setting by simply not putting an / in the end. Well I was wrong to assume this. :P

@Tzahi12345
Copy link
Owner

Personally, I'd favor putting the archive related files into a specified directory.

You have a good point here, perhaps an appdata/archives folder can serve that role. I didn't want the audio and video archives to interfere, but I can just set them to have different names (like archive_audio.txt and archive_video.txt). I'll have to give it more thought.

Even when using a directory with underscores I get download failed errors.

Hm, I think I'm reproducing this bug too. This isn't expected behavior, I'll look more into it.

Snippet of error log when using video/%(uploader)s [%(channel_id)s]/%(upload_date)s - %(title)s [%(id)s] as output:

Just to clarify, you're using the custom output in Advanced mode, right? If so, and I need to update the Simulated output to reflect this, the Custom output is actually relative to your audio and video folders. So if you want it to end up in the video folder, you would use %(uploader)s [%(channel_id)s]/%(upload_date)s - %(title)s [%(id)s]. This shouldn't however, cause an error. It should just put the video in video/video/..., so it looks like something else is going on here.

However, as you can see there are definitely users who want to have a variable output directory, so expect that in your coding, but also a variable filename.

Good idea - there's a few roadblocks to implementing something like this. The file manager relies on a static output directory, so videos downloaded outside are not seen by it. The way the video/audio player is coded relies on static paths for non-subscription downloads, though this is easier to change.

I think the easiest way of implementing something like this is letting the global youtube-dl "-o" arg override the existing "-o", which it currently doesn't. It should be doing so anyways, so I'll consider that a bug worth fixing.

@Tzahi12345 Tzahi12345 added is: bug Something isn't working is: enhancement New feature or extension of existing functionality labels Mar 23, 2020
@Tzahi12345
Copy link
Owner

Quick update on this issue:

Archives were moved to the appdata/archives in the last update. I also just got around to allowing global custom args to override output, so you can set the custom output globally (relevant commit).

Regarding file names with spaces when using custom args, this has also been fixed! You can see the relevant commit here. The only thing to note is that args are now delimited by double commas (,,) rather than a space.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
is: bug Something isn't working is: enhancement New feature or extension of existing functionality
Projects
None yet
Development

No branches or pull requests

2 participants