Skip to content
This repository was archived by the owner on Nov 19, 2020. It is now read-only.

Conversation

stigvoss
Copy link
Contributor

Added fix for issues #700 #699

Added pre-processing conditional statement to disable UseSeparateConnectionGroup in .NET Standard 2.0
Added check for stream.CanTimeout before setting ReadTimeout
Added check for stream.CanTimeout before reading from stream

  • If CanTimeout is false, use ReadAsync with cancellation token to timeout read action
  • Throw TimeoutException if ReadAsync times out

…ectionGroup in .NET Standard 2.0

Added check for stream.CanTimeout before setting ReadTimeout
Added check for stream.CanTimeout before reading from stream
 - If CanTimeout is false, use ReadAsync with cancellation token to timeout read action
 - Throw TimeoutException if ReadAsync times out
@cesarsouza
Copy link
Member

Thanks a lot @stigvoss. I see you even took care to use the same non-standard formatting currently being used in those files. That's a really great work.

Just to give some explanation on why those classes may look so different from the rest of the framework, it's because the MJPEGStream came from the AForge.NET project and they have been barely touched ever since. However, one day it would be nice to be able to go through those classes and update them with the same standards used in the rest of the framework (maybe a relevant issue to track this would be #95).

Again, thanks a lot for the contribution,
Cesar

@cesarsouza cesarsouza merged commit d40d801 into accord-net:development Jul 15, 2017
@stigvoss
Copy link
Contributor Author

No problem, I am happy to help. I am aware of Accord's roots in AForge as I used AForge earlier and even though I tried, I failed to stay with the current in-file code convention. :-) Do you have a file or document containing the preferred Accord.NET code convention? If so, I may refactor parts of the Accord.Video library at some point to follow the correct code conventions and possibly increase the testability, and readability of the code.

@cesarsouza
Copy link
Member

I do not have a document right now, but they are mostly Visual Studio's default conventions. Whatever Ctrl+E, Ctrl+D gives you should be the standard used across the framework, as this is what most people are likely to be used to.

@cesarsouza
Copy link
Member

cesarsouza commented Jul 16, 2017

Oh, one thing I just noticed: After the change, the project does not compile anymore when targeting .NET 3.5. I will try to commit a change in the next few minutes to make the .NET 3.5 version compile again.

cesarsouza added a commit that referenced this pull request Jul 16, 2017
cesarsouza added a commit that referenced this pull request Jul 16, 2017
@stigvoss
Copy link
Contributor Author

Yes, of course. Tasks, as used, was not introduced in .NET Framework till version 4.0 and therefore it makes completely sense. Thank you for correcting the issue.

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

Successfully merging this pull request may close these issues.

2 participants