BlobView only provides an API to read data. The gallery needs also to be able to write metadata back to media files and it already uses blobview for reading operations. We might consider to change the name of BlobView to something else if we provide both input and output APIs.
Attachment #807032 - Flags: review?(dflanagan)
Comment on attachment 807032 [details] pullRequest.html Diego, The code looks fine but I'm setting r- because I don't understand how these methods will be used in practice. BlobView is designed to allow you to view small portions of a blob without having to read the entire thing into memory. Once you use these write methods to modify a copy of a portion of a blob, what comes next? How do you get those edited bytes back into a complete blob? Do you plan to add a buildEditedBlob() function that will create a new blob with the edited array buffer sandwiched by slices from the original blob? I think I'd prefer to keep BlobView as a read-only abstraction, and have you do your EXIF edits in some other way. But maybe you can convince me otherwise. Feel free to find me on IRC If you want to discuss this more.
Attachment #807032 - Flags: review?(dflanagan) → review-
My understanding was that blobview is also a wrapper around the DataView API that makes easier to work with Blobs. It's confusing for the user to have a higher level API to read but for writing operations she has to directly use the DataView API. If a higher level API is offered I would expect it to be exhaustive. I don't want to have to learn both the low level API and the abstraction. It adds cognitive load since I have to keep more things in my head. We might think about refactor blobview in two parts. One API to efficiently access chunks of a blob and another API that just offers convenient read/write functions on top of DataView API (e.g readNullTerminatedUTF8Text).
For the moment I'm not going to modify the shared version of BlobView. I'm using a modified version of the library specific to the exif parser. https://github.com/dmarcos/exif-parser
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.