| Message ID | 1524463833-8025-1-git-send-email-kjeyapal@akamai.com |
|---|---|
| State | Accepted |
| Commit | 5b6cc3a73acb90df0d4fe5ca8f51527b64592bf8 |
| Headers | show |
Hi, On Mon, Apr 23, 2018 at 2:10 AM, Karthick J <kjeyapal@akamai.com> wrote: > - int level = par->level == FF_LEVEL_UNKNOWN ? 0 : par->level; > + int level = par->level == FF_LEVEL_UNKNOWN ? > + get_vp9_level(par, frame_rate) : par->level; After this change, how will I create a file without a level? This patch blurs the line between "unknown", "unspecified", "non-existent" and "autodetect". Linguistically, each of these mean something fundamentally different. I think it's acceptable to not have too many ways of specifying something which in practice comes down to "nope", but you're removing the "nope" altogether, which isn't quite right either. Ronald
On 4/23/2018 8:58 AM, Ronald S. Bultje wrote: > Hi, > > On Mon, Apr 23, 2018 at 2:10 AM, Karthick J <kjeyapal@akamai.com> wrote: > >> - int level = par->level == FF_LEVEL_UNKNOWN ? 0 : par->level; >> + int level = par->level == FF_LEVEL_UNKNOWN ? >> + get_vp9_level(par, frame_rate) : par->level; > > > > After this change, how will I create a file without a level? > > This patch blurs the line between "unknown", "unspecified", "non-existent" > and "autodetect". Linguistically, each of these mean something > fundamentally different. I think it's acceptable to not have too many ways > of specifying something which in practice comes down to "nope", but you're > removing the "nope" altogether, which isn't quite right either. The spec (VP9 in mp4) doesn't allow 0 nor defines an "unknown" value to be muxed into the vpcc atom. Technically speaking, we have been creating invalid files all this time by using 0 when the profile is unknown. See https://www.webmproject.org/vp9/mp4/ I don't think anything really looks at this field, beyond maybe just making sure it's a valid value if the parser is super pedantic, as it's not needed to actually start decoding. So an approximation is probably better than an out of range value like 0.
>From: "Ronald S. Bultje" <rsbultje@gmail.com> >Hi, > >On Mon, Apr 23, 2018 at 2:10 AM, Karthick J <mailto:kjeyapal@akamai.com> wrote: >>- int level = par->level == FF_LEVEL_UNKNOWN ? 0 : par->level; >>+ int level = par->level == FF_LEVEL_UNKNOWN ? >+ get_vp9_level(par, frame_rate) : par->level; > > >After this change, how will I create a file without a level? For best player side behavior, you shouldn't create a file without a level. Theoretically level is meant to signal the player, a maximum decoder complexity required to play the encoded content. Ideally the player should compare the stream's level with its maximum VP9 level supported by the device to decide if the device is capable of playing the current VP9 stream. If we set this to 0, based on the player implementation it will either take it as invalid or will assume the stream has a level lower than the device's maximum supported level. Hence setting the level more accurately is always better than setting it as 0, for best player side experience. Also, as James mentioned 0 is not mentioned in the spec as "unknown" or "unspecified". > >This patch blurs the line between "unknown", "unspecified", "non-existent" and "autodetect". Linguistically, each of these mean something fundamentally different. I think it's acceptable to not have too many ways of specifying something which in practice comes down to >"nope", but you're removing the "nope" altogether, which isn't quite right either. > >Ronald
diff --git a/libavformat/vpcc.c b/libavformat/vpcc.c index 66d0df6..7951448 100644 --- a/libavformat/vpcc.c +++ b/libavformat/vpcc.c @@ -67,11 +67,58 @@ static int get_vpx_video_full_range_flag(enum AVColorRange color_range) return color_range == AVCOL_RANGE_JPEG; } +// Find approximate VP9 level based on the Luma's Sample rate and Picture size. +static int get_vp9_level(AVCodecParameters *par, AVRational *frame_rate) { + int picture_size = par->width * par->height; + int64_t sample_rate; + + // All decisions will be based on picture_size, if frame rate is missing/invalid + if (!frame_rate || !frame_rate->den) + sample_rate = 0; + else + sample_rate = ((int64_t)picture_size * frame_rate->num) / frame_rate->den; + + if (picture_size <= 0) { + return 0; + } else if (sample_rate <= 829440 && picture_size <= 36864) { + return 0x10; + } else if (sample_rate <= 2764800 && picture_size <= 73728) { + return 0x11; + } else if (sample_rate <= 4608000 && picture_size <= 122880) { + return 0x20; + } else if (sample_rate <= 9216000 && picture_size <= 245760) { + return 0x21; + } else if (sample_rate <= 20736000 && picture_size <= 552960) { + return 0x30; + } else if (sample_rate <= 36864000 && picture_size <= 983040) { + return 0x31; + } else if (sample_rate <= 83558400 && picture_size <= 2228224) { + return 0x40; + } else if (sample_rate <= 160432128 && picture_size <= 2228224) { + return 0x41; + } else if (sample_rate <= 311951360 && picture_size <= 8912896) { + return 0x50; + } else if (sample_rate <= 588251136 && picture_size <= 8912896) { + return 0x51; + } else if (sample_rate <= 1176502272 && picture_size <= 8912896) { + return 0x52; + } else if (sample_rate <= 1176502272 && picture_size <= 35651584) { + return 0x60; + } else if (sample_rate <= 2353004544 && picture_size <= 35651584) { + return 0x61; + } else if (sample_rate <= 4706009088 && picture_size <= 35651584) { + return 0x62; + } else { + return 0; + } +} + int ff_isom_get_vpcc_features(AVFormatContext *s, AVCodecParameters *par, - VPCC *vpcc) + AVRational *frame_rate, VPCC *vpcc) { int profile = par->profile; - int level = par->level == FF_LEVEL_UNKNOWN ? 0 : par->level; + int level = par->level == FF_LEVEL_UNKNOWN ? + get_vp9_level(par, frame_rate) : par->level; int bit_depth = get_bit_depth(s, par->format); int vpx_chroma_subsampling = get_vpx_chroma_subsampling(s, par->format, par->chroma_location); @@ -105,7 +152,7 @@ int ff_isom_write_vpcc(AVFormatContext *s, AVIOContext *pb, VPCC vpcc; int ret; - ret = ff_isom_get_vpcc_features(s, par, &vpcc); + ret = ff_isom_get_vpcc_features(s, par, NULL, &vpcc); if (ret < 0) return ret; diff --git a/libavformat/vpcc.h b/libavformat/vpcc.h index d71ba05..e87bec5 100644 --- a/libavformat/vpcc.h +++ b/libavformat/vpcc.h @@ -53,6 +53,6 @@ int ff_isom_write_vpcc(AVFormatContext *s, AVIOContext *pb, AVCodecParameters *par); int ff_isom_get_vpcc_features(AVFormatContext *s, AVCodecParameters *par, - VPCC *vpcc); + AVRational *frame_rate, VPCC *vpcc); #endif /* AVFORMAT_VPCC_H */
From: Karthick Jeyapal <kjeyapal@akamai.com> --- libavformat/vpcc.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++--- libavformat/vpcc.h | 2 +- 2 files changed, 51 insertions(+), 4 deletions(-)