-
Notifications
You must be signed in to change notification settings - Fork 30
Bug fixes and improvements #6
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
base: master
Are you sure you want to change the base?
Conversation
cleeque
left a comment
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.
Thank you for your job. Nice MR. Could you please take a look at my comments? Have you tested it on real data?
| if 'records' in self.config['mediaData'] and len(self.config['mediaData']['records']) > 0: | ||
| return b64decode(self.config['mediaData']['records'][0]['token']).decode('utf-8') | ||
| else: | ||
| return b64decode(self.config['mediaData']['token']).decode('utf-8') | ||
| return b64decode(self.config['mediaData']['token']).decode('utf-8') |
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.
When records are not available yet, token can be retrieved from 'mediaData' directly. The same works for m3u8 playlist.
| --entity TiAR7aDs --pin 123-456-789 --resolution "640x360" | ||
| ``` | ||
|
|
||
| If resolution is not specified, the video with a highest one will be dowloaded. |
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 think this is important.
|
|
||
| def __init__(self): | ||
| parser = argparse.ArgumentParser(description='boomstream.com downloader') | ||
| parser.add_argument('--url', type=str, required=True) |
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 would say url is much readable for the most of the users.
| return filenames | ||
|
|
||
| def merge_chunks(self, key): | ||
| def merge_chunks(self, filenames, key, expected_result_duration): |
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.
Good idea with expected_result_duration. Have you tested it on real data?
| result_duration = float([line[len("duration="):] for line in result_format.split('\n') if line.startswith("duration=")][0]) | ||
| print(f"Result duration: {result_duration:.2f}") | ||
| print(f"Expected duration: {expected_result_duration:.2f}") | ||
| if abs(result_duration - expected_result_duration) > 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.
This is nice, but depends on ffmpeg output. Maybe we can make this check optional?
| page = open(result_path).read() | ||
| else: | ||
| r = requests.get(self.args.url, headers=headers) | ||
| r = requests.get(f'https://play.boomstream.com/{self.args.entity}', headers=headers, cookies=cookies) |
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 think for user would be easier to copy the URL from browser command line than extracting and copying entity argument manually.
|
Hi @asiunov for some reason you code doesn't work currently although much work was done. I am getting errors with decrypting the stream. I've created a small PR to fix the current code here to work. Would have been great to use yours. |
|
This is an example of how a ton of good fixes have to wait a few review comments forever... |
Changes
config['mediaData']['records']. I did not get where it comes from, I do not have it in my video configs, please, let me know if this is a special video type that I'm not aware of. I guess it is just an outdated format.