| Message ID | 20170421232540.13604-1-h.leppkes@gmail.com |
|---|---|
| State | Accepted |
| Commit | 64ad44a3817a288168ba97f6c022160b940cd22e |
| Headers | show |
On Sat, Apr 22, 2017 at 1:25 AM, Hendrik Leppkes <h.leppkes@gmail.com> wrote: > This brings our generation of the vpcC box up to date to version 1.0 > of the VP Codec ISO Media File Format Binding. > > Specifically, color/transfer properties are now written with values > based on ISO/IEC 23001-8, which is the same reference specification the > AVColor* enumerations are based on. > --- > libavformat/movenc.c | 3 ++- > libavformat/vpcc.c | 55 +++++----------------------------------------------- > 2 files changed, 7 insertions(+), 51 deletions(-) > Actually I should probably update the main commit message to mention that its v1.0 and not a draft any longer?
-- KongQun Yang (KQ) On Fri, Apr 21, 2017 at 4:49 PM, Hendrik Leppkes <h.leppkes@gmail.com> wrote: > On Sat, Apr 22, 2017 at 1:25 AM, Hendrik Leppkes <h.leppkes@gmail.com> > wrote: > > This brings our generation of the vpcC box up to date to version 1.0 > > of the VP Codec ISO Media File Format Binding. > > > > Specifically, color/transfer properties are now written with values > > based on ISO/IEC 23001-8, which is the same reference specification the > > AVColor* enumerations are based on. > > --- > > libavformat/movenc.c | 3 ++- > > libavformat/vpcc.c | 55 +++++------------------------- > ---------------------- > > 2 files changed, 7 insertions(+), 51 deletions(-) > > > > Actually I should probably update the main commit message to mention > that its v1.0 and not a draft any longer? > SGTM. Should we remove the experimental check as well: https://github.com/FFmpeg/FFmpeg/blob/b905ba5bc18c89c7fccd8621795755644442ef19/libavformat/movenc.c#L5989 ? > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
On 4/24/2017 6:48 PM, KongQun Yang wrote: > -- KongQun Yang (KQ) > > On Fri, Apr 21, 2017 at 4:49 PM, Hendrik Leppkes <h.leppkes@gmail.com> > wrote: > >> On Sat, Apr 22, 2017 at 1:25 AM, Hendrik Leppkes <h.leppkes@gmail.com> >> wrote: >>> This brings our generation of the vpcC box up to date to version 1.0 >>> of the VP Codec ISO Media File Format Binding. >>> >>> Specifically, color/transfer properties are now written with values >>> based on ISO/IEC 23001-8, which is the same reference specification the >>> AVColor* enumerations are based on. >>> --- >>> libavformat/movenc.c | 3 ++- >>> libavformat/vpcc.c | 55 +++++------------------------- >> ---------------------- >>> 2 files changed, 7 insertions(+), 51 deletions(-) >>> >> >> Actually I should probably update the main commit message to mention >> that its v1.0 and not a draft any longer? >> > > SGTM. Should we remove the experimental check as well: > https://github.com/FFmpeg/FFmpeg/blob/b905ba5bc18c89c7fccd8621795755644442ef19/libavformat/movenc.c#L5989 > ? IMO yes, assuming the vpcc boxes generated after this patch are 100% v1.0 compliant.
On Mon, Apr 24, 2017 at 11:59 PM, James Almer <jamrial@gmail.com> wrote: > On 4/24/2017 6:48 PM, KongQun Yang wrote: >> -- KongQun Yang (KQ) >> >> On Fri, Apr 21, 2017 at 4:49 PM, Hendrik Leppkes <h.leppkes@gmail.com> >> wrote: >> >>> On Sat, Apr 22, 2017 at 1:25 AM, Hendrik Leppkes <h.leppkes@gmail.com> >>> wrote: >>>> This brings our generation of the vpcC box up to date to version 1.0 >>>> of the VP Codec ISO Media File Format Binding. >>>> >>>> Specifically, color/transfer properties are now written with values >>>> based on ISO/IEC 23001-8, which is the same reference specification the >>>> AVColor* enumerations are based on. >>>> --- >>>> libavformat/movenc.c | 3 ++- >>>> libavformat/vpcc.c | 55 +++++------------------------- >>> ---------------------- >>>> 2 files changed, 7 insertions(+), 51 deletions(-) >>>> >>> >>> Actually I should probably update the main commit message to mention >>> that its v1.0 and not a draft any longer? >>> >> >> SGTM. Should we remove the experimental check as well: >> https://github.com/FFmpeg/FFmpeg/blob/b905ba5bc18c89c7fccd8621795755644442ef19/libavformat/movenc.c#L5989 >> ? > > IMO yes, assuming the vpcc boxes generated after this patch are 100% > v1.0 compliant. The vpcC bos is compliant, but I have no idea if anything else in the muxer is required to create fully compliant files. I randomy stumbled upon the vpcC generation and figured I would update it, I'm not that well versed in movenc otherwise. - Hendrik
On 4/25/2017 6:11 AM, Hendrik Leppkes wrote: > On Mon, Apr 24, 2017 at 11:59 PM, James Almer <jamrial@gmail.com> wrote: >> On 4/24/2017 6:48 PM, KongQun Yang wrote: >>> -- KongQun Yang (KQ) >>> >>> On Fri, Apr 21, 2017 at 4:49 PM, Hendrik Leppkes <h.leppkes@gmail.com> >>> wrote: >>> >>>> On Sat, Apr 22, 2017 at 1:25 AM, Hendrik Leppkes <h.leppkes@gmail.com> >>>> wrote: >>>>> This brings our generation of the vpcC box up to date to version 1.0 >>>>> of the VP Codec ISO Media File Format Binding. >>>>> >>>>> Specifically, color/transfer properties are now written with values >>>>> based on ISO/IEC 23001-8, which is the same reference specification the >>>>> AVColor* enumerations are based on. >>>>> --- >>>>> libavformat/movenc.c | 3 ++- >>>>> libavformat/vpcc.c | 55 +++++------------------------- >>>> ---------------------- >>>>> 2 files changed, 7 insertions(+), 51 deletions(-) >>>>> >>>> >>>> Actually I should probably update the main commit message to mention >>>> that its v1.0 and not a draft any longer? >>>> >>> >>> SGTM. Should we remove the experimental check as well: >>> https://github.com/FFmpeg/FFmpeg/blob/b905ba5bc18c89c7fccd8621795755644442ef19/libavformat/movenc.c#L5989 >>> ? >> >> IMO yes, assuming the vpcc boxes generated after this patch are 100% >> v1.0 compliant. > > The vpcC bos is compliant, but I have no idea if anything else in the > muxer is required to create fully compliant files. I randomy stumbled > upon the vpcC generation and figured I would update it, I'm not that > well versed in movenc otherwise. Don't think so. Should be good as is. You could set "VPC Coding" as compressor name in find_compressor(), but it's optional.
On Tue, Apr 25, 2017 at 11:11 AM, Hendrik Leppkes <h.leppkes@gmail.com> wrote: > On Mon, Apr 24, 2017 at 11:59 PM, James Almer <jamrial@gmail.com> wrote: >> On 4/24/2017 6:48 PM, KongQun Yang wrote: >>> -- KongQun Yang (KQ) >>> >>> On Fri, Apr 21, 2017 at 4:49 PM, Hendrik Leppkes <h.leppkes@gmail.com> >>> wrote: >>> >>>> On Sat, Apr 22, 2017 at 1:25 AM, Hendrik Leppkes <h.leppkes@gmail.com> >>>> wrote: >>>>> This brings our generation of the vpcC box up to date to version 1.0 >>>>> of the VP Codec ISO Media File Format Binding. >>>>> >>>>> Specifically, color/transfer properties are now written with values >>>>> based on ISO/IEC 23001-8, which is the same reference specification the >>>>> AVColor* enumerations are based on. >>>>> --- >>>>> libavformat/movenc.c | 3 ++- >>>>> libavformat/vpcc.c | 55 +++++------------------------- >>>> ---------------------- >>>>> 2 files changed, 7 insertions(+), 51 deletions(-) >>>>> >>>> >>>> Actually I should probably update the main commit message to mention >>>> that its v1.0 and not a draft any longer? >>>> >>> >>> SGTM. Should we remove the experimental check as well: >>> https://github.com/FFmpeg/FFmpeg/blob/b905ba5bc18c89c7fccd8621795755644442ef19/libavformat/movenc.c#L5989 >>> ? >> >> IMO yes, assuming the vpcc boxes generated after this patch are 100% >> v1.0 compliant. > > The vpcC bos is compliant, but I have no idea if anything else in the > muxer is required to create fully compliant files. I randomy stumbled > upon the vpcC generation and figured I would update it, I'm not that > well versed in movenc otherwise. > Pushed without the removal of experimental, that should be done in a separate change anyway. - Hendrik
diff --git a/libavformat/movenc.c b/libavformat/movenc.c index e6e2313c53..e2f57586e2 100644 --- a/libavformat/movenc.c +++ b/libavformat/movenc.c @@ -1112,7 +1112,8 @@ static int mov_write_vpcc_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *tra avio_wb32(pb, 0); ffio_wfourcc(pb, "vpcC"); - avio_wb32(pb, 0); /* version & flags */ + avio_w8(pb, 1); /* version */ + avio_wb24(pb, 0); /* flags */ ff_isom_write_vpcc(s, pb, track->par); return update_size(pb, pos); } diff --git a/libavformat/vpcc.c b/libavformat/vpcc.c index 2390e1711c..df08de59a6 100644 --- a/libavformat/vpcc.c +++ b/libavformat/vpcc.c @@ -23,44 +23,6 @@ #include "libavutil/pixfmt.h" #include "vpcc.h" -enum VpxColorSpace -{ - VPX_COLOR_SPACE_UNSPECIFIED = 0, - VPX_COLOR_SPACE_BT601 = 1, - VPX_COLOR_SPACE_BT709 = 2, - VPX_COLOR_SPACE_SMPTE_170 = 3, - VPX_COLOR_SPACE_SMPTE_240 = 4, - VPX_COLOR_SPACE_BT2020_NCL = 5, - VPX_COLOR_SPACE_BT2020_CL = 6, - VPX_COLOR_SPACE_RGB = 7, -}; - -static int get_vpx_color_space(AVFormatContext *s, - enum AVColorSpace color_space) -{ - switch (color_space) { - case AVCOL_SPC_RGB: - return VPX_COLOR_SPACE_RGB; - case AVCOL_SPC_BT709: - return VPX_COLOR_SPACE_BT709; - case AVCOL_SPC_UNSPECIFIED: - return VPX_COLOR_SPACE_UNSPECIFIED; - case AVCOL_SPC_BT470BG: - return VPX_COLOR_SPACE_BT601; - case AVCOL_SPC_SMPTE170M: - return VPX_COLOR_SPACE_SMPTE_170; - case AVCOL_SPC_SMPTE240M: - return VPX_COLOR_SPACE_SMPTE_240; - case AVCOL_SPC_BT2020_NCL: - return VPX_COLOR_SPACE_BT2020_NCL; - case AVCOL_SPC_BT2020_CL: - return VPX_COLOR_SPACE_BT2020_CL; - default: - av_log(s, AV_LOG_ERROR, "Unsupported color space (%d)\n", color_space); - return -1; - } -} - enum VPX_CHROMA_SUBSAMPLING { VPX_SUBSAMPLING_420_VERTICAL = 0, @@ -100,12 +62,6 @@ static int get_bit_depth(AVFormatContext *s, enum AVPixelFormat pixel_format) return desc->comp[0].depth; } -static int get_vpx_transfer_function( - enum AVColorTransferCharacteristic transfer) -{ - return transfer == AVCOL_TRC_SMPTEST2084; -} - static int get_vpx_video_full_range_flag(enum AVColorRange color_range) { return color_range == AVCOL_RANGE_JPEG; @@ -117,14 +73,12 @@ int ff_isom_write_vpcc(AVFormatContext *s, AVIOContext *pb, int profile = par->profile; int level = par->level == FF_LEVEL_UNKNOWN ? 0 : par->level; int bit_depth = get_bit_depth(s, par->format); - int vpx_color_space = get_vpx_color_space(s, par->color_space); int vpx_chroma_subsampling = get_vpx_chroma_subsampling(s, par->format, par->chroma_location); - int vpx_transfer_function = get_vpx_transfer_function(par->color_trc); int vpx_video_full_range_flag = get_vpx_video_full_range_flag(par->color_range); - if (bit_depth < 0 || vpx_color_space < 0 || vpx_chroma_subsampling < 0) + if (bit_depth < 0 || vpx_chroma_subsampling < 0) return AVERROR_INVALIDDATA; if (profile == FF_PROFILE_UNKNOWN) { @@ -138,9 +92,10 @@ int ff_isom_write_vpcc(AVFormatContext *s, AVIOContext *pb, avio_w8(pb, profile); avio_w8(pb, level); - avio_w8(pb, (bit_depth << 4) | vpx_color_space); - avio_w8(pb, (vpx_chroma_subsampling << 4) | (vpx_transfer_function << 1) | - vpx_video_full_range_flag); + avio_w8(pb, (bit_depth << 4) | (vpx_chroma_subsampling << 1) | vpx_video_full_range_flag); + avio_w8(pb, par->color_primaries); + avio_w8(pb, par->color_trc); + avio_w8(pb, par->color_space); // vp9 does not have codec initialization data. avio_wb16(pb, 0);