DISCUSS: Ideal browser behaviour for MP4 mime type canPlayType() query

DISCUSS: Ideal browser behaviour for MP4 mime type canPlayType() query

This is with respect to CL https://codereview.chromium.org/150653008/

Proposal:
audio/mpeg -> any codecs = ""
audio/mp3 -> any codecs = ""

audio/mp4 -> avc1.* = ""
audio/mp4 -> avc3.* = ""
audio/mp4 -> mp4a.* = "maybe"

video/mp4 -> avc1.* = "maybe"
video/mp4 -> avc3.* = "maybe"
video/mp4 -> mp4a.* = "maybe"

BUG=53193

[amogh.bihani, 2014-03-28 12:21:02]

I have created this just for discussion so that I can move ahead on the other
patch mentioned in the description.

I have a confusion on what should be the ideal behaviour for MP4 mime types.

acolwell@ had stated that just "avc1", "avc3", "mp4a" are not RFC compliant, so
I think we should not return "maybe" instead of "probably" for this query.

qinmin@ had stated that the suffixes like ".64001F", ".40.2" are not same across
platforms and hence we can't be sure which extension should be used.
I think in that case we should give "maybe" for such queries as we should give
"probably" only when we are totally sure.

In that case we may end up giving, at best, a "maybe" for mp4 containers.

Please give your feedback.

[amogh.bihani, 2014-03-28 12:22:06]

> acolwell@ had stated that just "avc1", "avc3", "mp4a" are not RFC compliant,
so
> I think we should not return "maybe" instead of "probably" for this query.

Correction:

acolwell@ had stated that just "avc1", "avc3", "mp4a" are not RFC compliant, so
I think we should return "maybe" instead of "probably" for this query.

[ddorwin, 2014-03-28 22:51:33]

On 2014/03/28 12:22:06, amogh.bihani wrote:
> > acolwell@ had stated that just "avc1", "avc3", "mp4a" are not RFC compliant,
> so
> > I think we should not return "maybe" instead of "probably" for this query.
> 
> Correction:
> 
> acolwell@ had stated that just "avc1", "avc3", "mp4a" are not RFC compliant,
so
> I think we should return "maybe" instead of "probably" for this query.

The proposal SGTM. scherkus@ should weigh in too (I made him a reviewer).

I think the following is a more accurate characterization of your change:

Only report support avc1, avc3, and mp4a in an MP4 container.

When proprietary codecs are supported, the results will be as follows:

audio/mpeg = "probably"
audio/mp3 = "probably"
audio/mpeg -> any codec(s) = "" 
audio/mp3 -> any codec(s) = "" 

audio/mp4 -> mp4a.* = "maybe" 
audio/mp4 -> any other codec(s) = "" 

video/mp4 -> any combination of avc1.*, avc3.*, and mp4a.* = "maybe" 
video/mp4 -> any other codec(s) = ""

[scherkus (not reviewing), 2014-03-29 00:48:48]

apologies I hadn't been closely following the other discussion

is this discussion limited to how we handle the lack of profiles for mp4 types?

can you give me an example of what would return "probably"?

[acolwell GONE FROM CHROMIUM, 2014-03-29 01:21:35]

On 2014/03/28 12:22:06, amogh.bihani wrote:
> > acolwell@ had stated that just "avc1", "avc3", "mp4a" are not RFC compliant,
> so
> > I think we should not return "maybe" instead of "probably" for this query.
> 
> Correction:
> 
> acolwell@ had stated that just "avc1", "avc3", "mp4a" are not RFC compliant,
so
> I think we should return "maybe" instead of "probably" for this query.

I believe this needs further clarification.
"avc1" and "avc3" are RFC 6381 compliant and should return "maybe" since there
isn't enough info to know if we can play it or not.
"avc1.", "avc3." and anything that starts with "avc1." or "avc3." and is not
immediately followed by 6 valid hex characters is NOT compliant and I think we
should respond with "".
I also believe we should return "" for anything that starts with an "mp4a."
prefix and does not comply with the RFC BNF. 
In general, I don't think we should reward web applications with a "maybe" if
they didn't form the mimetype correctly.

[amogh.bihani, 2014-03-29 06:38:15]

scherkus@
I have added comments on what should be "probably" but why can't we decisively
say it is "probably"

https://codereview.chromium.org/216893002/diff/1/content/browser/media/media_...
File content/browser/media/media_canplaytype_browsertest.cc (right):

https://codereview.chromium.org/216893002/diff/1/content/browser/media/media_...
content/browser/media/media_canplaytype_browsertest.cc:579:
EXPECT_EQ(kPropMaybe, CanPlay("'video/mp4; codecs=\"avc1.4D401E\"'"));
scherkus@:
this is what should ideally be given "probably" but because we do not know which
platform would support which profile we cannot decisively say that this is
"probably".
And even in various profiles there are many variations like
baseline profile: .42E01E, .4D401E, .4D001E, .42001E
high: .64001E, .640028 etc.

https://codereview.chromium.org/216893002/diff/1/content/browser/media/media_...
content/browser/media/media_canplaytype_browsertest.cc:660:
EXPECT_EQ(kPropMaybe, CanPlay("'video/x-m4v; codecs=\"avc1.4D401E\"'"));
ditto.

https://codereview.chromium.org/216893002/diff/1/content/browser/media/media_...
content/browser/media/media_canplaytype_browsertest.cc:737:
EXPECT_EQ(kPropMaybe, CanPlay("'audio/mp4; codecs=\"mp4a.40.2\"'"));
ditto.
And even in this mp4a.40 is RFC compliant but we support only mp4a.40.2 and
mp4a.40.5. (Please correct me if I am wrong)

https://codereview.chromium.org/216893002/diff/1/content/browser/media/media_...
content/browser/media/media_canplaytype_browsertest.cc:813:
EXPECT_EQ(kPropMaybe, CanPlay("'audio/x-m4a; codecs=\"mp4a.40.2\"'"));
ditto

[amogh.bihani, 2014-04-01 04:12:41]

So should I implement this?

audio/mpeg = "probably"
audio/mp3 = "probably"
audio/mpeg -> any codecs = ""
audio/mp3 -> any codecs = ""

audio/mp4 -> mp4a = "maybe"
audio/mp4 -> mp4a.unknown = ""
audio/mp4 -> mp4a.40 = "maybe"

video/mp4 -> avc1 = "maybe"
video/mp4 -> avc1.unknown = ""
video/mp4 -> avc1.<6_Digit_Hexadecimal> = "maybe"
(similarly for avc3)

[amogh.bihani, 2014-04-21 13:39:28]

For making canPlayType query strict for mp4 containers, I was thinking of using
regular expressions like (avc1.)[0-9A-F]{6}
However, regex library is available in c++11 only. Can I use this library in
chromium?
If not should I make a method to parse such an expression (not with all
functionalities, just the bare minimum, required for mp4 codecs)?
Or should I make a method which is sort of hard coded for mp4 containers, i.e.
for video/mp4 it expects avc1/avc3 followed by 6 digit hexadecimal and so on.

[ddorwin, 2014-04-24 20:24:51]

On 2014/04/21 13:39:28, amogh.bihani wrote:
> For making canPlayType query strict for mp4 containers, I was thinking of
using
> regular expressions like (avc1.)[0-9A-F]{6}
> However, regex library is available in c++11 only. Can I use this library in
> chromium?
> If not should I make a method to parse such an expression (not with all
> functionalities, just the bare minimum, required for mp4 codecs)?
> Or should I make a method which is sort of hard coded for mp4 containers, i.e.
> for video/mp4 it expects avc1/avc3 followed by 6 digit hexadecimal and so on.

I think regex is overkill for this.
If we want to check the validity of the right hand side, we can do something
simpler:
if "avc1"
  return true;
if "avc1." and remainder length is 6:
  for i = 0 to 5:
    if !IsHexDigit(remainder[i]) // defined in base/strings/string_util.h
      return false;
  return true;

https://codereview.chromium.org/216893002/diff/1/content/browser/media/media_...
File content/browser/media/media_canplaytype_browsertest.cc (right):

https://codereview.chromium.org/216893002/diff/1/content/browser/media/media_...
content/browser/media/media_canplaytype_browsertest.cc:416: // audio/mpeg and
audio/mp3 do not allow any codecs parameter
These comments should be removed (or moved), right?

[amogh.bihani, 2014-04-25 08:57:19]

Thanks. I'll make the changes

https://codereview.chromium.org/216893002/diff/1/content/browser/media/media_...
File content/browser/media/media_canplaytype_browsertest.cc (right):

https://codereview.chromium.org/216893002/diff/1/content/browser/media/media_...
content/browser/media/media_canplaytype_browsertest.cc:416: // audio/mpeg and
audio/mp3 do not allow any codecs parameter
On 2014/04/24 20:24:52, ddorwin wrote:
> These comments should be removed (or moved), right?

Oh yes. These have been moved. I am sorry I did not rebase this.

[amogh.bihani, 2014-06-16 09:09:36]

Closing this Discussion thread as it's objectiv is complete.
The corresponding patch merged as r277386.
:)