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. > Also please document that the type is codec specific (it is IIUC) Yes; above there is: """ typedef struct CodedBitstreamUnit { /** * Codec-specific type of this unit. */ uint32_t type; """ Thanks, - Mark _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel