Message ID | 20170927221920.46302-1-ffmpeg@tmm1.net |
---|---|
State | New |
Headers | show |
On 9/27/2017 7:19 PM, Aman Gupta wrote: > From: Aman Gupta <aman@tmm1.net> > > additional changes to hevc patchset, as suggested on-list > > if these look fine, I will squash into previous patchset and push it to > master. > --- > libavcodec/videotoolbox.c | 27 +++++++++------------------ > 1 file changed, 9 insertions(+), 18 deletions(-) > > diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c > index 078174cc61..c96200cbdb 100644 > --- a/libavcodec/videotoolbox.c > +++ b/libavcodec/videotoolbox.c > @@ -165,10 +165,8 @@ CFDataRef ff_videotoolbox_hvcc_extradata_create(AVCodecContext *avctx) > ptlc.non_packed_constraint_flag << 5 | > ptlc.frame_only_constraint_flag << 4); > AV_W8(p + 7, 0); > - AV_W8(p + 8, 0); > - AV_W8(p + 9, 0); > - AV_W8(p + 10, 0); > - AV_W8(p + 11, 0); > + AV_WB16(p + 8, 0); > + AV_WB16(p + 10, 0); AV_WB32(p + 8, 0) or even AV_WN32(p + 8, 0) since it's 0 and endianness doesn't matter. > > /* unsigned int(8) general_level_idc; */ > AV_W8(p + 12, ptlc.level_idc); > @@ -215,8 +213,7 @@ CFDataRef ff_videotoolbox_hvcc_extradata_create(AVCodecContext *avctx) > AV_W8(p + 18, (sps->bit_depth_chroma - 8) | 0xfc); > > /* bit(16) avgFrameRate; */ > - AV_W8(p + 19, 0); > - AV_W8(p + 20, 0); > + AV_WB16(p + 19, 0); > > /* > * bit(2) constantFrameRate; > @@ -242,11 +239,9 @@ CFDataRef ff_videotoolbox_hvcc_extradata_create(AVCodecContext *avctx) > AV_W8(p, 1 << 7 | > HEVC_NAL_VPS & 0x3f); > /* unsigned int(16) numNalus; */ > - AV_W8(p + 1, 0); > - AV_W8(p + 2, 1); > + AV_WB16(p + 1, 1); > /* unsigned int(16) nalUnitLength; */ > - AV_W8(p + 3, vps->data_size >> 8); > - AV_W8(p + 4, vps->data_size & 0xffff); > + AV_WB16(p + 3, vps->data_size); > /* bit(8*nalUnitLength) nalUnit; */ > memcpy(p + 5, vps->data, vps->data_size); > p += 5 + vps->data_size; > @@ -254,24 +249,20 @@ CFDataRef ff_videotoolbox_hvcc_extradata_create(AVCodecContext *avctx) > /* sps */ > AV_W8(p, 1 << 7 | > HEVC_NAL_SPS & 0x3f); > - AV_W8(p + 1, 0); > - AV_W8(p + 2, 1); > - AV_W8(p + 3, sps->data_size >> 8); > - AV_W8(p + 4, sps->data_size & 0xffff); > + AV_WB16(p + 1, 1); > + AV_WB16(p + 3, sps->data_size); > memcpy(p + 5, sps->data, sps->data_size); > p += 5 + sps->data_size; > > /* pps */ > AV_W8(p, 1 << 7 | > HEVC_NAL_PPS & 0x3f); > - AV_W8(p + 1, num_pps >> 8); > - AV_W8(p + 2, num_pps & 0xffff); > + AV_WB16(p + 1, num_pps); > p += 3; > for (i = 0; i < MAX_PPS_COUNT; i++) { > if (h->ps.pps_list[i]) { > const HEVCPPS *pps = (const HEVCPPS *)h->ps.pps_list[i]->data; > - AV_W8(p + 0, pps->data_size >> 8); > - AV_W8(p + 1, pps->data_size & 0xffff); > + AV_WB16(p, pps->data_size); > memcpy(p + 2, pps->data, pps->data_size); > p += 2 + pps->data_size; > } >
On Wed, 27 Sep 2017 19:45:23 -0300 James Almer <jamrial@gmail.com> wrote: > On 9/27/2017 7:19 PM, Aman Gupta wrote: > > From: Aman Gupta <aman@tmm1.net> > > > > additional changes to hevc patchset, as suggested on-list > > > > if these look fine, I will squash into previous patchset and push it to > > master. > > --- > > libavcodec/videotoolbox.c | 27 +++++++++------------------ > > 1 file changed, 9 insertions(+), 18 deletions(-) > > > > diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c > > index 078174cc61..c96200cbdb 100644 > > --- a/libavcodec/videotoolbox.c > > +++ b/libavcodec/videotoolbox.c > > @@ -165,10 +165,8 @@ CFDataRef ff_videotoolbox_hvcc_extradata_create(AVCodecContext *avctx) > > ptlc.non_packed_constraint_flag << 5 | > > ptlc.frame_only_constraint_flag << 4); > > AV_W8(p + 7, 0); > > - AV_W8(p + 8, 0); > > - AV_W8(p + 9, 0); > > - AV_W8(p + 10, 0); > > - AV_W8(p + 11, 0); > > + AV_WB16(p + 8, 0); > > + AV_WB16(p + 10, 0); > > AV_WB32(p + 8, 0) > > or even AV_WN32(p + 8, 0) since it's 0 and endianness doesn't matter. IMO it should reflect whatever bitstream elements these are. Ideally, we'd have comments about what each element is too.
On Wed, Sep 27, 2017 at 4:13 PM, wm4 <nfxjfg@googlemail.com> wrote: > On Wed, 27 Sep 2017 19:45:23 -0300 > James Almer <jamrial@gmail.com> wrote: > > > On 9/27/2017 7:19 PM, Aman Gupta wrote: > > > From: Aman Gupta <aman@tmm1.net> > > > > > > additional changes to hevc patchset, as suggested on-list > > > > > > if these look fine, I will squash into previous patchset and push it to > > > master. > > > --- > > > libavcodec/videotoolbox.c | 27 +++++++++------------------ > > > 1 file changed, 9 insertions(+), 18 deletions(-) > > > > > > diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c > > > index 078174cc61..c96200cbdb 100644 > > > --- a/libavcodec/videotoolbox.c > > > +++ b/libavcodec/videotoolbox.c > > > @@ -165,10 +165,8 @@ CFDataRef ff_videotoolbox_hvcc_extradata_create(AVCodecContext > *avctx) > > > ptlc.non_packed_constraint_flag << 5 | > > > ptlc.frame_only_constraint_flag << 4); > > > AV_W8(p + 7, 0); > > > - AV_W8(p + 8, 0); > > > - AV_W8(p + 9, 0); > > > - AV_W8(p + 10, 0); > > > - AV_W8(p + 11, 0); > > > + AV_WB16(p + 8, 0); > > > + AV_WB16(p + 10, 0); > > > > AV_WB32(p + 8, 0) > > > > or even AV_WN32(p + 8, 0) since it's 0 and endianness doesn't matter. > > IMO it should reflect whatever bitstream elements these are. Ideally, > we'd have comments about what each element is too. > There is already a comment in this section. It's supposed to be 48bits of flags, but the spec has 44 of those bits defined as 'reserved zero bits'. /* unsigned int(48) general_constraint_indicator_flags; */ AV_W8(p + 6, ptlc.progressive_source_flag << 7 | ptlc.interlaced_source_flag << 6 | ptlc.non_packed_constraint_flag << 5 | ptlc.frame_only_constraint_flag << 4); AV_W8(p + 7, 0); AV_WN32(p + 8, 0); In hvcc_write (lavf/hevc.c), libavformat uses: /* unsigned int(48) general_constraint_indicator_flags; */ avio_wb32(pb, hvcc->general_constraint_indicator_flags >> 16); avio_wb16(pb, hvcc->general_constraint_indicator_flags); But the PTL parsing code in libavcodec defines the flags individually. See decode_profile_tier_level (lavc/hevc_ps.c): ptl->progressive_source_flag = get_bits1(gb); ptl->interlaced_source_flag = get_bits1(gb); ptl->non_packed_constraint_flag = get_bits1(gb); ptl->frame_only_constraint_flag = get_bits1(gb); skip_bits(gb, 16); // XXX_reserved_zero_44bits[0..15] skip_bits(gb, 16); // XXX_reserved_zero_44bits[16..31] skip_bits(gb, 12); // XXX_reserved_zero_44bits[32..43] Aman > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
diff --git a/libavcodec/videotoolbox.c b/libavcodec/videotoolbox.c index 078174cc61..c96200cbdb 100644 --- a/libavcodec/videotoolbox.c +++ b/libavcodec/videotoolbox.c @@ -165,10 +165,8 @@ CFDataRef ff_videotoolbox_hvcc_extradata_create(AVCodecContext *avctx) ptlc.non_packed_constraint_flag << 5 | ptlc.frame_only_constraint_flag << 4); AV_W8(p + 7, 0); - AV_W8(p + 8, 0); - AV_W8(p + 9, 0); - AV_W8(p + 10, 0); - AV_W8(p + 11, 0); + AV_WB16(p + 8, 0); + AV_WB16(p + 10, 0); /* unsigned int(8) general_level_idc; */ AV_W8(p + 12, ptlc.level_idc); @@ -215,8 +213,7 @@ CFDataRef ff_videotoolbox_hvcc_extradata_create(AVCodecContext *avctx) AV_W8(p + 18, (sps->bit_depth_chroma - 8) | 0xfc); /* bit(16) avgFrameRate; */ - AV_W8(p + 19, 0); - AV_W8(p + 20, 0); + AV_WB16(p + 19, 0); /* * bit(2) constantFrameRate; @@ -242,11 +239,9 @@ CFDataRef ff_videotoolbox_hvcc_extradata_create(AVCodecContext *avctx) AV_W8(p, 1 << 7 | HEVC_NAL_VPS & 0x3f); /* unsigned int(16) numNalus; */ - AV_W8(p + 1, 0); - AV_W8(p + 2, 1); + AV_WB16(p + 1, 1); /* unsigned int(16) nalUnitLength; */ - AV_W8(p + 3, vps->data_size >> 8); - AV_W8(p + 4, vps->data_size & 0xffff); + AV_WB16(p + 3, vps->data_size); /* bit(8*nalUnitLength) nalUnit; */ memcpy(p + 5, vps->data, vps->data_size); p += 5 + vps->data_size; @@ -254,24 +249,20 @@ CFDataRef ff_videotoolbox_hvcc_extradata_create(AVCodecContext *avctx) /* sps */ AV_W8(p, 1 << 7 | HEVC_NAL_SPS & 0x3f); - AV_W8(p + 1, 0); - AV_W8(p + 2, 1); - AV_W8(p + 3, sps->data_size >> 8); - AV_W8(p + 4, sps->data_size & 0xffff); + AV_WB16(p + 1, 1); + AV_WB16(p + 3, sps->data_size); memcpy(p + 5, sps->data, sps->data_size); p += 5 + sps->data_size; /* pps */ AV_W8(p, 1 << 7 | HEVC_NAL_PPS & 0x3f); - AV_W8(p + 1, num_pps >> 8); - AV_W8(p + 2, num_pps & 0xffff); + AV_WB16(p + 1, num_pps); p += 3; for (i = 0; i < MAX_PPS_COUNT; i++) { if (h->ps.pps_list[i]) { const HEVCPPS *pps = (const HEVCPPS *)h->ps.pps_list[i]->data; - AV_W8(p + 0, pps->data_size >> 8); - AV_W8(p + 1, pps->data_size & 0xffff); + AV_WB16(p, pps->data_size); memcpy(p + 2, pps->data, pps->data_size); p += 2 + pps->data_size; }
From: Aman Gupta <aman@tmm1.net> additional changes to hevc patchset, as suggested on-list if these look fine, I will squash into previous patchset and push it to master. --- libavcodec/videotoolbox.c | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-)