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

Use y* instead of y# #6975

Closed
Yay295 opened this issue Feb 25, 2023 · 2 comments · Fixed by #6988
Closed

Use y* instead of y# #6975

Yay295 opened this issue Feb 25, 2023 · 2 comments · Fixed by #6988

Comments

@Yay295
Copy link
Contributor

Yay295 commented Feb 25, 2023

#6974 changed

if (!PyArg_ParseTuple(args, "y#", &buffer, &bufsize)) {
to use y* instead of y# so that it can accept a memoryview object, and notes that y* is actually the recommended way to accept binary data. y# is used in a few other locations; should these be changed as well?

if (!PyArg_ParseTuple(args, "y#:frombytes", &ptr, &bytes)) {

"y#(ii)(iiii):_load",

"ss|nnny#",

"ss|nnnnnnnnOz#y#y#",

if (!PyArg_ParseTuple(args, "sy#", &rawmode, &palette, &palettesize)) {

if (!PyArg_ParseTuple(args, "y#", &values, &length)) {

"y#y#",

args, "O!y#", &Imaging_Type, &imagep, &glyphdata, &glyphdata_length)) {

if (!PyArg_ParseTuple(args, "y#:profile_frombytes", &pProfile, &nProfile)) {

"etn|nsy#n",

list_name = Py_BuildValue("y#", name.string, name.string_len);

axis_name = Py_BuildValue("y#", name.string, name.string_len);

"y#iiifss#iis#s#",

@radarhere
Copy link
Member

Some of the code you've highlighted is using Py_BuildValue, which doesn't look like it takes y*..

Just to write it out, the other functions you've highlighted are WebPEncode_wrapper, getfont, cms_profile_fromstring, _font_new, _get_projection, _putpalettealphas, _putpalete, PyImaging_JpegEncoderNew, PyImaging_ZipEncoderNew, PyImaging_DrawWmf and _frombytes.

@radarhere
Copy link
Member

I've created PR #6988 for _frombytes.

For some of methods, the data is read first. So there isn't a basic situation where the data passed to the C layer will be a memoryview.

  • PyImaging_DrawWmf is only called internally from Image.core.drawwmf in
    Image.core.drawwmf(im.fp.read(), im.size, self.bbox),
  • cms_profile_fromstring is only called internally from

    Pillow/src/PIL/ImageCms.py

    Lines 187 to 191 in 1457d2c

    self._set(core.profile_frombytes(f.read()))
    return
    self._set(core.profile_open(profile), profile)
    elif hasattr(profile, "read"):
    self._set(core.profile_frombytes(profile.read()))
    .
  • getfont is only called internally from

    Pillow/src/PIL/ImageFont.py

    Lines 236 to 255 in 1457d2c

    def load_from_bytes(f):
    self.font_bytes = f.read()
    self.font = core.getfont(
    "", size, index, encoding, self.font_bytes, layout_engine
    )
    if is_path(font):
    if sys.platform == "win32":
    font_bytes_path = font if isinstance(font, bytes) else font.encode()
    try:
    font_bytes_path.decode("ascii")
    except UnicodeDecodeError:
    # FreeType cannot load fonts with non-ASCII characters on Windows
    # So load it into memory first
    with open(font, "rb") as f:
    load_from_bytes(f)
    return
    self.font = core.getfont(
    font, size, index, encoding, layout_engine=layout_engine
    )

For WebPEncode_wrapper, it is called from _webp.WebPEncode in

data = _webp.WebPEncode(
im.tobytes(),

and im.tobytes() won't create a memoryview.

The other two instances were PyImaging_JpegEncoderNew and PyImaging_ZipEncoderNew. When I attempted to save a Pillow image to a memoryview, I got AttributeError: 'memoryview' object has no attribute 'write', so that isn't appropriate for that scenario.

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 a pull request may close this issue.

2 participants