Skip to content
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

Add support for PNG string encoding representation for Image #574

Open
rly opened this issue Apr 16, 2024 · 12 comments
Open

Add support for PNG string encoding representation for Image #574

rly opened this issue Apr 16, 2024 · 12 comments
Labels
category: extension proposed extensions priority: medium non-critical problem and/or affecting only a small set of NWB users
Milestone

Comments

@rly
Copy link
Contributor

rly commented Apr 16, 2024

@talmop suggested that we add support for storing a PNG byte stream as the data for an Image type. PNG uses lossless compression and typically compresses 4:1 to 10:1. It is better than gzip/blosc compression of the image data matrix. Some applications involve storing lots of images, like storing the training frames in ndx-pose, all in a single container.

This would be done in an extension. We could discuss with the TAB and community whether this should be integrated into core eventually. I am just creating an issue here to document the request for such an extension here before I forget.

In Python, one can read the PNG byte stream like so:

from PIL import Image
import io

image_data = ... # byte values of the image
image = Image.open(io.BytesIO(image_data))
image.show()

@lawrence-mbf and I could not find a Matlab function that creates a Matlab Image object from a byte stream. But in that case, you could write the bytes to a png file and use imread(filepath) to read the file from disk.

Similar extensions could be made for typical audio file formats and maybe video file formats. Related:
NeurodataWithoutBorders/pynwb#1647
https://nwb-overview.readthedocs.io/en/latest/faq.html#why-is-it-discouraged-to-write-videos-from-lossy-formats-mpg-mp4-to-internal-nwb-datasets

@rly rly added category: proposal proposed enhancements or new features category: extension proposed extensions and removed category: proposal proposed enhancements or new features labels Apr 16, 2024
@rly rly added this to the Future milestone Apr 16, 2024
@rly
Copy link
Contributor Author

rly commented May 10, 2024

To add: @magland wrote an MP4 numcodecs codec that is compatible with Zarr. On encode, it takes an array of frame data, writes it to a temporary mp4 file using opencv, and then stores the file byte stream. On decode, it writes the byte stream to a temporary mp4 file and then reads the frames using opencv into a numpy array. We could incorporate this into an NWB extension for MP4 data. We could also do something similar, as described above, for PNG data and other external formats.

@bendichter
Copy link
Contributor

I like the idea in principle, but we would need to tightly control what formats are allowed with clear justifications for what types of formats would qualify. Otherwise, this could turn into a backdoor for all sorts of unusual proprietary formats.

@rly
Copy link
Contributor Author

rly commented May 11, 2024

I experimented a bit with this and hit a roadblock: HDMF and the schema language do not support storage of raw binary data. I propose that we add a new "binary" dtype. hdmf-dev/hdmf-schema-language#34

@bendichter
Copy link
Contributor

How would this be represented in HDF5 and Zarr?

@rly
Copy link
Contributor Author

rly commented May 11, 2024

H5py recommends using the HDF5 OPAQUE data type which h5py maps to np.void. So we could map an mp4 file with 1000000 bytes to a dataset with shape (1000000,) and dtype V1.

Zarr supports np.void (V) arrays as well. https://zarr-specs.readthedocs.io/en/latest/v2/v2.0.html#data-type-encoding

I added some examples in hdmf-dev/hdmf-schema-language#34

@rly
Copy link
Contributor Author

rly commented May 12, 2024

Experimenting here but writing the data does not work yet. https://github.com/rly/ndx-mp4

@bendichter
Copy link
Contributor

Interesting change in Zarr v3

We are explicitly looking for more feedback and prototypes of code using the r*, raw bits, for various endianness and whether the spec could be made clearer.

@rly
Copy link
Contributor Author

rly commented May 13, 2024

On further thought, our use cases here don't really make sense for storage in Zarr. The goal of storing the MP4/png as binary is to package all the data in one file for easier data management and sharing. The Zarr stores we primarily use in NWB are distributed across many files. The written array, if there are no additional filters and chunking is not applied, would be equal to the original file, just with a different file extension.

@bendichter
Copy link
Contributor

I guess the one advantage would be a universal protocol, i.e. you could access the frames of a TIFF series and the frames of a behavioral video in the exact same way. Maybe this is better handled on the API level though?

@rly
Copy link
Contributor Author

rly commented May 29, 2024

@oruebel wrote:

I can see the advantage for image formats (e.g., PNG) where one typically reads full images and support for reading these formats in code is pretty good. However, for video formats, e.g., MP4, I don’t think this is a good option. It’s not easy to read the movie file with standard tools (even in code) without reading and converting the whole dataset and standard video-players can’t use the data either.

I’m also wondering, whether it would be more appropriate to implement these compression methods as plugins to HDF5 (e.g., in h5plugins) rather than making these extension that require users to compress/decompress the images themselves.

@rly
Copy link
Contributor Author

rly commented May 29, 2024

I guess the one advantage would be a universal protocol, i.e. you could access the frames of a TIFF series and the frames of a behavioral video in the exact same way. Maybe this is better handled on the API level though?

Yeah, I think this would be better handled at the API level.

@sneakers-the-rat
Copy link

sneakers-the-rat commented Aug 8, 2024

fwiw it already works in nwb-linkml (dynamictable branch)

from nwb_linkml.models.pydantic.core.v2_7_0.namespace import TimeSeries
ts = TimeSeries(
    name = "video", 
    data = {
        "value": "./monsterdon/godzilla_dies.mp4",
        "unit": "its a video idk if it has a unit"
    }
)

ts.data.value.shape
# (900, 360, 642, 3)

ts.data.value[0]
# array([[[0, 0, 0],
#        [0, 0, 0],
#        [0, 0, 0],
#        ...,
#        [0, 0, 0],
#        [0, 0, 0],
#        [0, 0, 0]]], dtype=uint8)

bc numpydantic is cool like that. and adding image support would just be writing another interface which would be like 100 lines.

(working on that need for the {"value"} dict and the additional .value subscripting rn, byproduct of the combination of attributes and array data at same level but fixable with a mixin that i already made but haven't injected yet)

but ya it would be cool to move all the Image classes into core.nwb.image, consolidate ImageReferences, Images, and ImageSeries, and add a Video class.

@stephprince stephprince added the priority: medium non-critical problem and/or affecting only a small set of NWB users label Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: extension proposed extensions priority: medium non-critical problem and/or affecting only a small set of NWB users
Projects
None yet
Development

No branches or pull requests

4 participants