-
Notifications
You must be signed in to change notification settings - Fork 25
#132 allow users to optionally provide an output buffer when calling transcode #136
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
Changes from 18 commits
f5b64a6
9e5bb40
090cb9c
b6d5f9d
2ce1efb
49b5b82
78d2b3b
20e82d9
c010983
829c655
3d17ac8
1bf636d
cd753f6
0d1e934
3b90dd6
5ec6bf3
a11043e
e926f4f
c5cd743
8931cb4
b743bcd
72e6d0a
8134ab1
93c28d4
b3f444b
9a4b867
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,17 +26,17 @@ mutable struct Buffer | |
| # the total number of transcoded bytes | ||
| transcoded::Int64 | ||
|
|
||
| function Buffer(size::Integer) | ||
| return new(Vector{UInt8}(undef, size), 0, 1, 1, 0) | ||
| function Buffer(data::Vector{UInt8}, keepbytes::Integer=length(data)) | ||
| return new(data, 0, 1, keepbytes+1, 0) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like that we now have a single inner constructor. Could we do a sanity check here to see if keepbytes is not greater than the length of data and also nonnegative?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
| end | ||
| end | ||
|
|
||
| function Buffer(data::Vector{UInt8}) | ||
| return new(data, 0, 1, length(data)+1, 0) | ||
| end | ||
| function Buffer(size::Integer = 0) | ||
| return Buffer(Vector{UInt8}(undef, size), 0) | ||
| end | ||
|
|
||
| function Buffer(data::Base.CodeUnits{UInt8}) | ||
| return Buffer(Vector{UInt8}(data)) | ||
| function Buffer(data::Base.CodeUnits{UInt8}, keepbytes::Integer=length(data)) | ||
| return Buffer(Vector{UInt8}(data), keepbytes) | ||
| end | ||
|
|
||
| function Base.length(buf::Buffer) | ||
|
|
@@ -199,6 +199,11 @@ function copydata!(buf::Buffer, data::Ptr{UInt8}, nbytes::Integer) | |
| return buf | ||
| end | ||
|
|
||
| # Copy data from `data` to `buf`. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we make this a docstring?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we really want this? All these things are internal.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In my mind, it's an internal thing. If you're a user, you don't care about internals. If you care about internals you might as well read the code comments. That said, if you feel strongly about this I can make them docstrings :-) |
||
| function copydata!(buf::Buffer, data::Buffer, nbytes::Integer = length(data)) | ||
| return copydata!(buf, bufferptr(data), nbytes) | ||
| end | ||
|
|
||
| # Copy data from `buf` to `data`. | ||
| function copydata!(data::Ptr{UInt8}, buf::Buffer, nbytes::Integer) | ||
| # NOTE: It's caller's responsibility to ensure that the buffer has at least | ||
|
|
||
|
baumgold marked this conversation as resolved.
|
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -2,7 +2,10 @@ | |||
| # ========= | ||||
|
|
||||
| """ | ||||
| transcode(::Type{C}, data::Vector{UInt8})::Vector{UInt8} where C<:Codec | ||||
| transcode( | ||||
| ::Type{C}, | ||||
| data::Union{Vector{UInt8},Base.CodeUnits{UInt8}}, | ||||
| )::Union{Vector{UInt8},Base.CodeUnits{UInt8}} where {C<:Codec} | ||||
|
|
||||
| Transcode `data` by applying a codec `C()`. | ||||
|
|
||||
|
|
@@ -27,21 +30,34 @@ julia> String(decompressed) | |||
|
|
||||
| ``` | ||||
| """ | ||||
| function Base.transcode(::Type{C}, data::ByteData) where C<:Codec | ||||
| function Base.transcode(::Type{C}, args...) where {C<:Codec} | ||||
| codec = C() | ||||
| initialize(codec) | ||||
| try | ||||
| return transcode(codec, data) | ||||
| return transcode(codec, args...) | ||||
| finally | ||||
| finalize(codec) | ||||
| end | ||||
| end | ||||
|
|
||||
| _default_output_buffer(codec, input) = Buffer( | ||||
| initial_output_size( | ||||
| codec, | ||||
| buffermem(input) | ||||
| ) | ||||
| ) | ||||
|
|
||||
| """ | ||||
| transcode(codec::Codec, data::Vector{UInt8})::Vector{UInt8} | ||||
| transcode( | ||||
| codec::Codec, | ||||
| data::Union{Vector{UInt8},Base.CodeUnits{UInt8},Buffer}, | ||||
| [output::Union{Vector{UInt8},Base.CodeUnits{UInt8},Buffer}], | ||||
| )::Union{Vector{UInt8},Base.CodeUnits{UInt8}} | ||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This signature line implies we can mix and match Can we document what the output type will be?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see no reason to forbid mix and match types. It's more flexibility for users at no performance cost. I'm not against enforcing the same type either, you tell me. Regarding the return value, I believe it's always [1] TranscodingStreams.jl/src/transcode.jl Line 132 in e926f4f
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I consider this done, let me know. |
||||
|
|
||||
| Transcode `data` by applying `codec`. | ||||
|
|
||||
| If `output` is unspecified, then this method will allocate it. | ||||
|
|
||||
| Note that this method does not initialize or finalize `codec`. This is | ||||
| efficient when you transcode a number of pieces of data, but you need to call | ||||
| [`TranscodingStreams.initialize`](@ref) and | ||||
|
|
@@ -59,7 +75,9 @@ julia> codec = ZlibCompressor(); | |||
|
|
||||
| julia> TranscodingStreams.initialize(codec) | ||||
|
|
||||
| julia> compressed = transcode(codec, data); | ||||
| julia> compressed = Vector{UInt8}() | ||||
|
|
||||
| julia> transcode(codec, data, compressed); | ||||
|
|
||||
| julia> TranscodingStreams.finalize(codec) | ||||
|
|
||||
|
|
@@ -76,9 +94,11 @@ julia> String(decompressed) | |||
|
|
||||
| ``` | ||||
| """ | ||||
|
baumgold marked this conversation as resolved.
|
||||
| function Base.transcode(codec::Codec, data::ByteData) | ||||
| input = Buffer(data) | ||||
| output = Buffer(initial_output_size(codec, buffermem(input))) | ||||
| function Base.transcode( | ||||
|
baumgold marked this conversation as resolved.
|
||||
| codec::Codec, | ||||
| input::Buffer, | ||||
| output::Buffer = _default_output_buffer(codec, input), | ||||
| ) | ||||
|
baumgold marked this conversation as resolved.
baumgold marked this conversation as resolved.
|
||||
| error = Error() | ||||
| code = startproc(codec, :write, error) | ||||
| if code === :error | ||||
|
|
@@ -121,6 +141,12 @@ function Base.transcode(codec::Codec, data::ByteData) | |||
| throw(error[]) | ||||
| end | ||||
|
|
||||
| Base.transcode(codec::Codec, data::Buffer, output::ByteData) = | ||||
| transcode(codec, data, Buffer(output)) | ||||
|
|
||||
| Base.transcode(codec::Codec, data::ByteData, args...) = | ||||
| transcode(codec, Buffer(data), args...) | ||||
|
|
||||
| # Return the initial output buffer size. | ||||
| function initial_output_size(codec::Codec, input::Memory) | ||||
| return max( | ||||
|
|
||||
Uh oh!
There was an error while loading. Please reload this page.