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

handle invalid images #2312

Merged
merged 6 commits into from
Sep 5, 2024
Merged

handle invalid images #2312

merged 6 commits into from
Sep 5, 2024

Conversation

irexyc
Copy link
Collaborator

@irexyc irexyc commented Aug 15, 2024

Motivation

Handle invalid image with invalid url, invalid base64 or truncated data.

related #2302

# Load image from local path
img = Image.open(image_url)
# check image valid
img = img.convert('RGB')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a better way to validate the image?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Currently all models will convert the image to RGB, and checking here will not increase the time consumption

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a better way to validate the image?

I'm not sure if load() function satisfied the need. Besides, if convert('RGB') is a must for all the model, we may do it here and remove them from forward function.

@lvhan028 lvhan028 requested a review from AllentDan September 5, 2024 08:43
Copy link
Collaborator

@AllentDan AllentDan left a comment

Choose a reason for hiding this comment

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

Did not pass the following UT:

def test_load_image_base64():
    im = PIL.Image.new(mode='RGB', size=(200, 200))
    encoded = encode_image_base64(im)
    url = f'data:image/jpeg;base64,{encoded}'
    load_image(url)
    import pytest
    with pytest.raises(ValueError):
        load_image(url[:-100])

Ref: #1615

Update, it is ok since we create dummy image now.

Copy link
Collaborator

@AllentDan AllentDan left a comment

Choose a reason for hiding this comment

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

LGTM. We may validate the image outside generate function in the future.

@lvhan028 lvhan028 merged commit f32fc53 into InternLM:main Sep 5, 2024
5 checks passed
@allen-ash
Copy link

allen-ash commented Sep 10, 2024

Considering of VLM service, if the image provided by the request is invalid, why bother to generate a dummy image? Is this gonna return the wrong answer for the request?

@irexyc
Copy link
Collaborator Author

irexyc commented Sep 10, 2024

@allen-ash

A reasonable approach is to return an error code if the inputs is invalid. The reason for doing this is that some people have reported that the service will crash if the input is invalid and using a dummy image requires less code changes. We may use error code in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants