The spec is at least clear that currentTime should be correct while paused. In your testing, does the current stack reliably return correct values for everything else (videoWidth/videoHeight/texture contents) when 'paused' is emitted?

If not those would be tractable more immediately than getting traction on a bigger spec.

(While we're talking specs though, I would propose something more similar to WebAudio where you redirect the whole output stream. This would make it far more likely we can implement it correctly on Android.)

The video team and I discussed this offline and the first thing to do would be to prototype a few properties on HTMLVideoElement, behind a command-line flag, which would provide them the data they absolutely need: frame time, video width, and video height for the last frame uploaded to WebGL via tex{Sub}Image2D.

If these work well then we can see about the best way to spec them. Implementing the WEBGL_dynamic_texture extension may well be the best way to go, but it'll be a lot more work than simply providing the information about the last uploaded frame.

#3: What you say makes sense, I just want to clarify my point about Android above. Currently the texImage2D() step can fail if we were optimizing for power before the call. Once you've managed to texImage2D() there are no platform-specific concerns.

Currently this only affects fullscreen <video> elements, but we expect all cases to be affected eventually (that is, once we have inline SurfaceView support).

Talked with Kai about this and he will probably prototype and give it to the YouTube/media teams to test.

Let's do this experiment for Q1 this year. It shouldn't take long to implement.

Note for future self:
* Snapshot the video width, height, frame time (frame number?) upon upload
* Expose it via new properties of HTMLVideoElement (hidden behind enable-experimental-canvas-features)

Relabeling as M-58 as I have other time sensitive work this week before the M57 branch.

Taking this back since I have time now.

Awesome! This is definitely something we're still interested in :)

I've started a patch. Can you take a look at the IDL and tell me if it's what you're looking for? (What data is exposed and the types of that data)

https://codereview.chromium.org/2749653003/diff/20001/third_party/WebKit/Source/core/html/HTMLVideoElement.idl

Looks good, assuming the updated values are available immediately after texImage2D returns.

Ditto. Thanks!

anjalibh, scherkus: Thanks for the confirmation. Still iterating on the implementation (fields will probably move to the WebGLTexture object) but yes, they should be available immediately.

An I2I is filed here:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/YLR3v0t8WF0

And the patch should be complete, I'm in the process of landing it.
https://codereview.chromium.org/2749653003

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/36eeff88e474d2e562f2d656bb94d0e37770e245

commit 36eeff88e474d2e562f2d656bb94d0e37770e245
Author: kainino <kainino@chromium.org>
Date: Thu Mar 30 00:55:30 2017

Prototype HTMLVideoElement properties for WebGL texImage2D

These properties provide the width, height, and time of the last video
frame uploaded to a WebGL texture via texImage2D.

BUG=639174
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2749653003
Cr-Commit-Position: refs/heads/master@{#460606}

[modify] https://crrev.com/36eeff88e474d2e562f2d656bb94d0e37770e245/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/36eeff88e474d2e562f2d656bb94d0e37770e245/media/blink/webmediaplayer_impl.h
[modify] https://crrev.com/36eeff88e474d2e562f2d656bb94d0e37770e245/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/36eeff88e474d2e562f2d656bb94d0e37770e245/third_party/WebKit/LayoutTests/VirtualTestSuites
[add] https://crrev.com/36eeff88e474d2e562f2d656bb94d0e37770e245/third_party/WebKit/LayoutTests/fast/canvas-experimental/webgl/texImage-video-last-uploaded-metadata-expected.txt
[add] https://crrev.com/36eeff88e474d2e562f2d656bb94d0e37770e245/third_party/WebKit/LayoutTests/fast/canvas-experimental/webgl/texImage-video-last-uploaded-metadata.html
[add] https://crrev.com/36eeff88e474d2e562f2d656bb94d0e37770e245/third_party/WebKit/LayoutTests/virtual/experimental-canvas-features/fast/canvas-experimental/README.txt
[modify] https://crrev.com/36eeff88e474d2e562f2d656bb94d0e37770e245/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/36eeff88e474d2e562f2d656bb94d0e37770e245/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
[modify] https://crrev.com/36eeff88e474d2e562f2d656bb94d0e37770e245/third_party/WebKit/Source/modules/webgl/WebGLTexture.cpp
[modify] https://crrev.com/36eeff88e474d2e562f2d656bb94d0e37770e245/third_party/WebKit/Source/modules/webgl/WebGLTexture.h
[modify] https://crrev.com/36eeff88e474d2e562f2d656bb94d0e37770e245/third_party/WebKit/Source/modules/webgl/WebGLTexture.idl
[modify] https://crrev.com/36eeff88e474d2e562f2d656bb94d0e37770e245/third_party/WebKit/public/platform/WebMediaPlayer.h

scherkus: This change should be in Canary now. Please let me know if any updates are needed.

For reference, you need to enable chrome://flags/#enable-experimental-canvas-features , and the interface is on WebGLTexture:

interface WebGLTexture {
    [RuntimeEnabled=ExperimentalCanvasFeatures] readonly attribute unsigned long lastUploadedVideoWidth;
    [RuntimeEnabled=ExperimentalCanvasFeatures] readonly attribute unsigned long lastUploadedVideoHeight;
    [RuntimeEnabled=ExperimentalCanvasFeatures] readonly attribute double lastUploadedVideoTimestamp;
};

Removing M-59 and reducing to P2 since first iteration is done.

Kai: after internal discussion, could you also please add:

    [RuntimeEnabled=ExperimentalCanvasFeatures] readonly attribute int lastUploadedVideoFrameId;

This would correspond to VideoFrame::unique_id() (src/media/base/video_frame.h) and is a better identifier in Chrome for testing whether the video frame has changed.

Thanks!

One more thing: after adding that property, could you also please add code to the video-to-WebGL-texture upload paths to short-circuit the upload if the property exists, and it's the same as the unique ID of the current video frame. (Only doing the short-circuiting if --enable-experimental-canvas-features is enabled.)

This may provide significant framerate increases for apps which are uploading large videos that have a relatively low frame rate (e.g. 30 FPS as opposed to 60 FPS for the display).

Thanks!

This ID was never intended to be exposed to the web platform. I don't see any immediate security concerns (it's global, but not used for security-related decisions), but I'm not excited about locking in this implementation detail.

Notably, the current implementation is less than a year old; it's not what I would consider a stable part of the VideoFrame API.

I would suggest instead that the upload path can use this field, but should do so to decide to increment its own counter which is exposed (or, really, to set just a bool, which seems to be what's needed).

Can you share more about the internal discussion that led to this request?

The discussion involved a partner doing 360 degree video display via WebGL and needing to skip redundant texture uploads for performance reasons. There are really only a couple of ways to do so -- either have the app query the HTMLVideoElement for some currently-nonexistent metadata to know whether to skip the upload, or have the implementation skip the upload itself. (SkCanvasVideoRenderer already skips redundant video frame uploads, but the video-to-WebGL upload path doesn't.)

dalecurtis@ had the good idea to use the experimental metadata being exposed from this bug to know when to skip video frame uploads to WebGL internally. We could plausibly spec this fairly quickly at some point.

It's a fair point that we could expose this to the web platform just as a bool like "lastUploadedVideoFrameWasSkipped". Would that be better from your standpoint?

See chrome-facebook@ mailing list for the discussion. I don't think we necessarily need to expose this id to js, but it should definitely be used internally for avoiding unnecessary work.

For an experimental API exposing it seems fine; if we're really concerned about the counter leaking any information we can just generate an unguessable token per frame or a hash of one token plus the counter value. Though I think that's not necessary, we can defer to security team if this does end up needing to be exposed in the final API.

I've been following this discussion, and if I understand it correctly, it sounds like a potential solution to some 360 video issues we've encountered too. Namely, the ability to know exactly when a given frame has been (or will be) displayed. In lieu of the ability to tag a frame, we're left guessing, and we often miss by a frame or two. Unfortunately, missing by even a single frame won't work. I've been following this work hoping that it's exposed for use in our player.

FYI, it will be at least 2.5 weeks before I'm able to work on this. Marking as available for now in case anyone else can take it.

Thanks for telling us Kai. Let me take this temporarily and see if I can add this quickly.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9e8ae29ba31af922e26d8af9ea768ca8a99014ce

commit 9e8ae29ba31af922e26d8af9ea768ca8a99014ce
Author: Kai Ninomiya <kainino@chromium.org>
Date: Wed Sep 06 23:45:16 2017

Video texture metadata prototype: add lastUploadedVideoFrameWasSkipped

This requires some fairly intrusive changes. Took advantage of this to
make the previous hack (https://crrev.com/2749653003) less hacky too.

Bug: 639174
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I9cbdae3dbadbe8788c696bba0d89bfa69692ce1c
Reviewed-on: https://chromium-review.googlesource.com/616188
Reviewed-by: Dimitri Glazkov <dglazkov@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Commit-Queue: Kai Ninomiya <kainino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500126}
[modify] https://crrev.com/9e8ae29ba31af922e26d8af9ea768ca8a99014ce/content/renderer/media/webmediaplayer_ms.cc
[modify] https://crrev.com/9e8ae29ba31af922e26d8af9ea768ca8a99014ce/content/renderer/media/webmediaplayer_ms.h
[modify] https://crrev.com/9e8ae29ba31af922e26d8af9ea768ca8a99014ce/content/renderer/media_capture_from_element/html_video_element_capturer_source_unittest.cc
[modify] https://crrev.com/9e8ae29ba31af922e26d8af9ea768ca8a99014ce/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/9e8ae29ba31af922e26d8af9ea768ca8a99014ce/media/blink/webmediaplayer_impl.h
[modify] https://crrev.com/9e8ae29ba31af922e26d8af9ea768ca8a99014ce/third_party/WebKit/LayoutTests/fast/canvas/webgl/texImage-video-last-uploaded-metadata.html
[modify] https://crrev.com/9e8ae29ba31af922e26d8af9ea768ca8a99014ce/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/9e8ae29ba31af922e26d8af9ea768ca8a99014ce/third_party/WebKit/Source/core/html/HTMLVideoElement.cpp
[modify] https://crrev.com/9e8ae29ba31af922e26d8af9ea768ca8a99014ce/third_party/WebKit/Source/core/html/HTMLVideoElement.h
[modify] https://crrev.com/9e8ae29ba31af922e26d8af9ea768ca8a99014ce/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
[modify] https://crrev.com/9e8ae29ba31af922e26d8af9ea768ca8a99014ce/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.h
[modify] https://crrev.com/9e8ae29ba31af922e26d8af9ea768ca8a99014ce/third_party/WebKit/Source/modules/webgl/WebGLTexture.cpp
[modify] https://crrev.com/9e8ae29ba31af922e26d8af9ea768ca8a99014ce/third_party/WebKit/Source/modules/webgl/WebGLTexture.h
[modify] https://crrev.com/9e8ae29ba31af922e26d8af9ea768ca8a99014ce/third_party/WebKit/Source/modules/webgl/WebGLTexture.idl
[modify] https://crrev.com/9e8ae29ba31af922e26d8af9ea768ca8a99014ce/third_party/WebKit/Source/platform/testing/EmptyWebMediaPlayer.h
[modify] https://crrev.com/9e8ae29ba31af922e26d8af9ea768ca8a99014ce/third_party/WebKit/public/platform/WebMediaPlayer.h

anjalibh, scherkus: This should be out in Canary by now, please try it out and let us know if it does what you need.

Thanks! The person doing our web experiments is OOO this week but I'll let him know about change when he's back

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ef8c9e03e9812e220bd0c627507e1e41be129afa

commit ef8c9e03e9812e220bd0c627507e1e41be129afa
Author: Kai Ninomiya <kainino@chromium.org>
Date: Wed Sep 27 04:07:40 2017

Fix null-deref in video element metadata prototype

Bug:  767090 , 639174
Change-Id: I785d081990f2127d10a1dbf8cdbd9494af0b437e
Reviewed-on: https://chromium-review.googlesource.com/677088
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Commit-Queue: Kai Ninomiya <kainino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504576}
[modify] https://crrev.com/ef8c9e03e9812e220bd0c627507e1e41be129afa/media/blink/webmediaplayer_impl.cc

Reassigning back to myself. We're planning on turning this same prototype into a more shippable API form with a WebGL extension.

kainino: Are you referring to issue 776222?

Yes, I'm pretty sure that's the one.

For my own notes:

To enable, use --enable-blink-features=ExtraWebGLVideoTextureMetadata

The way lastUploadedVideoFrameWasSkipped works, IIRC, is that if a video frame is the same as the last frame you tried to upload via texImage2D, then we will automatically skip the upload. You can check whether it was skipped by looking at lastUploadedVideoFrameWasSkipped - then avoid rerendering the frame altogether.

Thanks for pointing me over here Kai from  issue 867368 .

The flag seems to work well on OS X with file-based media, looks to report skipping and correctly internally short-circuit the upload. A stream from getUserMedia doesn't work however - the properties are set to 0 for width/height/timestamp and false for lastUploadedVideoFrameWasSkipped.

Test here: https://tango-bravo.net/WebGLVideoPerformance/perftest_skipcount.html

My use-case is performing real-time processing on a video stream. The internal short-circuiting of texUpload is definitely a good win, and presumably could be enabled in general as it's just an internal optimization and doesn't impact the API at all.

The ability to skip processing in the case the frame has not been updated is something that would need new API. The proposed lastUploadedVideoFrameWasSkipped is a sufficient solution for this as far as I can tell.

That said, for camera processing a nicer API would be to just have a function called the next time the video frame changes. Something like requestNextVideoFrame(), rather akin to requestAnimationFrame(), but it would be called at the video frame rate rather than the render rate. Until WebGL is available in WebWorkers it wouldn't be possible to fully decouple WebGL image processing from rendering the output frame.

It's not very clear to me if the grabFrame() API from ImageCapture [ https://www.w3.org/TR/image-capture/#dom-imagecapture-grabframe ] waits for the next frame before resolving the promise; but if so I suppose that's the kind of thing I'm talking about. In this case though it wouldn't actually give you the image data, it's more just to signal that doing a texUpload2D would result in a new frame being uploaded.

Hi  kainino@
After some fiddling around, it seems like the feature does not work perfectly well with MSE.

The texture still returns the properties properly, with lastUploadedVideoFrameWasSkipped alternating between true & false. 
However, the actual texture alternates between the video texture & pure black. 

I'll comeback with demo later if you don't know the cause off the top of your head.. MSE + WebGL makes it tricky to write a short standalone demo. 

I'm not too surprised that it behaves oddly - I only developed and tested
against one media code path.

Sorry, what is MSE?

Sorry, my bad.
Media Source Extension.

MSE and SRC= paths shouldn't be different as seen by WebGL, so there's something else strange going on here.

Interestingly the problem shows up on one of my computers & not the other, both running windows 10. 
Let me try to dig into it a little more & see what I can find. 

FYI, some fairly invasive wiring had to be done to support this. So I think
(IIUC) that the MSE and SRC paths could behave differently here.

It wasn't invasive past WMPI, at which point MSE and SRC= look the same. I think it's more likely this is the feature UseSurfaceLayerForVideo causing a breakage.

Please test with --disable-features=UseSurfaceLayerForVideo to see if your problem goes away.

Ah, I see. That seems like a good hypothesis.

FYI, WebMediaPlayerMS::Paint ignores the already_uploaded_id and out_metadata.
https://cs.chromium.org/chromium/src/content/renderer/media/stream/webmediaplayer_ms.cc?l=767&rcl=1a48ecfaf1a8fc990ed5c525d2b52753e2ea5054
This was intentional, as I had no need for it when I was prototyping. However it does mean that this API doesn't do anything for media streams.

Re: #46, when working on  issue 833902 , there was some talk around also adding an AnimationFrameProvider for video elements. However I don't know what the status of that is, or if there are other, better ways to handle it (maybe grabFrame works for the camera case, but I don't think we have anything for video files).

Hi
Sry for been in-responsive for a few days.. I got distracted
I tried to test this on all the computers I could get my hand on to see if I see a pattern that determines whether the black flash shows up or not.
(and nope, --disable-features=UseSurfaceLayerForVideo does not fix it)

So far I have tested
Linux w/GTX 960, not reproducible

Linux w/HD520, not reproducible

Windows laptop w/HD520, not reproducible

Macbook bootcamped in Windows w/intel 5100, reproducible
The same laptop, in MacOS, not reproducible

Windows w/GTX770, reproducible. 
Funny thing with this one, I upgraded from GTX770 to 1080Ti a few days ago on this desktop, and the problem vanished. 

Another thing I noticed was that the problem is not actually MSE (sry about it)
It's actually specific to VP9 decoding.
I was able to reproduce it on a normal video element w/ VP9 files, it just happened that when I initially did the test, all MSE sources were also playing VP9/VP8...
Once I swap the VP9 file out for a mp4 one, the issue vanishes. 

I don't know enough about the VP9 decoding pipeline to tell where the problem lies.. It's probably not a hw vs sw decoder thing, because it was not reproducible on one of my Windows laptops, which was doing software decoding.

The easiest way to check if a particular machine is affected is to open any video in youtube.com/360 with the Chrome flag enabled.. I can try to come up with a more standalone test if so desired.

Thanks!

Could that issue be related to  issue 819914 ? That one is a VP8 video file.

potentially, but I'm not sure how that bug will interact with this work, and why corrupted frames would cause black textures when we do the optimization in this work.

Let me try it again on Canary in a few days & see if that helps.