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 ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".