1748 replies to this topic

[zakalibit]

That might be problematic, on a different git where I did the work originally I still have big chunks in commits, what I did was m_ prefixed all variables, as I could not distinguish easily in the code a class variable from a local, then added chunked transfer and filtration and some wrappers rework. So what you're asking here it is doable but very time consuming. I have the code running on my dream and amico alien, I'm an IPTV heavy user, so the code is very well tested in the real day to day use. 

 

So, to reiterate, I will not do small commits in the near future as the time for me is at the premium, as well I do understand your concerns, so I do not mind if you will not merge the change.

One last argument I'd have is that  1st service as is now in the openpli is not usable, so you wont loose anything if you merge my changes, as well I'd be happy to look at any issues that might arise in the future with the code.

Edited by zakalibit, 6 November 2013 - 12:07.

[Erik Slagter]

Then I think we must consider importing the patch locally (not on sourceforge) and then we split it up somewhat and then commit it.

[littlesat]

But can you at least look at the white spaces...

 

WIth those we mean extra spaces that are not required at the end of lines of codes.... empty lines with no code and spaces.... and mixed up taps/spaces...

 

Examples below, with some speudo code where I replace the spaces by underscores...

 

if (x == y)_________

{

____________________

   x = y;

}__________________________

 

This is a simple method to keep code clean....

 

And indeed it is better to have patches "splintered" into functional changes.

Edited by littlesat, 6 November 2013 - 12:14.

[Erik Slagter]

Also a little bit of coding style would be appreciated. I saw a line like this: "if (a) do_b;". This is not considered good practise. Please split it into two lines.

[zakalibit]

Also a little bit of coding style would be appreciated. I saw a line like this: "if (a) do_b;". This is not considered good practise. Please split it into two lines.

this is matter of taste, but in my opinion good practice is for one liner if statement:

 

if(a) do_b;

 

or

 

if(a) {

    do_b;

}

 

i'd like to hear your argument why oneliner if statement is not a good practice I want to hear practical arguments

 

 

sorry about the white spaces, it is probably a copy paste artifact, will fix them

[littlesat]

The white spaces are most time something the editor does induce... e.g. a good editor makes the correct intends for your... but in between it also creates sometimes unintended indents...

[zakalibit]

And indeed it is better to have patches "splintered" into functional changes.

I hear you I'd have the same reaction for sure, but when was writing the code I was not thinking about this just was pissed of the way the streams were played, I spent a lot of time to come-up with this code, I had few other worse incarnations of this code with circular buffers and reconnect feature, but after long time staring at the enigma logs I realized that they give more trouble the benefit so the current version is the bare minimum ( well, if you don take the cosmetics in consideration )

Edited by zakalibit, 6 November 2013 - 12:35.

[zakalibit]

fixed whitespaces and indentation

https://sourceforge....ge-requests/26/

https://sourceforge....22b2252501e339/

[zakalibit]

Actually the best to review the code is not to look at the diff but at the code it self, I reckon peterg is very familiar with the httpstream code, so he can look at my version of file, should not be hard to understand my code, there is nothing too complex

[Erik Slagter]

Also a little bit of coding style would be appreciated. I saw a line like this: "if (a) do_b;". This is not considered good practise. Please split it into two lines.

this is matter of taste, but in my opinion good practice is for one liner if statement:

...

i'd like to hear your argument why oneliner if statement is not a good practice I want to hear practical arguments

 

There is nothing practical about sharing code in a project. It's not about that I like it this way (or not), it's about a common coding style. Have you ever tried to read/change a project that has many different styles through de code? That's not something you want.

 

BTW idents we do with tabs of four "spaces", not spaces. I don't know if you already adhere to that.

[zakalibit]

There is nothing practical about sharing code in a project. It's not about that I like it this way (or not), it's about a common coding style. Have you ever tried to read/change a project that has many different styles through de code? That's not something you want.

 

BTW idents we do with tabs of four "spaces", not spaces. I don't know if you already adhere to that.

 

Ok, I see no point having an argument over this,  just one thing to keep in might is coding style has 2 aspect practical and cosmetic, which is a matter of taste, the practical one is about code readability and how much code you can see on one page on the screen, this is how I see it.

 

I'll retab the code this is another one that I'd disagree, it is a horizontal space waster.

Edited by zakalibit, 6 November 2013 - 13:15.

[Erik Slagter]

I'll retab the code this is another one that I'd disagree, it is a horizontal space waster.

I don't understand?

 

You mean you're using some editor that cannot configure the tab width?

[zakalibit]

I'll retab the code this is another one that I'd disagree, it is a horizontal space waster.

I don't understand?

 

You mean you're using some editor that cannot configure the tab width?

 

you're right I've over-exaggerated on this one

[zakalibit]

ok converted to tabs https://sourceforge....e9031c1a89495d/

[Erik Slagter]

I'm a great fan of tabs for indenting:

- every coder can configure their favourite tab width locally

- every indent is one character in the code, easier for editing, finding etc. imho

Edited by Erik Slagter, 6 November 2013 - 13:23.

[zakalibit]

I'm a great fan of tabs for indenting:

- every coder can configure their favourite tab width locally

- every indent is one character in the code, easier for editing, finding etc. imho

ok you sold it to me

[pieterg]

what I did was m_ prefixed all variables, as I could not distinguish easily in the code a class variable from a local

The convention used in that particular piece of code is such that local variables don't use capitals, and class members use camelcase (with a low-case first character).
I admit e2 is a bit of a mess, with different naming conventions.
I usually stick to the naming conventions of the particular class I'm working on, with a slight deprecation of the Hungarian style, because that reminds me of Windows programmers (no offense).
So if you have to change the naming convention, change it for the whole class at once, in a seperate commit.
Please don't mix cosmetics with functionality, in a single commit.

Also, those int-->size_t changes are more or less cosmetic, because they don't fix actual bugs (there are no signed/unsigned mixups afaics).

With that out of the way, I'm very interested in your fixes.
Please add a detailed description of what you are changing, and why you are changing it, in each of your commits.
Then we'll be more than happy to merge or cherrypick your changes.

[zakalibit]

I'm not sure I'll do that soon, all you say makes sense but you know you do not think about all this while having a beer in the evening and trying to make streaming working properly and I wasn't planing to push it in to pli in the first place, I just fixed for my self at the time, so I can enjoy watching iptv. 

Let me digest all this and see if I'll get in to the mood of redoing all this.

[theparasol]

Proof of concept is done.
Perhaps someone else can do the wanted recoding?

[zakalibit]

Proof of concept is done.
Perhaps someone else can do the wanted recoding?

I could assist with code review and general help as time permits.