Das Erste: Livestream not working (3 Viewers)

[SilentBob]

Hi
Selected each highlighted item shown in the attached screen shots starting at DE1.
Using the same procedure on my unmodified HTPC (MP 1.19) the same vid plays OK as do several other I tested, but of course the live streams don't.

UPDATE:-
Just wanted to clarify that when I said all catchup content failed, I actually meant all catchup content in the Das Erste category. ZDF Mediatek seems to be working 100% all live (even Arte) and catchup as are WDR. Deutsche Welle live stream in Das Erste is good and catchup from its own category, also both Arte (Deutsch and French) steams work.

View attachment 197828 View attachment 197829 View attachment 197830 View attachment 197831

please try with updated files here
Das Erste: Livestream not working
if you see other issues please let me know

FYI, it's not necessary to put the whole onlinevideos folder (nor the mpe1) here just to fix some site.
That is, assuming you only changed things in the DasErsteMediathekUtil.cs or other files in the OnlineVideos.Sites.offbyone folder.
You only need the OnlineVideos.Sites.offbyone.dll.
If you can commit the changes I'm more than happy publishing that dll so that it will be automatically picked up in the onlinevideos plugin

before i create a pull request i want to test it at least a little more
btw. i also added a small patch into OnlineVideos.dll, is this a problem for a pull request?

 

[joecrow]

please try with updated files here
Das Erste: Livestream not working
if you see other issues please let me know

On a very quick check it is looking good.
I will run a more thorough test later today and report back then but have no time right now, sorry.

 

[catavolt]

New 2.3.1.2 works as expected - all live streams and mediathek videos are played w/o errors
Well done

 

[joecrow]

Everything I have tried works perfect.
Good work.

 

[doskabouter]

...
before i create a pull request i want to test it at least a little more
btw. i also added a small patch into OnlineVideos.dll, is this a problem for a pull request?

Ok.
Patching onlinevideos.dll is no problem for a pullrequest, however if the working of DasErste is dependant on it, a new version of onlinevideos has to be released.
I do have some extra rights on the repo but sadly not enough to do a release...

 

[SilentBob]

Okay, so let me create two pull requests.
one without adapting onlinevideo.dll and pulling needed adaption as a hack into the site dll.

And one where I remove the hack from site dll and adapting the onlinevideo.dll, as it makes sense to have the changes there.

Then we can already update the site dll and wait for an online video update.

 

[doskabouter]

Ok, it seems I have a lot to learn to review a pull request. (Couldn't get it to merge in my local source using tortoisegit to get a bit of an overview)

3 remarks/questions:
- May I point you to the HlsPlaylistParser class? That may save you a lot of code (or maybe not, didn't do an in-depth analysis)
- That change in onlinevideos, couldn't that throw unwanted exceptions?
- Either way, until a new version of onlinevideos is released, I think it's better to put that part in the util itself, so everyone can benefit without having to manually copy files and such

 

[SilentBob]

3 remarks/questions:
- May I point you to the HlsPlaylistParser class? That may save you a lot of code (or maybe not, didn't do an in-depth analysis)

That's a good hint, i just wanted to fix the mediathek streams and did not look larger around. so my first intention was to quick fix it, and most of the code was already there from offbyone. In a next step i would have provided such a playlist parser as well, but good to see there is already one.
Just a question: Is it by intention that only one stream is returned per streamname (e.g. if there are Urls for same size and bitrate, only one is returned)

You asked me to provide the code as fast as possible

If you can commit the changes I'm more than happy publishing that dll so that it will be automatically picked up in the onlinevideos plugin

I can update the pull request or create another one, as i would like to do some clenaups as well.

- That change in onlinevideos, couldn't that throw unwanted exceptions?

not more than already in. i changed the last fallback which already today returns default(T), default(T) means in most cases: null

These are the default values for the more important types inside of CLR,

• Classes - null
• Nullable<T> - null
• Numerics structs (int, double, decimal, etc) - 0
• DateTime structs - 01/01/0001.
• Char structs - empty char.
• Bool structs - false

The Convert call does not throw an exception it tries to convert, if possible you get your type if not null... so similar to todays implementation

- Either way, until a new version of onlinevideos is released, I think it's better to put that part in the util itself, so everyone can benefit without having to manually copy files and such

I did not got this point, maybe you could give an example, or mark the code in pullrequest comment.
For me this pull request is just one fix of the mediathek(s) as I saw other issues as well. I would like to refactor some code and consolidate some communalties of the ard, zdf and wdr mediathek code

 

[doskabouter]

...
Just a question: Is it by intention that only one stream is returned per streamname (e.g. if there are Urls for same size and bitrate, only one is returned)

For now: yes, the user isn't going to be wanting one over the other I presumed

You asked me to provide the code as fast as possible
I can update the pull request or create another one, as i would like to do some clenaups as well.

Do you want me to wait for that?
Presumably no...

not more than already in. i changed the last fallback which already today returns default(T), default(T) means in most cases: null

These are the default values for the more important types inside of CLR,

Classes - null
Nullable<T> - null
Numerics structs (int, double, decimal, etc) - 0
DateTime structs - 01/01/0001.
Char structs - empty char.
Bool structs - false

The Convert call does not throw an exception it tries to convert, if possible you get your type if not null... so similar to todays implementation

Well, judging from the code I would expect JsonConvert.DeserializeObject to generate an exception if f.e. it isn't valid json, but I will try to understand it better. (Planning on deploying without this change anyway)

I did not got this point, maybe you could give an example, or mark the code in pullrequest comment.
For me this pull request is just one fix of the mediathek(s) as I saw other issues as well. I would like to refactor some code and consolidate some communalties of the ard, zdf and wdr mediathek code

Well, what I meant was that I'm planning on merging the pullrequest #164 (the one without onlinevideos modifications) so that everyone can just automatically update the site, and not having to wait for an onlinevideos update

 

[SilentBob]

Well, judging from the code I would expect JsonConvert.DeserializeObject to generate an exception if f.e. it isn't valid json, but I will try to understand it better. (Planning on deploying without this change anyway)

Well, what I meant was that I'm planning on merging the pullrequest #164 (the one without onlinevideos modifications) so that everyone can just automatically update the site, and not having to wait for an onlinevideos update

yes do this, merge without onlinevideo dll, which has a smaller impact, and everyone can have the fixes already.
As I will do some code cleanups and create another pull request for that, we can review it again and see then how we proceed with online videos dll.
Currently only one site plugin benefits from that change, and therefore it is also okay to have it just there in the specific siteutil