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

/Stream/ byterange is broken #787

Closed
bigretromike opened this issue May 31, 2019 · 24 comments
Closed

/Stream/ byterange is broken #787

bigretromike opened this issue May 31, 2019 · 24 comments
Assignees

Comments

@bigretromike
Copy link
Contributor

/Stream/ byterange is broken because of Context is null.

Just a remainder

@Cazzar
Copy link
Member

Cazzar commented May 31, 2019

We should be using Request not HttpContext if you want the headers.

@Cazzar
Copy link
Member

Cazzar commented May 31, 2019

https://github.com/ShokoAnime/ShokoServer/blob/master/Shoko.Server/API/v1/Implementations/ShokoServiceImplementationStream.cs

Also which endpoint is actually broken, since you just pointed to an entire Controller being "broken"
I tested StreamVideo which seemed fine with chrome streaming it
MPC-HC was getting an error due to making a request that isn't implemented

@bigretromike
Copy link
Contributor Author

Nancy code worked fine, you could seek video without issue.
ASPNetCode don't work with seeking, it only move video +9sec ahead no matter where you want to watch it.
I said Controller because HttpContext was null, and you yourself point out that it shouldn't be null;

@bigretromike
Copy link
Contributor Author

Maybe is a matter of missing headers, because of null, maybe its because how controller wipe context each time it execute command. Can't tell, im not an ASPCoreNet guy. I was happy enough that I could bypass that error because before it didn't work at all, and all Nakamori users what where watching thing not on the server were affected.

@bigretromike
Copy link
Contributor Author

var request = Request;
same issue;

@Cazzar
Copy link
Member

Cazzar commented May 31, 2019

cayde@Kanade:~$ curl -v -H "Range: 0-100" -o /dev/null http://localhost:8111/Stream/10/1/False/file.mkv
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0*   Trying 127.0.0.1...
* Connected to localhost (127.0.0.1) port 8111 (#0)
> GET /Stream/10/1/False/file.mkv HTTP/1.1
> Host: localhost:8111
> User-Agent: curl/7.47.0
> Accept: */*
> Range: 0-100
>
< HTTP/1.1 200 OK
< Date: Fri, 31 May 2019 08:13:43 GMT
< Server: Kestrel
< Transfer-Encoding: chunked
< api-deprecated-versions: 1.0
<
{ [107 bytes data]```

and I removed the try-catch

@bigretromike
Copy link
Contributor Author

curl is not a player...
I removed try-catch and it does not work (the seek)

@Cazzar
Copy link
Member

Cazzar commented May 31, 2019

image
image
I would need more of a method to reproduce the issue.
Since as shown by those two images, replacing with Request

But that's pointing possibly more towards other issues.

As curl is the only thing right now I can easily use to test, as MPC does a HTTP OPTIONS beforehand.

@Cazzar
Copy link
Member

Cazzar commented May 31, 2019

And further to this chrome doesn't do byte ranges.

@bigretromike
Copy link
Contributor Author

Only thing I can see in wireshark is:

Range: bytes=0-

@bigretromike
Copy link
Contributor Author

@Cazzar
Copy link
Member

Cazzar commented May 31, 2019

What I need to try and debug this... is a way of replicating it @bigretromike as I said I need a way of reproducing the issue

@bigretromike
Copy link
Contributor Author

Install Kodi, Install Nakamori, go to any series, play any file, profit with debugging;
If you on same machine as server/have access to same path to series files there need to be change one line in nakamori.
The easiest/fastest way.

@bigretromike
Copy link
Contributor Author

or better, I can push that change to repo, so you dont have to change anything in nakamori by hand.

@Cazzar
Copy link
Member

Cazzar commented May 31, 2019

Is there a simpler for example other players and such that also do this? Since a setup with Kodi/Nakamori isn't the easiest

@da3dsoul
Copy link
Member

VLC does it if you feed it the URL I believe

@bigretromike
Copy link
Contributor Author

bigretromike commented May 31, 2019

its super simple!

  1. download & install kodi
  2. download https://shokunin.monogatari.pl/repo/repository.nakamori-dev/repository.nakamori-dev-1.0.0.zip
  3. open kodi, navigate to, 'gear icon' top left, system, addons, unknown sources = enable
  4. go back to main menu
  5. pick Add-ons (left menu), the 'package icon' top left, install from zip file, pick the zip you download,
  6. same as 5, but pick install from repository, nakamori dev-repository, video addons, nakamori.
  7. profit

edit: step 3 is not nessasary because if you skip it kodi will ask for it and navigate you there.
after this installing its updates depends of the setting so no more installing anything from hand.
if @da3dsoul did't use custom scripts from official repo we could skip 2,3,4,5, and do 1,6,7. :-)

@Cazzar
Copy link
Member

Cazzar commented May 31, 2019

If someone asks for an easier setup, it usually means that they want to replicate the issue at a core, not install a few things....

@bigretromike
Copy link
Contributor Author

bigretromike commented May 31, 2019

Sorry, @da3dsoul did wrote the original code and I never test it against with anything other than Kodi, so I cannot give you 100% sure info about anything else.
I would assume that ffplay should act similar because its whats inside kodi, but hell knows.
And I don't want to tell you 'hey check with mpv' while you waste time on that and it would end up working in mpv and not in kodi.

@Cazzar
Copy link
Member

Cazzar commented May 31, 2019

I found the issue, it seems to be the missing response headers, working on a way of fixing it properly

@Cazzar
Copy link
Member

Cazzar commented May 31, 2019

But yeah for future reference BRM, your initial report was saying the stream controller didn't support byte ranges.
Though the request that was done via curl suggested different

This would have been best to report as "Streaming files via HTTP does not allow seeking"

@Cazzar Cazzar closed this as completed in cc0ed2f May 31, 2019
@bigretromike
Copy link
Contributor Author

I can confirm that this is finally working.
I back what I said about you not being able to fix it, to be honest I was counting that you will "show him what I'm mad of"
I initially didn't know what was the problem - I saw that someone comment out Nancy Controller and was thinking that someone messed up, so sorry about misinformation on that point.

Also for future reference if you rewrite something, It would be awesome if you check big part of it ;) and test if its compatible, or just delete legacy code away if its not tested :-) imo.

But than again, thank you for fixing the error from rewrite process, I knew that if you can't do it no one would (except old @da3dsoul) - you are the NetCore expert here 👍

I take your words to my heart and will try to give you less specific titles of issues, or those that im 100% sure of.

@Cazzar
Copy link
Member

Cazzar commented May 31, 2019

With the scale of changes at the time, testing was still to be done @bigretromike....
I am well aware for such large changes that is needed, but you are realizing you are complaining for me not testing a few small changes that were part of a large rewrite that took a months worth of effort to even get to a build able state?

And if you want to check on that, check the netcore branch and the amount of commits that were done.

@bigretromike
Copy link
Contributor Author

I'm not complaining, I'm happy that the very important function is working :-)

You did great job, I'm still looking forward 👍

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

No branches or pull requests

3 participants