On Tue, 26 Sep 2017 19:49:34 -0700
Aman Gupta <ffm...@tmm1.net> wrote:
> 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.
AV_W8 doesn't actually exist - it's just a trivial macro defined in
videotoolbox.c. I added it because it looks more readable and symmetric,
especially mixed with AV_W16.
As for using bytestream writers etc. - I chose to manually write the
data, because this was the simplest at the time.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
