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

Support multiple audio tracks / languages for southpark.de #21271

Open
SebiderSushi opened this issue Jun 1, 2019 · 6 comments
Open

Support multiple audio tracks / languages for southpark.de #21271

SebiderSushi opened this issue Jun 1, 2019 · 6 comments
Labels

Comments

@SebiderSushi
Copy link

@SebiderSushi SebiderSushi commented Jun 1, 2019

Checklist

  • I'm reporting a site feature request
  • I've verified that I'm running youtube-dl version 2019.05.20
  • I've searched the bugtracker for similar site feature requests including closed ones

Description

The Problem is: southpark.de is offering its video content with two audio languages. First off english, as it is the original language of the series. Then in german as this is the country at which southpark.de is addressed to. When using the website to play a video, the language can be switched using a combination of javascript buttons which causes the wesite to reload and switch to the selected audio track.
When using youtube-dl to download a video from southpark.de, there is no option (at least none that i could find) to select which audio tracks are downloaded - downloading with youtube-dl yields a video with just the local language audio track (in this case german).

This could be fixed by finding a way to download the english audio track.

My proposed solution would look this way: When downloading a video from southpark.de that is being offered in multiple languages, youtube-dl will output a video file with all available languages as seperate audio tracks embedded into it.

Ideally, this should be done in a way so that it applies to all southpark sites for all countries, but this will sadly be impossible to test properly without proxies or similar since any southpark site always redirects to it's local site due to georestriction.

@dstftw
Copy link
Collaborator

@dstftw dstftw commented Jun 1, 2019

Post concrete example URLs.

@dstftw dstftw closed this Jun 1, 2019
@dstftw dstftw added the incomplete label Jun 1, 2019
@SebiderSushi
Copy link
Author

@SebiderSushi SebiderSushi commented Jun 3, 2019

Sorry. I concluded that this wasn't nessesary since the topic concerns the whole site along with every video as far as I know. But the issue description requirements are worded very precise.

Example URLs:
http://www.southpark.de/alle-episoden/s22e02-a-boy-and-a-priest
http://www.southpark.de/alle-episoden/s22e08-buddha-box

@remitamine remitamine reopened this Jun 3, 2019
@remitamine remitamine added request and removed incomplete labels Jun 3, 2019
@remitamine remitamine closed this in 0c84002 Jun 3, 2019
@SebiderSushi
Copy link
Author

@SebiderSushi SebiderSushi commented Jun 3, 2019

Sorry, i don't follow.
As far as i can tell, your commit causes the extractor for SouthparkDE to check what language is set in the cookies, after which the according language is downloaded. But how do i change this when i simply run youtube-dl from command line? Is there some switch that i missed or do i have to use a cookie file?

And: Does youtube-dl currently have any infrastructure to support multiple audio tracks in one video?

@SebiderSushi
Copy link
Author

@SebiderSushi SebiderSushi commented Jun 3, 2019

Is your code only able to get the cookie if downloading an url under http://www.southpark.de/alle-episoden/? Would be a good time to mention that URLs like the following should also be covered:

http://www.southpark.de/clips/152307/garrisons-hohle
http://www.southpark.de/clips/sz9szy/its-a-miracle
http://www.southpark.de/clips/sz9szy

http://www.southpark.de/collections/9996/all-about-stan
http://www.southpark.de/collections/9996/all-about-stan/1
http://www.southpark.de/collections/1256/season-7-creator-commentary/1
http://www.southpark.de/collections/7988/super-heldenschurken/1
http://www.southpark.de/collections/7988

And also the commentaries which are only offered with english audio:
http://www.southpark.de/collections/1256/season-7-creator-commentary/1

Also this change seems pretty simple, as all necessary extractor work is already implemented and this is only a question of specifying the correct language. Would it be safe to implement this for all southpark extractors of all country divisions since they're all using the same backend?

@remitamine remitamine reopened this Jun 3, 2019
@SebiderSushi
Copy link
Author

@SebiderSushi SebiderSushi commented Jun 4, 2019

N.B. on the requirement of example URLs: Only now i start to grasp the workflow of a youtube-dl developer and the role of example URLs and why they always should be provided.
 While it is perfectly comprehensible to expect that contribution guidelines will just be followed - you're the ones doing the work here - it might be good to give some understanding to the issue requester. In my case a clause like "Remember we're all very short on time here. Give your example URLs so that even if a contributor will exclusively use the provided URLs to try out a new implementation they represent every possible URL sheme that can be found on the site." would have made clear what to post & why.
 While it should be clear to just follow the contribution guidelines i was kind of left alone on deciding which URLs would qualify as a useful example. My reasoning was: since it's an issue concerning the enire website, all URLs do qualify so why put up examples and especially which? I'm sorry that that's been all that came to my mind that moment. Now i definitely know better.

This reminds me of #18303 where the user Siddhant suggested to change an error message for users who don't get it.
 While reading this i also thought at first "What's not clear in the error message?". But after reading Siddanth's comment i also thought it might be better to just change the message and make a clear point even for users who don't get it.

Maybe i'm the only one who didn't get it with the example URLs but if this happens often i hope changing the relevant paragraph will be considered.

@barsnick
Copy link

@barsnick barsnick commented Jun 5, 2019

In my case a clause like "Remember we're all very short on time here. Give your example URLs so that even if a contributor will exclusively use the provided URLs to try out a new implementation they represent every possible URL sheme that can be found on the site." would have made clear what to post & why.

In my opinion, this is not correct. Every properly implemented extractor has a host of test cases representing known "good" cases. These tests can and should be run, in order to avoid regressions. Unless you're trying to improve a known restricted extractor, you shouldn't be forced to provide previously good cases.

The problem is: I have stumbled across this myself, when trying to improve extractors. (Improving them myself meaning: Fix or amend the python code, test with my particular use case, and try to create a PR (pull request).) My impression was (don't take this as full fact): Some testcases are outdated, some actually don't work anymore, some require a login (but AFAICT the test infrastructure doesn't allow for providing login credentials, while the CLI does), some of the checked details have changed but have never been updated... So before fixing something, I need to go through the existing tests first. Oh, and I haven't found a way to say "run all tests", they apparently need to be specified with "_1", "_2", "_3". This makes it very tedious, and the benefit of the test cases becomes slightly questionable.

Sorry for this rant. youtube-dl is an excellent tool, and I'm impressed by its quality and how much progress it makes with limited developer time. These issues of mine belong in a separate ticket. I'm just trying to say: You probably needn't have worried about providing existing URLs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.