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

Get higher video and StoryItem image quality if logged-in #712

Merged
merged 7 commits into from
Aug 21, 2020

Conversation

fireattack
Copy link
Collaborator

Similar to 610b624, this commit tires to get highest quality of video from iPhone API.

@fireattack
Copy link
Collaborator Author

fireattack commented Jun 30, 2020

A note/caveat: iPhone ones always have equal or higher resolution AFAIK. But when the resolution is the same, the two files are different and while the difference is minimal, iPhone one likely would have lower bit-rate.

So, we can further improve it by comparing dimensions first (but I don't know if it's easy to get the dimensions of video of the web version).

@aandergr
Copy link
Member

aandergr commented Jul 4, 2020

I tried this out with the IGTV posts from natgeo. Whereas it indeed could fetch higher-resolution videos for some posts (720x1280 instead of 640x1136), it failed with a "400 Bad Request" error message in many cases when fetching the iphone struct. I think prior merging this, we should somehow eliminate this error. Maybe there is some regularity where the iphone struct is available, so that we only do the iphone request when it is safe?

@fireattack
Copy link
Collaborator Author

I will look into it.

In the meantime, I noticed that 610b624 doesn't seem to fetch high resolution image for stories either. We may want to check that too.

@aandergr
Copy link
Member

aandergr commented Jul 9, 2020

Thanks!

In the meantime, I noticed that 610b624 doesn't seem to fetch high resolution image for stories either. We may want to check that too.

Yes, that commit only addresses the quality of Posts, not of StoryItems.

@aandergr aandergr changed the base branch from master to upcoming/v4.6 August 2, 2020 09:17
@fireattack
Copy link
Collaborator Author

fireattack commented Aug 15, 2020

Hi, finally have some time to check this (sorry!).

I found out that it's the UA we're using
Instagram 123.1.0.26.115 (iPhone12,1; iOS 13_3; en_US; en-US; scale=2.00; 1656x3584; 190542906)
is the cause of the 400 problem. After replacing it to an Android one,
Instagram 146.0.0.27.125 Android (23/6.0.1; 640dpi; 1440x2560; samsung; SM-G930F; herolte; samsungexynos8890; en_US),
I no longer have any 400 issue when downloading all the IGTV posts from natgeo.

The author of MaxURL (@qsniyg) also told me they block requests from older versions of the client, so maybe it's not iPhone that is an issue but its version just being too old.

I haven't find any side-effect of this change yet, should we just swap this new UA in?

@fireattack
Copy link
Collaborator Author

And I honestly don't know how to get the current iPhone UA either as I don't have any iOS devices. So if you can help test on that it would be very appreciated too!

@fireattack
Copy link
Collaborator Author

fireattack commented Aug 15, 2020

On an unrelated note, I guess it's impossible to merge from upstream on my fork (for testing purpose) without polluting this PR with a "merged from xxx" commit? I guess I will just rebase then (totally newbie of git).

@fireattack fireattack changed the base branch from upcoming/v4.6 to master August 15, 2020 14:15
@fireattack fireattack changed the base branch from master to upcoming/v4.6 August 15, 2020 14:18
@aandergr
Copy link
Member

aandergr commented Aug 17, 2020

I found out that it's the UA we're using
Instagram 123.1.0.26.115 (iPhone12,1; iOS 13_3; en_US; en-US; scale=2.00; 1656x3584; 190542906)
is the cause of the 400 problem. After replacing it to an Android one,
Instagram 146.0.0.27.125 Android (23/6.0.1; 640dpi; 1440x2560; samsung; SM-G930F; herolte; samsungexynos8890; en_US),
I no longer have any 400 issue when downloading all the IGTV posts from natgeo.

Good call! I found out that just taking the Android Instagram version number and leaving the rest as is also works, i.e. Instagram 146.0.0.27.125 (iPhone12,1; iOS 13_3; en_US; en-US; scale=2.00; 1656x3584; 190542906). I once found out that only changing the resolution indeed has an effect on the returned image versions. Generally I am very unsure about the side-effects of changing the user agent string, so maybe we should change as little as possible.

And I honestly don't know how to get the current iPhone UA either as I don't have any iOS devices. So if you can help test on that it would be very appreciated too!

This user agent is mostly of a trial-and-error origin. I just did some tests with the "new" Instagram 146 iPhone agent and it seems to work fine :)

On an unrelated note, I guess it's impossible to merge from upstream on my fork (for testing purpose) without polluting this PR with a "merged from xxx" commit? I guess I will just rebase then (totally newbie of git).

Rebasing is fine. Actually I prefer rebasing, it produces a cleaner history than merging.

@fireattack fireattack changed the title Get higher video quality if logged-in Get higher video and StoryItem image quality if logged-in Aug 20, 2020
@fireattack
Copy link
Collaborator Author

fireattack commented Aug 21, 2020

Thanks for the suggestion, I've changed that.

I think that's all I have for this PR.

Copy link
Member

@aandergr aandergr left a comment

Choose a reason for hiding this comment

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

Thanks! I rebased it once again to current master and tested it and I can confirm that it works fine and downloads higher-resolution StoryItem pictures and Post videos that have a higher-resolution while having a lower bit rate. Also, I tested all other functions that use the iPhone endpoint and they still work perfectly.

One thing that needs to be addressed though is that StoryItem.__init__ loads the iphone_struct from node['iphone_struct'], but StoryItem._asdict() does not yet put it there.

instaloader/structures.py Show resolved Hide resolved
@aandergr aandergr merged commit b57bbe2 into instaloader:upcoming/v4.6 Aug 21, 2020
@aandergr
Copy link
Member

Thanks! Merged into upcoming/v4.6.

@psonnosp
Copy link

Do you need to add anything except --stories to make this work when running on the win10 standalone?
Because I don't notice any difference with previous versions.

@aandergr
Copy link
Member

It should automatically retrieve the best available quality if logged-in

  • for stories, both with :stories and --stories, and
  • for regular posts that are videos.

The change is included since version 4.6a1.

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

Successfully merging this pull request may close these issues.

None yet

4 participants