On Fri, 27 Jan 2017 16:09:06 +0200 (EET) Martin Storsjö <[email protected]> wrote:
> > + // common Webm codecs are not part of RFC 6381
> > + switch (par->codec_id) {
> > + case AV_CODEC_ID_VP8:
> > + snprintf(str, size, "vp8");
> > + return;
> > + case AV_CODEC_ID_VP9:
> > + snprintf(str, size, "vp9");
> > + return;
> > + case AV_CODEC_ID_VORBIS:
> > + snprintf(str, size, "vorbis");
> > + return;
> > + case AV_CODEC_ID_OPUS:
> > + snprintf(str, size, "opus");
> > + return;
> > + }
> > +
>
> Hmm, I'm pondering if it'd be worth to store these in some more compact
> form, like a table, for all codecs which just have a single simple plain
> string?
Do you have some example code? I'm not sure if I get how to do tables in C.
Like with a struct?
struct codec_string {
int codec_id;
const char *codec_str;
};
struct codec_string codecs[] = {
{AV_CODEC_ID_VP8, "vp8"},
{AV_CODEC_ID_VP9, "vp9"},
...
};
> Using oformat->name here feels a little fragile. I think I'd rather have
> that stored as a string somewhere in the context. (Not sure if it's better
> to have it "flexible" to compose it as video/<string> and audio/<string>,
> or store the full "video/mp4" etc as audioMime and videoMime.)
I might opt for the short version, since I use the format name also for
conditionals like
if (!strcmp(os->ctx->oformat->name, "mp4"))
Therefor maybe a char *format_name in the OutputStream struct?
> > ctx->interrupt_callback = s->interrupt_callback;
> > ctx->opaque = s->opaque;
> > ctx->io_close = s->io_close;
> > ctx->io_open = s->io_open;
> > + av_dict_copy(&ctx->metadata, s->metadata, 0);
>
> Does the webm muxer need some specific metadata which we don't pass
> through right now, or is it just for making sure that metadata ends up set
> on the chained muxer and included in the stream? I'd at least want a
> comment in the commit message explaining why this is necessary.
No specific metadata is needed. I can move this to a separate commit.
> > if (!(st = avformat_new_stream(ctx, NULL))) {
> > ret = AVERROR(ENOMEM);
> > @@ -798,7 +829,10 @@ static int dash_write_header(AVFormatContext *s)
> > avcodec_parameters_copy(st->codecpar, s->streams[i]->codecpar);
> > st->sample_aspect_ratio = s->streams[i]->sample_aspect_ratio;
> > st->time_base = s->streams[i]->time_base;
> > + st->avg_frame_rate = s->streams[i]->avg_frame_rate;
> > ctx->avoid_negative_ts = s->avoid_negative_ts;
> > + ctx->flags = s->flags;
> > + ctx->max_delay = s->max_delay;
>
> Are these new settings strictly necessary for chained-webm? If not, I'd
> rather add them in a separate commit afterwards, with an explanation on
> what it helps for.
To be honest, I haven't checked. I just looked at what other muxers having
subordinate muxers do, when they initialize the context.
Regards
Peter
pgpMsVZki2eCC.pgp
Description: OpenPGP digital signature
_______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
