Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 Video Llava #29733
Add Video Llava #29733
Changes from 1 commit
dce6678
72626df
8cca731
4ea4f70
c36819d
c1a8fd5
c591c75
5ff8d18
a6bc68d
eb309ed
2f46f6c
6b51b7e
e112958
5cb6163
930147d
24ec2b3
142bfc0
fdec895
e83251c
4fcfe72
327030d
33289a5
dfef75a
ebf1042
aa1b278
7802922
9fce414
e8b4569
bb1cc26
e2e92b2
5c77fff
99518cb
451fd72
95a9a01
347fa8c
3e2f1b4
3cd1222
242703a
9c1a10d
b4145e1
5803d5a
975d959
7f30e3b
6bdad81
a817f31
dba80e2
6b3eafb
ba4e125
6cc8af1
885a5ae
377aafe
637b197
a411347
0d83eaf
8134039
8e15514
5d1e976
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you expand this to show a full example using the model? Users typically want to just copy-paste
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should verify it's one of the two valid types here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
self.vision_feature_select_strategy
is checked a few lines above, inself._get_vision_features()
. We try to get the feature and if not raise ValueErrorThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should add tests for the image processor - in particular to test that it correctly handles just images, just videos and image + video inputs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added tests, but there is one thing to note. If we call directly the ImageProcessor class, it requires and argument
images
to be present. A workaround is to pass explicitlyimages=None
for VideoLlavaImageProcessor, which I did for the tests.I can override call and to make the argument
images = None
. so that it is optional, but not sure how good is overriding call. Also, I do not think many ppl call image processor explicitly.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the image processor takes both
images
andvideos
as input, and only one of them is required, then settingimage = None
seems reasonable