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

Split up media into video, audio and track #437

Merged
merged 1 commit into from Mar 7, 2017

Conversation

yoavweiss
Copy link
Collaborator

@yoavweiss yoavweiss commented Dec 16, 2016

As discussed on IRC:

  • video, audio and track types are currently a single destination: media
  • This is because we agreed on that as part of expand Request.destination with more granular values #211
  • I agreed to this change, saying "Merging track into media is probably also possible."
  • It is not. (Not in a clean way, anyway)
  • track is quite different from audio/video and is represented internally inside Blink using a different resource type and performs different type checks
  • @annevk prefers that if we split track from media, we'd split them all to 3 different destinations, which will enable audio/video specific Accept headers.
  • destination is also used (and exposed) in SW, but according to @jakearchibald it is not yet shipped, so now is a good time to make this change.

/cc @igrigorik @foolip


Preview | Diff

@igrigorik
Copy link
Member

The fact that media-src affects all of them.. is, fine?

@annevk
Copy link
Member

annevk commented Jan 12, 2017

@igrigorik CSP varies on type per https://w3c.github.io/webappsec-csp/#media-src-pre-request so it shouldn't affect that, but good that we checked.

@annevk
Copy link
Member

annevk commented Jan 12, 2017

This PR seems to have the same problem as the other one. You updated the table, but not the normative places surrounding the values.

Also needs tests (or updated tests if we have some).

@annevk annevk added the needs tests Moving the issue forward requires someone to write tests label Jan 17, 2017
@yoavweiss
Copy link
Collaborator Author

Corresponding test change at web-platform-tests/wpt#5033

@zcorpan
Copy link
Member

zcorpan commented Mar 1, 2017

Why do we want different Accept headers for audio and video? Do browsers do that now?

@yoavweiss
Copy link
Collaborator Author

@zcorpan: I updated the original discussion link. Browsers don't currently do that AFAIK, but splitting destinations would enable them to do that in the future.

@zcorpan
Copy link
Member

zcorpan commented Mar 1, 2017

Why do we want to enable them to do that in the future?

It could make sense to use <audio> for video if one wants to only have the audio part of a video (and maybe show the browser-provided controls). I can't come up with a convincing use case right now though.

It makes perfect sense to use <video> for audio if one wants to use a poster image or show captions.

What effect would it have on caches if <audio> and <video> have different Accept and the same URL is used in both elements?

@yoavweiss
Copy link
Collaborator Author

What effect would it have on caches if and

The intent is not to enable serving video and audio off the same URL, but serving different video formats and different audio formats to supporting browsers. e.g. Accept: video/mp4

I'm not sure how codecs would work with the accept header here, and not sure the use-case is valid unless browsers can also advertise supported codecs.

@annevk
Copy link
Member

annevk commented Mar 1, 2017

  1. We're not changing requirements around the Accept header in this pull request. They are merely enabled.
  2. It seems unlikely we'd never want to differentiate between audio and video in the networking layer. Or is that in fact what you're suggesting?

@zcorpan
Copy link
Member

zcorpan commented Mar 1, 2017

Well... I guess it's hard to argue against 2. But at least I think Accept should be the same and I don't know about any reason to differentiate them in the network.

@yoavweiss
Copy link
Collaborator Author

Chromium CL: https://codereview.chromium.org/2732853003/

@annevk
Copy link
Member

annevk commented Mar 6, 2017

Landing this is blocked on speced/bikeshed#941 and maybe we should also have a clearer commit message to indicate what is being changed.

For one thing, currently Accept uses request's type and not destination, so this change doesn't really impact that one way or another.

@yoavweiss
Copy link
Collaborator Author

Added a commit message

This splits up the `media` destination into three distinct destinations:
`video`, `audio` and `track`.

The reason for that is that track is quite different from video and
audio, and at least in Chromium, treating it internally as
a media resource type is problematic.

This change would allow type specific checks for track.
@yoavweiss
Copy link
Collaborator Author

@zcorpan - Thanks! I'll fix up the Blink CL and land both at the same time (as the Blink CL will land the tests as well)

@annevk annevk removed the needs tests Moving the issue forward requires someone to write tests label Mar 6, 2017
@annevk annevk merged commit ba175cf into whatwg:master Mar 7, 2017
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Mar 9, 2017
@yoavweiss yoavweiss deleted the split_media_destination branch March 9, 2017 14:31
MXEBot pushed a commit to mirror/chromium that referenced this pull request Mar 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants