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

[ZDF/3sat/phoenix.de] Extractor updates and fixes (#21185) #22191

Closed
wants to merge 2 commits into from
Closed

[ZDF/3sat/phoenix.de] Extractor updates and fixes (#21185) #22191

wants to merge 2 commits into from

Conversation

FliegendeWurst
Copy link

@FliegendeWurst FliegendeWurst commented Aug 23, 2019

What is the purpose of your pull request?


3sat recently (?) changed their tech stack. It seems to work like ZDF's stack. In case you're wondering, they're both public TV channels in Germany. It's only logical that they would use the same tech.
Edit: turns out that phoenix.de still uses the older stack, so we have to keep dreisat.py for now move that code to phoenix.py.

Should the extractor file be renamed to zdf3sat.py or something like that?

@@ -39,10 +39,22 @@ def _extract_player(self, webpage, video_id, fatal=True):


class ZDFIE(ZDFBaseIE):
_VALID_URL = r'https?://www\.zdf\.de/(?:[^/]+/)*(?P<id>[^/?]+)\.html'
IE_NAME = "ZDF-3sat"

Choose a reason for hiding this comment

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

A bit confusing, since the filename is still zdf.py

Copy link
Author

Choose a reason for hiding this comment

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

Should I rename the file?

@@ -39,10 +39,22 @@ def _extract_player(self, webpage, video_id, fatal=True):


class ZDFIE(ZDFBaseIE):
_VALID_URL = r'https?://www\.zdf\.de/(?:[^/]+/)*(?P<id>[^/?]+)\.html'
IE_NAME = "ZDF-3sat"
_VALID_URL = r'https?://www\.(zdf|3sat)\.de/(?:[^/]+/)*(?P<id>[^/?]+)\.html'

Choose a reason for hiding this comment

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

Please look at #21356 (comment)

Copy link
Author

@FliegendeWurst FliegendeWurst Aug 23, 2019

Choose a reason for hiding this comment

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

I'll try to find a suitable test case. If anyone can supply one, I would be very happy!

Choose a reason for hiding this comment

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

@FliegendeWurst
Copy link
Author

@dstftw: I cannot "fix" the downloader without a test case.

@LinuxOpa
Copy link

@dstftw: I cannot "fix" the downloader without a test case.

wie kann ich helfen / how can I help ?

@FliegendeWurst
Copy link
Author

wie kann ich helfen / how can I help ?

If you know of any video on either site that doesn't work with the patch, feel free to post the link. I'm not certain such videos can be found by just browsing the website, since (probably) the whole tech stack was replaced.

@LinuxOpa
Copy link

wie kann ich helfen / how can I help ?

If you know of any video on either site that doesn't work with the patch, feel free to post the link.
I'm not certain such videos can be found by just browsing the website, since (probably)
the whole tech stack was replaced.

So far all downloads from ZDF and 3SAT work with the patch, and I download much of the above, since June I have manually inserted the patch into youtube-dl. I'm just waiting for the "3SATZDF done" to turn on the update function again. - Bis jetzt funktionieren alle Downloads von ZDF und 3SAT mit dem Patch, und ich lade viel von den Genannten herunter, seit Juni habe ich den Patch manuell eingefügt in youtube-dl. Ich warte nur noch auf das "3SATZDF erledigt" um dann die Update-Funktion wieder einzuschalten.

@Wikinaut Wikinaut mentioned this pull request Sep 16, 2019
@Wikinaut
Copy link

I confirm that it works with (example) https://www.3sat.de/migration/3sat/robert-frank-don-t-blink-100.html .

@FliegendeWurst FliegendeWurst changed the title [ZDF/3sat] Let ZDF extractor work with 3sat.de (fixes #21185) [ZDF/3sat/phoenix.de] Extractor updates and fixes (#21185) Sep 16, 2019
@w4grfw
Copy link

w4grfw commented Sep 24, 2019

Phoenix is now in ARD Mediathek: https://www.ardmediathek.de/phoenix/

@mojo-hakase
Copy link

So, what is the problem with this pull request and that #21356. Why don't they get merged. Is there anything that need to be done?

@TechupBusiness
Copy link

So, what is the problem with this pull request and that #21356. Why don't they get merged. Is there anything that need to be done?

Yes wondering too... The issue seems to be more about decisions than actual implementation.
Dear youtube-dl-maintainer (@dstftw ), please make your decisions so it can go on. Thanks a lot!

@pboettch
Copy link

pboettch commented Jan 9, 2020

@FliegendeWurst If I were you, I'd close this PR, rebase your changes and issue a new PR to get it integrated. Seeing the bandwidth on this repo I could understand why the maintainer might oversee updated PRs.

Thanks for your work.

@FliegendeWurst
Copy link
Author

@dstftw I rebased this PR, can you review it again?

@rolfn
Copy link

rolfn commented Aug 2, 2020

I can also confirm that the patch works. It would be very nice to add it to the master brunch.

@blackjack4494
Copy link

works like a charm. added it to my fork :)

@TechupBusiness
Copy link

TechupBusiness commented Sep 17, 2020

Yes works, would be great to see this finally merged @dstftw Thanks!
3sat is not usable since ages with the official youtube-dl release.

@FliegendeWurst
Copy link
Author

Closing this PR since my repository is not available. I will open another PR.

@TechupBusiness
Copy link

Closing this PR since my repository is not available. I will open another PR.

Thanks @FliegendeWurst !!! 👏 We really need to get this accepted for the/a next release!

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

Successfully merging this pull request may close these issues.