[FFmpeg-devel] libavcodec: add timer bitstream filter [v2]

[Moritz Barsnick]

Hi Andreas,

On Fri, Aug 02, 2019 at 13:03:03 +0000, Andreas Håkon wrote:

While trying to figure out how to add features I'd like to see into
this ;-), some remarks:

> +Apply an offset to the PTS/DTS timestamps.
> +
> + at table @option
> + at item offset
> +The offset value to apply to the PTS/DTS timestamps.

For the unknowing, it might be useful to point out what sort of
increments this is (i.e. not an actual "time" stamp or seconds).

> +// TODO: Control time wrapping
[...]
> +    int offset;
[...]
> +    { "offset", NULL, OFFSET(offset), AV_OPT_TYPE_INT, { .i64 = 0 }, INT_MIN, INT_MAX, FLAGS },

Considering PTS/DTS is int64, wouldn't it be useful for the offset also
being int64 (and using limits INT64_MIN, INT64_MAX)?

> +// TODO: Instead of using absolute timestamp offsets, use frame numbers

Alternatively? Or perhaps also AV_OPT_TYPE_DURATION? TODO is for later,
anyway...

> +    if (!s->first_debug_done) {
> +        av_log(ctx, AV_LOG_DEBUG, "Updated PTS/DTS (%"PRId64"/%"PRId64" : %"PRId64"/%"PRId64") with offset:%d\n",
> +                pkt->pts, pkt->dts, opts, odts, s->offset );
> +    }
> +    s->first_debug_done = 1;

You should set the variable inside the if() block. otherwise it gets
assigned on every packet.

> +static const AVClass timer_class = {
> +    .class_name = "timer",

In about half of the existing BSFs, this would be called "timer_bsf". I
don't care, I just look at other existing code. ;-)

Cheers,
Moritz