Skip to content

feat: store key frame indices when loading video tensor from uri#880

Merged
samsja merged 17 commits into
mainfrom
feat-add-key-frame-indices
Dec 5, 2022
Merged

feat: store key frame indices when loading video tensor from uri#880
samsja merged 17 commits into
mainfrom
feat-add-key-frame-indices

Conversation

@anna-charlotte

@anna-charlotte anna-charlotte commented Nov 30, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Goals:

When calling .load_uri_to_video_tensor we want to additionally store the indices of the key frames in the Document's .tags['key_frame_indices']. This makes it easier to access selected frames, e.g.:

from docarray import Document

d = Document(uri='toy.mp4')
d.load_uri_to_video_tensor()
first_scene = d.tensor[d.tags['key_frame_indices'][0]: d.tags['key_frame_indices'][1]]

print(first_scene.shape)
(95, 320, 176, 3)

The indices will be stored independently of the value of the key_frames parameter.

  • store key frame indices in .tags
  • check and update documentation, if required. See guide

anna-charlotte added 3 commits November 30, 2022 17:01
test: add test video convert pipe with key frame indices
63718e5 
Signed-off-by: anna-charlotte <charlotte.gerhaher@jina.ai>
test: adjust test
536fc1a 
Signed-off-by: anna-charlotte <charlotte.gerhaher@jina.ai>
feat: store key frame indices in tags
a7d34ef 
Signed-off-by: anna-charlotte <charlotte.gerhaher@jina.ai>
@JoanFM JoanFM changed the title Store key frame indices when loading video tensor from uri feat: store key frame indices when loading video tensor from uri Nov 30, 2022
@anna-charlotte anna-charlotte linked an issue Nov 30, 2022 that may be closed by this pull request
Provide functionality to get frame indices for keyframes #878
Closed
@codecov-commenter

codecov-commenter commented Nov 30, 2022 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Base: 88.13% // Head: 88.14% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (218f838) compared to base (59db0ff).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main     #880   +/-   ##
=======================================
  Coverage   88.13%   88.14%           
=======================================
  Files         138      138           
  Lines        7137     7142    +5     
=======================================
+ Hits         6290     6295    +5     
  Misses        847      847
Flag Coverage Δ
docarray 88.14% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
docarray/document/mixins/video.py 57.57% <100.00%> (+3.47%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

samsja
samsja reviewed Dec 1, 2022
View reviewed changes
Comment thread docarray/document/mixins/video.py Outdated
samsja
samsja reviewed Dec 1, 2022
View reviewed changes
Comment thread docarray/document/mixins/video.py Outdated
anna-charlotte added 6 commits December 1, 2022 09:59
fix: only store indices in only keyframes is false
32ed587 
Signed-off-by: anna-charlotte <charlotte.gerhaher@jina.ai>
fix: us av is keyframe function and adjust tests
d14a47d 
Signed-off-by: anna-charlotte <charlotte.gerhaher@jina.ai>
docs: adjust documentation
9ce8d40 
Signed-off-by: anna-charlotte <charlotte.gerhaher@jina.ai>
refactor: keyframe instead of key frames
3cc56ee 
Signed-off-by: anna-charlotte <charlotte.gerhaher@jina.ai>
feat: use frame key frame from av
afee7d8 
Signed-off-by: anna-charlotte <charlotte.gerhaher@jina.ai>
Merge branch 'main' into feat-add-key-frame-indices
0a69ed4
@anna-charlotte anna-charlotte marked this pull request as ready for review December 1, 2022 12:12
JohannesMessner
JohannesMessner reviewed Dec 1, 2022
View reviewed changes
Comment thread docarray/document/mixins/video.py Outdated
Comment thread docs/datatypes/video/index.md Outdated
docs: apply suggestions from review
79e20d2 
Signed-off-by: anna-charlotte <charlotte.gerhaher@jina.ai>
samsja
samsja approved these changes Dec 1, 2022
View reviewed changes

@samsja samsja left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@JohannesMessner
docs: apply suggestions from review
60688de 
Co-authored-by: Johannes Messner <44071807+JohannesMessner@users.noreply.github.com>
Signed-off-by: anna-charlotte <charlotte.gerhaher@jina.ai>
alexcg1
alexcg1 suggested changes Dec 1, 2022
View reviewed changes
Comment thread docarray/document/mixins/video.py Outdated
Comment thread docs/datatypes/video/index.md Outdated
anna-charlotte and others added 2 commits December 1, 2022 15:00
@alexcg1
docs: apply changes
c2f89e2 
Co-authored-by: Alex Cureton-Griffiths <alexcg1@users.noreply.github.com>
Signed-off-by: anna-charlotte <charlotte.gerhaher@jina.ai>
@alexcg1
docs: apply changes
218f838 
Co-authored-by: Alex Cureton-Griffiths <alexcg1@users.noreply.github.com>
Signed-off-by: anna-charlotte <charlotte.gerhaher@jina.ai>
@github-actions

github-actions Bot commented Dec 1, 2022

Copy link
Copy Markdown

📝 Docs are deployed on https://ft-feat-add-key-frame-indices--jina-docs.netlify.app 🎉

alexcg1
alexcg1 approved these changes Dec 1, 2022
View reviewed changes

@alexcg1 alexcg1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM 👍

@samsja samsja merged commit e087dfa into main Dec 5, 2022
@samsja samsja deleted the feat-add-key-frame-indices branch December 5, 2022 06:48
@alexcg1 alexcg1 mentioned this pull request Dec 6, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JohannesMessner JohannesMessner JohannesMessner left review comments

@JoanFM JoanFM JoanFM approved these changes

@samsja samsja samsja approved these changes

+1 more reviewer

@alexcg1 alexcg1 alexcg1 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area/core area/docs area/testing component/document size/s

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Provide functionality to get frame indices for keyframes

6 participants

@anna-charlotte @codecov-commenter @alexcg1 @JoanFM @JohannesMessner @samsja