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.
> +
> +@table @option
> +@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
_______________________________________________
ffmpeg-devel mailing list
[email protected]
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
[email protected] with subject "unsubscribe".
