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 image format changing in WR #943

Closed
JerryShih opened this issue Mar 1, 2017 · 6 comments
Closed

Handle image format changing in WR #943

JerryShih opened this issue Mar 1, 2017 · 6 comments

Comments

@JerryShih
Copy link
Contributor

@JerryShih JerryShih commented Mar 1, 2017

@glennw @kvark @nical @sotaroikeda

In gecko, we can set different video source to a HTMLMediaElement. Then, the compositor will receive various formats of image(rgb or yuv) for composition.
In current WR, we should push different display item for different image format(add_image() for rgb and add_yuv_image() for yuv format in frame_builder.rs). When the format changes, we should push the corresponding display item again and re-build the whole frame data. For the performance purpose, it's good to have a universal display item type which could handle both yuv and rgb format. Then, we could use the similar webrender animation api(#832) or other different api to update the final properties(e.g. the image format and external image id) for that universal video image display item.

@sotaroikeda
Copy link
Contributor

@sotaroikeda sotaroikeda commented Mar 1, 2017

gecko has a special path to render Images via ImageBridge. In this path, updating images happens off main thread ways(without layout change). It is used for video playback, WebRTC and plugin rendering. The following change could happen at any timing.

  • size change
  • format change(RGB vs YUV, YUV color format change"BT601/BT709")
  • video frame clear(No Image)

It would be nice if we could have a place holder of Image and could update Image without new DisplayItem. On current gecko, the change is handled by ImageLayerComposite.

@sotaroikeda
Copy link
Contributor

@sotaroikeda sotaroikeda commented Mar 1, 2017

Even when if there is not format or size change, we need a way to update rendering video images without pushing new DisplayList.

@kvark
Copy link
Member

@kvark kvark commented Mar 1, 2017

When the format changes, we should push the corresponding display item again and re-build the whole frame data. For the performance purpose, it's good to have a universal display item type which could handle both yuv and rgb format.

I don't consider this a performance issue, since I don't assume you change the format/size every frame.

@nical
Copy link
Collaborator

@nical nical commented Mar 1, 2017

Even when if there is not format or size change, we need a way to update rendering video images without pushing new DisplayList.

I am not certain about needing to update the format without changing updating the display list.

If the format changes often within a given video stream, I agree. If it only happens occasionally while switching between decoders or source, I would not say that rebuilding the display list is that much of an issue (as opposed to rebuilding the display list each frame). Remember that we rebuild display list when anything significant changes on the page (hover over something, etc.).

I don't want to dismiss adding a universal image display item in WR, but doing so isn't necessarily easier or cleaner than handing these format switches in gecko. We can still consider both options.

@nical
Copy link
Collaborator

@nical nical commented Mar 6, 2017

It is maybe a silly idea but if we are worried about the cost of rebuilding the display list when a video changes its format, how about isolating video elements in their own pipeline? This would make it very cheap to build and serialize the small display list that only contains the video, although it would not affect the cost of re-building batches when the format changes.

@nical
Copy link
Collaborator

@nical nical commented Aug 9, 2017

We can now change the format of an image in update_image (but not switch between rgb and yuv since yuv is composed of three images). Gecko now uses a pipeline per video.

@nical nical closed this Aug 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.