| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
tested against the released client library 1.1.0. |
Sorry, something went wrong.
| emotions = frame.attributes[0].emotions | ||
|
|
||
| # from videointelligence.enums | ||
| emotion_labels = ( |
There was a problem hiding this comment.
Is there any way to use the videointelligence.enums object directly or pull this programmatically? How badly will this break if the enum changes in the future?
Sorry, something went wrong.
There was a problem hiding this comment.
The simplest way is for the enums in videointelligence.enums to extend python's Enums class, which would allow us to map the enums back to their names. However the client libraries do not do that (yet).
I don't know of a clean way to pull the names programmatically, and as it stands now the sample code will break in these possible ways:
the order of the enums changes (unlikely), in which case the printout of the sample will be incorrect.
more enums are added (possible) and the data we use for testing receive those added enums in the API responses, in which case we would get an IndexError in our testing.
(worst) more enums are added, but the testing data does not receive them, so we don't notice it is broken. Users relying on the same tuple to map enums to names might get IndexError with their data.
The best long term solution is to simply extend Enums. When that happens we will need to come back and update these samples.
Sorry, something went wrong.
There was a problem hiding this comment.
+1 Programmatically, this keeps showing up in Python samples and, as a user, it seems really frustrating that Python makes you do this... you get the proto's enum value index for enums via the library but not the enum value name?
I don't think any of the other languages have this problem?
(Make this better! 😭)
Sorry, something went wrong.
There was a problem hiding this comment.
Without fixing the client library generation tool, this is the closest I could come up with:
from google.cloud import videointelligence_v1p1beta1 as videointelligence
emotion_label = {value:name for name, value in vars(videointelligence.enums.Emotion).iteritems() if not name.startswith('__')}
I would rather not have this as is in a code snippet - but perhaps abstracted away as a helper function? The reader of the sample would not be able to see the full list of enums this way.
Sorry, something went wrong.
There was a problem hiding this comment.
To be clear, I wasn't thinking about fixing this in the code sample, but rather in google-cloud-core
IMO The code sample should use an inline, literal List with the values. The display name of the enum value should be printed by getting it by index. As we do today.
Sorry, something went wrong.
There was a problem hiding this comment.
I'll move this to a thread on google-cloud-core 😄
Sorry, something went wrong.
There was a problem hiding this comment.
Code in this PR regarding Enum LGTM
Sorry, something went wrong.
There was a problem hiding this comment.
Looks pretty good, mostly stuff for discussion
Sorry, something went wrong.
|
|
||
| Usage Examples: | ||
| python beta_snippets.py boxes \ | ||
| gs://python-docs-samples-tests/video/googlework_short.mp4 |
There was a problem hiding this comment.
Should we make another demo project to hold public files that can be used across languages?
Sorry, something went wrong.
| # limitations under the License. | ||
|
|
||
| """This application demonstrates face detection, label detection, | ||
| explicit content, and shot change detection using the Google Cloud API. |
There was a problem hiding this comment.
... face detection, face emotions, video transcription
Sorry, something went wrong.
There was a problem hiding this comment.
done.
Sorry, something went wrong.
| # include_bounding_boxes must be set to True for include_emotions | ||
| # to work. | ||
| config = videointelligence.types.FaceConfig( | ||
| include_bounding_boxes=True, |
There was a problem hiding this comment.
Is this still required? I thought we had them pull that?
Sorry, something went wrong.
There was a problem hiding this comment.
done.
Sorry, something went wrong.
|
|
||
|
|
||
| # [START video_face_bounding_boxes] | ||
| def face_bounding_boxes(path): |
There was a problem hiding this comment.
Can they use a local file too or just a GCS file?
Sorry, something went wrong.
There was a problem hiding this comment.
They can. The request needs to be only slightly different, such as https://github.com/GoogleCloudPlatform/python-docs-samples/blob/master/video/cloud-client/analyze/analyze.py#L134
Sorry, something went wrong.
There was a problem hiding this comment.
Do we only need to provide local file snippets, no GCS snippets?
Sorry, something went wrong.
There was a problem hiding this comment.
Here we are showing only GCS snippets.
Sorry, something went wrong.
There was a problem hiding this comment.
OH, do Python samples all use path for GCS? I figured that was for a local file
It should be uri, based on existing samples, eg: https://cloud.google.com/vision/docs/detecting-labels#vision-label-detection-python
def detect_labels(path):
"""Detects labels in the file."""
client = vision.ImageAnnotatorClient()def detect_labels_uri(uri):
"""Detects labels in the file located in Google Cloud Storage or on the
Web."""
client = vision.ImageAnnotatorClient()
Sorry, something went wrong.
There was a problem hiding this comment.
Let's make this change across the board to all 3 samples
Sorry, something went wrong.
There was a problem hiding this comment.
you are right - it should be uri, or rather gcs_uri (since currently the API only supports that). fixed.
Sorry, something went wrong.
| print('\tSegment: {}\n'.format(positions)) | ||
|
|
||
| # There are typically many frames for each face, | ||
| # here we print information on only the first frame. |
There was a problem hiding this comment.
This comment confused me a little, especially with the possibility of segments above.
"There are typically many frames for each face, since a face is in a segment of time" (I don't know if that helps clarify it or just makes it more confusing)
Could a face appear in multiple segments?
Like:
Face 1: (Segment 0:00 --> 0:05, 0:15 --> 0:25) then the face would be in all the frames between those segments.
Sorry, something went wrong.
There was a problem hiding this comment.
I slightly reworded this comment, please take a look.
For face detection, each face indeed is a segment. If the same person appears in two segments of the same video, two separate faces will be returned, each with its own segment.
Sorry, something went wrong.
| for time_offset, emotion_label, score in frame_emotions: | ||
| print('\t{:04.2f}s: {:14}({:4.3f})'.format( | ||
| time_offset, emotion_label, score)) | ||
| print('\n') |
There was a problem hiding this comment.
Should we merely print out the results without sorting based on score? Will help simplify the code, if we were going across the whole segment to determine the most likely emotion across all frames I think that would be cool, but not as simple.
Also by printing out the time_offset, isn't it always the same, since we only get the one frame for each face?
Sorry, something went wrong.
There was a problem hiding this comment.
Here actually we are sorting by time_offset - an earlier version of of the API did not sort the frames by it. Emotion is a frame level output since detecting the change of emotions is useful.
The output might look like this:
30.56s: INTEREST (0.606) 30.60s: INTEREST (0.582) 30.63s: INTEREST (0.561) 30.66s: AMUSEMENT (0.559) 30.70s: AMUSEMENT (0.556) 30.73s: AMUSEMENT (0.549) 30.76s: CONTENTMENT (0.564) 30.80s: CONTENTMENT (0.634) 30.83s: CONTENTMENT (0.688)
Sorry, something went wrong.
|
|
||
| features = [ | ||
| videointelligence.enums.Feature.SPEECH_TRANSCRIPTION | ||
| ] |
There was a problem hiding this comment.
One line?
Sorry, something went wrong.
There was a problem hiding this comment.
done.
Sorry, something went wrong.
|
|
||
| result = operation.result(timeout=180) | ||
|
|
||
| annotation_results = result.annotation_results[0] |
There was a problem hiding this comment.
Can you add a comment that notes you are only pulling out the first result or is there only one result?
Sorry, something went wrong.
There was a problem hiding this comment.
done.
Sorry, something went wrong.
There was a problem hiding this comment.
Looks great (1 tiny fix)
Sorry, something went wrong.
| positions = '{}s to {}s'.format(start_time, end_time) | ||
| print('\tSegment: {}\n'.format(positions)) | ||
|
|
||
| # Each detected may appear in many frames of the video. |
There was a problem hiding this comment.
Each detected face
(I like the rewording)
Sorry, something went wrong.
|
|
||
| emotion, score = sorted( | ||
| [(em.emotion, em.score) for em in emotions], | ||
| key=lambda p: p[1])[-1] |
There was a problem hiding this comment.
Non-trivial logic alert!
Sorry, something went wrong.
There was a problem hiding this comment.
@nnegrey I think I misunderstood your comment about this part - indeed I was sorting by emotion scores. The reason being that the API returns one score for each emotion, and here I am trying to show only the one that scores the highest.
Sorry, something went wrong.
There was a problem hiding this comment.
added a comment here to clarify what I was doing. please take a look.
Sorry, something went wrong.
There was a problem hiding this comment.
This line is really impressive:
Sorry, something went wrong.
|
|
||
| # every emotion gets a score, here we sort them by | ||
| # scores and keep only the one that scores the highest. | ||
| most_likely_emotion = sorted(emotions, key=lambda em: em.score)[-1] |
There was a problem hiding this comment.
Still somewhat impressive!
Sorry, something went wrong.
There was a problem hiding this comment.
Simplified. Thanks for taking the time explaining the issues to me!
Sorry, something went wrong.
|
|
||
|
|
||
| # [START video_speech_transcription] | ||
| def speech_transcription(input_uri): |
There was a problem hiding this comment.
The other 2 functions use gcs_uri but this uses input_uri
We should make them consistent (and I'd TAL at whatever variable we use in all of the other Vision samples and use that for consistency)
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
This PR adds samples for the following beta features: