On Thu, Sep 14, 2017 at 09:51:31PM +0100, Mark Thompson wrote:
> On 14/09/17 21:28, Michael Niedermayer wrote:
> > On Thu, Sep 14, 2017 at 08:31:28AM +0100, Mark Thompson wrote:
> >> On 14/09/17 01:42, Michael Niedermayer wrote:
> >>> On Wed, Sep 13, 2017 at 12:43:53AM +0100, Mark Thompson wrote:
> > [...]
> >
> >>>> +int ff_cbs_write_packet(CodedBitstreamContext *ctx,
> >>>> + AVPacket *pkt,
> >>>> + CodedBitstreamFragment *frag)
> >>>> +{
> >>>> + int err;
> >>>> +
> >>>> + err = ff_cbs_write_fragment_data(ctx, frag);
> >>>> + if (err < 0)
> >>>> + return err;
> >>>> +
> >>>
> >>>> + av_new_packet(pkt, frag->data_size);
> >>>> + if (err < 0)
> >>>> + return err;
> >>>
> >>> that does not work
> >>
> >> I think I'm missing something. Can you be more precise about what doesn't
> >> work?
> >
> > you ignore the return code of av_new_packet()
> > the check does nothing
>
> Duh, oops. I spent a while looking for some subtlety in av_new_packet() and
> missed the obvious. Sorry!
>
> >>>> + */
> >>>> + int nb_units;
> >>>> + /**
> >>>> + * Pointer to an array of units of length nb_units.
> >>>> + *
> >>>> + * Must be NULL if nb_units is zero.
> >>>> + */
> >>>> + CodedBitstreamUnit *units;
> >>>> +} CodedBitstreamFragment;
> >>>> +
> >>>> +/**
> >>>> + * Context structure for coded bitstream operations.
> >>>> + */
> >>>> +typedef struct CodedBitstreamContext {
> >>>> + /**
> >>>> + * Logging context to be passed to all av_log() calls associated
> >>>> + * with this context.
> >>>> + */
> >>>> + void *log_ctx;
> >>>> +
> >>>> + /**
> >>>> + * Internal codec-specific hooks.
> >>>> + */
> >>>> + const struct CodedBitstreamType *codec;
> >>>> +
> >>>> + /**
> >>>> + * Internal codec-specific data.
> >>>> + *
> >>>> + * This contains any information needed when reading/writing
> >>>> + * bitsteams which will not necessarily be present in a fragment.
> >>>> + * For example, for H.264 it contains all currently visible
> >>>> + * parameter sets - they are required to determine the bitstream
> >>>> + * syntax but need not be present in every access unit.
> >>>> + */
> >>>> + void *priv_data;
> >>>> +
> >>>
> >>>> + /**
> >>>> + * Array of unit types which should be decomposed when reading.
> >>>> + *
> >>>> + * Types not in this list will be available in bitstream form only.
> >>>> + * If NULL, all supported types will be decomposed.
> >>>> + */
> >>>> + uint32_t *decompose_unit_types;
> >>>
> >>> this should not be a generic integer.
> >>> a typedef or enum would be better
> >>
> >> It's H.264 nal unit types union H.265 nal unit types union MPEG-2 start
> >> codes (union any other codec that gets added).
> >>
> >> What type should that be? I chose uint32_t to make it fixed-size and
> >> hopefully large enough to be able to make sense for any future codec.
> >
> > a typedef based type would be better than a litterally hardcoded
> > one. A hardcoded type would be duplicatedly hardcoded in many
> > places ...
>
> So have "typedef uint32_t CBSUnitType;"? I'm not really sure this adds
> anything since every implementation is using its own enum anyway, but ok.duplication is bad consider you ever want to change the underlaying type ... [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I have often repented speaking, but never of holding my tongue. -- Xenocrates
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
