-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[vlive] Add support for live videos #9324
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
Conversation
f5f93ec to
bcd4e31
Compare
|
Added correct sub URLs, though both ffmpeg with |
|
Get it, ffmpeg actually downloads HLS WebVTT fine, just in a bit strange way. Though
What do you think? |
|
FYI: Seems #6144 implements downloading subtitles from m3u8 playlists. |
|
Thanks, didn't know about it. Though it seems to be stalled? If that would be easier for you, I can comment subtitle field for now and make separate PR. Grabbing live video streams is useful even without subs. |
|
In view-source:http://static.vlive.tv/desktop/resources/generated/js/page/2016033117/video.min.js: case "VOD_ON_AIR":
if (a.vodId && a.vodInKey) {
this._vodOnAir(a)
} else {
if (a.isLive) {
this._liveEnd()
} else {
this._comingSoon()
}
}
breakMaybe youtube-dl should follow the same logic? |
youtube_dl/extractor/vlive.py
Outdated
| for f in formats], | ||
| 'vie': [{'ext': 'vtt', 'url': self._get_sub_url(f['url'], 'vie')} | ||
| for f in formats], | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not all live videos have subtitles. For example http://www.vlive.tv/video/7871/1996-%EC%95%88%EC%8A%B9%EC%A4%80-%EC%9C%A4%EC%A0%95%EC%9E%AC-%EB%9D%BC%EC%9D%B4%EB%B8%8C does not have a subtitle when I just checked. By the way, I guess _get_live_sub_url is a better function name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't found anything similar in Live API, seems like the only way to determine subtitle status is to download playlist.m3u8 and parse it manually. It looks like this:
#EXTM3U
#EXT-X-VERSION:3
#EXT-X-MEDIA:TYPE=SUBTITLES,GROUP-ID="subs",NAME="Vietnamese",FORCED=NO,AUTOSELECT=YES,URI="subtitlelist_lvie.m3u8",LANGUAGE="vie"
#EXT-X-MEDIA:TYPE=SUBTITLES,GROUP-ID="subs",NAME="English",FORCED=NO,AUTOSELECT=YES,URI="subtitlelist_leng.m3u8",LANGUAGE="eng"
#EXT-X-MEDIA:TYPE=SUBTITLES,GROUP-ID="subs",NAME="zcn",FORCED=NO,AUTOSELECT=YES,URI="subtitlelist_lzcn.m3u8",LANGUAGE="zcn"
#EXT-X-STREAM-INF:BANDWIDTH=1464974,CODECS="avc1.66.22,mp4a.40.2",RESOLUTION=640x368,SUBTITLES="subs"
chunklist.m3u8?__agda__=1461853927_d2d91835e251112f931019235adf7e99
Are you ok with that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Parsing should be the way. It's better than hard-coded values in general. An approach is modifying _extract_m3u8_formats so that it can handle subtitles, too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An approach is modifying _extract_m3u8_formats so that it can handle subtitles, too.
i already proposed this in #8820 as a first step to handle WEBVTT m3u8 subtitles.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be very cool if we can get it merged, don't want to reimplement already written code.
|
For your previous question on how to download live subtitles, there are two options:
There's an issue on ihe second approach. We need to add an option to disable subtitle downloading in ffmpeg to prevent duplicated traffic. |
If we won't spawn another thread for
What do you mean by duplicated? ffmpeg only follows
Do you want me to leave subtitle URLs handling in this PR? Also, I'm not sure regarding subtitles streams for various qualities. Flash player will use |
|
Haven't looked into the spec of M3U8 manifests. If each video segment corresponds to exactly one subtitle segment, then the subtitle can be downloaded immediately after the video segment.
I have seen "ffmpeg can handle subtitles" somewhere. I found it's not merged yet (https://trac.ffmpeg.org/ticket/2833), so the poster may be using a custom fork of ffmpeg.
Comment it out and add some comments. |
|
Btw, ffmpeg HLS dumper seems to be very slow, audio goes out of sync for high-res streams. I get much better results with |
|
Could you try again by |
|
I tried it, but native downloader seems to be not working on vlive streams. You could try it right away with |
|
If I pass it |
|
Yep |
youtube_dl/extractor/vlive.py
Outdated
|
|
||
| def _live(self, video_id, webpage, live_params): | ||
| formats = [{ | ||
| 'url': vid['cdnUrl'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should extract chunklists via self._extract_m3u8_formats here.
|
A bit unrelated question. Is it possible to get subtitle URL without downloading it, similar to |
|
No direct options. You can dump the JSON dict with |
youtube_dl/extractor/vlive.py
Outdated
| @@ -1,8 +1,12 @@ | |||
| # coding: utf-8 | |||
| from __future__ import division | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merge them: from __future__ import division, unicode_literals
youtube_dl/extractor/vlive.py
Outdated
| r'vlive\.tv\.video\.ajax\.request\.handler\.init\(\s*"[0-9]+"\s*,\s*"[^"]*"\s*,\s*"[^"]+"\s*,\s*"([^"]+)"', | ||
| webpage, 'key') | ||
| video_params = self._search_regex( | ||
| r'vlive\.tv\.video\.ajax\.request\.handler\.init\((.*)\)', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better to use ([^;]+) instead of (.*). It reduces the probability of matching something unexpected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Semicolon might appear in live params, it's not safe to match like that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK Then (.+)
|
@yan12125 addressed all comments. Tested on few lives, seems like it works as expected. |
|
There is issue with default format ids generated by |
|
You can override |
|
Extended |
|
What's the meaning of |
|
Fixed. |
youtube_dl/extractor/common.py
Outdated
| @@ -1139,7 +1139,8 @@ def _extract_m3u8_formats(self, m3u8_url, video_id, ext=None, | |||
| if m3u8_id: | |||
| format_id.append(m3u8_id) | |||
| last_media_name = last_media.get('NAME') if last_media and last_media.get('TYPE') != 'SUBTITLES' else None | |||
| format_id.append(last_media_name if last_media_name else '%d' % (tbr if tbr else len(formats))) | |||
| if last_media_name or not live: | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be and.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also add some comments for why not adding bitrates for live streams.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be and
We may result in identical format ids in that case. Though there might be several streams without NAME too. What about simple counter? E.g. fid, fid-1, fid-2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Such a feature is already implemented in YoutubeDL: https://github.com/rg3/youtube-dl/blob/bf09af3/youtube_dl/YoutubeDL.py#L1318-L1322
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Though there might be several streams without NAME too.
according to the standard NAME attribute is REQUIRED.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Such a feature is already implemented
Ah, that's cool, fixed.
according to the standard NAME attribute is REQUIRED
Not in case of vlive... (See example playlist above.)
|
Looks good. The final bit: add |
|
Fixed and squashed. |
|
Merged. Thanks for the work and sorry for the rushes. |
|
Thanks! |
Fixes #9321
This is WIP, I don't know how to handle HLS subtitles yet. But otherwise everything seems to be working.