Skip to content

AVI support #117

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

Merged
merged 4 commits into from
Jan 18, 2018
Merged

AVI support #117

merged 4 commits into from
Jan 18, 2018

Conversation

VictorLoktev
Copy link
Contributor

No description provided.


public override bool IsCloserToEnd(long numberOfBytes)
{
return _stream.Position + numberOfBytes >= _stream.Length;
Copy link
Owner

Choose a reason for hiding this comment

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

FYI the only reason I haven't merged this yet is because I want to look into this a bit more. Does Stream.Length ever throw or cause the stream to read to the end? Not all streams know their lengths. I'm incredibly busy at the moment but will get to this PR along with others that are pending ASAP.

I'm happy for anyone interested in the project to review PRs. It definitely helps improve quality, and I'm all for it.

@VictorLoktev
Copy link
Contributor Author

VictorLoktev commented Jan 18, 2018

I used Stream.Length in public override bool IsCloserToEnd(long numberOfBytes) like someone did in method public override void Skip(long n) just 2 method avove:
`

    public override void Skip(long n)
    {
        if (n < 0)
            throw new ArgumentException("n must be zero or greater.");
        if (_stream.Position + n > **_stream.Length**)
            throw new IOException("Unable to skip past of end of file");

        _stream.Seek(n, SeekOrigin.Current);
    }

`
You are right, not all streams support Seek or Length. Fortunately all implementations in the project work with files and support these methods. If you switch to reading data from internet streams it could bring problems.

@drewnoakes drewnoakes merged commit b779d22 into drewnoakes:master Jan 18, 2018
@drewnoakes drewnoakes changed the title Tabs replaced with spaces and other minor code format changes. AVI support Jan 18, 2018
@drewnoakes
Copy link
Owner

Thanks for the response. Indeed it looks like if this would be a problem, then it already is. Thanks for a great contribution.

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.

2 participants