On Tue, Sep 26, 2017 at 7:20 PM, James Almer <jamr...@gmail.com> wrote:
> On 9/26/2017 10:08 PM, Aman Gupta wrote: > > From: Aman Gupta <a...@tmm1.net> > > > > --- > > configure | 2 + > > libavcodec/allcodecs.c | 1 + > > libavcodec/hevc_refs.c | 3 + > > libavcodec/hevcdec.c | 12 ++- > > libavcodec/vda_vt_internal.h | 1 + > > libavcodec/videotoolbox.c | 203 ++++++++++++++++++++++++++++++ > +++++++++++++ > > 6 files changed, 221 insertions(+), 1 deletion(-) > > > +CFDataRef ff_videotoolbox_hvcc_extradata_create(AVCodecContext *avctx) > > +{ > > + HEVCContext *h = avctx->priv_data; > > + const HEVCVPS *vps = (const HEVCVPS *)h->ps.vps_list[0]->data; > > + const HEVCSPS *sps = (const HEVCSPS *)h->ps.sps_list[0]->data; > > + int i, num_pps = 0; > > + const HEVCPPS *pps = h->ps.pps; > One thing that surprised me.. when this is invoked, h->vps and h->sps are not yet set. Only h->pps has a value. I had to pull the vps and sps from the vps_list/sps_list directly. > > + PTLCommon ptlc = vps->ptl.general_ptl; > > + VUI vui = sps->vui; > > + uint8_t parallelismType; > > + CFDataRef data = NULL; > > + uint8_t *p; > > + int vt_extradata_size = 23 + 5 + vps->data_size + 5 + > sps->data_size + 3; > > + uint8_t *vt_extradata; > > + > > + 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; > > + vt_extradata_size += 2 + pps->data_size; > > + num_pps++; > > + } > > + } > > + > > + vt_extradata = av_malloc(vt_extradata_size); > > + if (!vt_extradata) > > + return NULL; > > + p = vt_extradata; > > + > > + /* unsigned int(8) configurationVersion = 1; */ > > + AV_W8(p + 0, 1); > > p is uint8_t*. Seems weird using AV_W8() for it. > > Yes, i know ff_videotoolbox_avcc_extradata_create() uses it, but there's > no reason to extend that behavior to new functions. > Are you recommending simple array access instead, i.e. `p[0] = 1`? I just noticed the avcc creation is using AV_WB16() which would simplify some of my code. > > > + > > + /* > > + * unsigned int(2) general_profile_space; > > + * unsigned int(1) general_tier_flag; > > + * unsigned int(5) general_profile_idc; > > + */ > > + AV_W8(p + 1, ptlc.profile_space << 6 | > > + ptlc.tier_flag << 5 | > > + ptlc.profile_idc); > > + > > + /* unsigned int(32) general_profile_compatibility_flags; */ > > + memcpy(p + 2, ptlc.profile_compatibility_flag, 4); > > + > > + /* 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_W8(p + 8, 0); > > + AV_W8(p + 9, 0); > > + AV_W8(p + 10, 0); > > + AV_W8(p + 11, 0); > > + > > + /* unsigned int(8) general_level_idc; */ > > + AV_W8(p + 12, ptlc.level_idc); > > + > > + /* > > + * bit(4) reserved = ‘1111’b; > > + * unsigned int(12) min_spatial_segmentation_idc; > > + */ > > + AV_W8(p + 13, 0xf0 | (vui.min_spatial_segmentation_idc >> 4)); > > + AV_W8(p + 14, vui.min_spatial_segmentation_idc & 0xff); > > + > > + /* > > + * bit(6) reserved = ‘111111’b; > > + * unsigned int(2) parallelismType; > > + */ > > + if (!vui.min_spatial_segmentation_idc) > > + parallelismType = 0; > > + else if (pps->entropy_coding_sync_enabled_flag && > pps->tiles_enabled_flag) > > + parallelismType = 0; > > + else if (pps->entropy_coding_sync_enabled_flag) > > + parallelismType = 3; > > + else if (pps->tiles_enabled_flag) > > + parallelismType = 2; > > + else > > + parallelismType = 1; > > + AV_W8(p + 15, 0xfc | parallelismType); > > + > > + /* > > + * bit(6) reserved = ‘111111’b; > > + * unsigned int(2) chromaFormat; > > + */ > > + AV_W8(p + 16, sps->chroma_format_idc | 0xfc); > > + > > + /* > > + * bit(5) reserved = ‘11111’b; > > + * unsigned int(3) bitDepthLumaMinus8; > > + */ > > + AV_W8(p + 17, (sps->bit_depth - 8) | 0xfc); > > + > > + /* > > + * bit(5) reserved = ‘11111’b; > > + * unsigned int(3) bitDepthChromaMinus8; > > + */ > > + AV_W8(p + 18, (sps->bit_depth_chroma - 8) | 0xfc); > > + > > + /* bit(16) avgFrameRate; */ > > + AV_W8(p + 19, 0); > > + AV_W8(p + 20, 0); > This could be AV_WB16() instead for instance. > > + > > + /* > > + * bit(2) constantFrameRate; > > + * bit(3) numTemporalLayers; > > + * bit(1) temporalIdNested; > > + * unsigned int(2) lengthSizeMinusOne; > > + */ > > + AV_W8(p + 21, 0 << 6 | > > + sps->max_sub_layers << 3 | > > + sps->temporal_id_nesting_flag << 2 | > > + 3); > > + > > + /* unsigned int(8) numOfArrays; */ > > + AV_W8(p + 22, 3); > > + > > + p += 23; > > + /* vps */ > > + /* > > + * bit(1) array_completeness; > > + * unsigned int(1) reserved = 0; > > + * unsigned int(6) NAL_unit_type; > > + */ > > + AV_W8(p, 1 << 7 | > > + HEVC_NAL_VPS & 0x3f); > > + /* unsigned int(16) numNalus; */ > > + AV_W8(p + 1, 0); > > + AV_W8(p + 2, 1); > Same here, with AV_WB16(). > > + /* unsigned int(16) nalUnitLength; */ > > + AV_W8(p + 3, vps->data_size >> 8); > > + AV_W8(p + 4, vps->data_size & 0xffff); > etc, etc. > > + /* bit(8*nalUnitLength) nalUnit; */ > > + memcpy(p + 5, vps->data, vps->data_size); > > + p += 5 + vps->data_size; > > + > > + /* 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); > > + 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); > > + p += 3; > > + for (i = 0; i < MAX_PPS_COUNT; i++) { > > + if (h->ps.pps_list[i]) { > > I think this hints that there's no guarantee that the entire buffer of > size vt_extradata_size will be filled with data. > Keeping that in mind, you should use av_mallocz() for vt_extradata. > > Also, why did you use MAX_PPS_COUNT here but not to set vt_extradata_size? > Hmm, I used MAX_PPS_COUNT in both loops (they should be identical as I copy/pasted), and the calculated size is exact based on the size of the *PS data. This is verified with the assert() at the end of this function. The approach was copied wholesale from avcc_create(). > > > > + 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); > > + memcpy(p + 2, pps->data, pps->data_size); > > + p += 2 + pps->data_size; > > + } > > + } > > + > > + av_assert0(p - vt_extradata == vt_extradata_size); > > This and all the pointer handling you did above makes me think the best > solution would be to use the bytestream2's PutByteContext API instead. > > > + > > + data = CFDataCreate(kCFAllocatorDefault, vt_extradata, > vt_extradata_size); > > As i mentioned above, i don't think the entire buffer is guaranteed to > be always filled to the last byte. The bytestream2 API would let you > keep track of how much you wrote to it and pass that to this function. > I will check out bytestream2 and see if it simplifies things. I'm using an extra loop to pre-calculate the size of the hvcC that I might be able to get rid of with the bytestream2 API. One advantage of the current approach is that it mimics the code in libavformat/hevc.c's hvcc_write(). This will make it easier to keep them in sync in the future. > > > + av_free(vt_extradata); > > + return data; > > +} > > + > > int ff_videotoolbox_buffer_create(VTContext *vtctx, AVFrame *frame) > > { > > av_buffer_unref(&frame->buf[0]); > > @@ -445,6 +614,18 @@ static int videotoolbox_h264_end_frame(AVCodecContext > *avctx) > > return videotoolbox_common_end_frame(avctx, frame); > > } > > > > +static int videotoolbox_hevc_end_frame(AVCodecContext *avctx) > > +{ > > + HEVCContext *h = avctx->priv_data; > > + AVFrame *frame = h->ref->frame; > > + VTContext *vtctx = avctx->internal->hwaccel_priv_data; > > + int ret; > > + > > + ret = videotoolbox_common_end_frame(avctx, frame); > > + vtctx->bitstream_size = 0; > > + return ret; > > +} > > + > > static int videotoolbox_mpeg_start_frame(AVCodecContext *avctx, > > const uint8_t *buffer, > > uint32_t size) > > @@ -501,6 +682,11 @@ static CFDictionaryRef > > videotoolbox_decoder_config_create(CMVideoCodecType > codec > > if (data) > > CFDictionarySetValue(avc_info, CFSTR("avcC"), data); > > break; > > + case kCMVideoCodecType_HEVC : > > + data = ff_videotoolbox_hvcc_extradata_create(avctx); > > + if (data) > > + CFDictionarySetValue(avc_info, CFSTR("hvcC"), data); > > + break; > > default: > > break; > > } > > @@ -600,6 +786,9 @@ static int videotoolbox_default_init(AVCodecContext > *avctx) > > case AV_CODEC_ID_H264 : > > videotoolbox->cm_codec_type = kCMVideoCodecType_H264; > > break; > > + case AV_CODEC_ID_HEVC : > > + videotoolbox->cm_codec_type = kCMVideoCodecType_HEVC; > > + break; > > case AV_CODEC_ID_MPEG1VIDEO : > > videotoolbox->cm_codec_type = kCMVideoCodecType_MPEG1Video; > > break; > > @@ -782,6 +971,20 @@ AVHWAccel ff_h263_videotoolbox_hwaccel = { > > .priv_data_size = sizeof(VTContext), > > }; > > > > +AVHWAccel ff_hevc_videotoolbox_hwaccel = { > > + .name = "hevc_videotoolbox", > > + .type = AVMEDIA_TYPE_VIDEO, > > + .id = AV_CODEC_ID_HEVC, > > + .pix_fmt = AV_PIX_FMT_VIDEOTOOLBOX, > > + .alloc_frame = ff_videotoolbox_alloc_frame, > > + .start_frame = ff_videotoolbox_h264_start_frame, > > + .decode_slice = ff_videotoolbox_h264_decode_slice, > > + .end_frame = videotoolbox_hevc_end_frame, > > + .init = videotoolbox_common_init, > > + .uninit = ff_videotoolbox_uninit, > > + .priv_data_size = sizeof(VTContext), > > +}; > > + > > AVHWAccel ff_h264_videotoolbox_hwaccel = { > > .name = "h264_videotoolbox", > > .type = AVMEDIA_TYPE_VIDEO, > > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel