Hi Moritz,
‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ On Monday, 5 de August de 2019 17:31, Moritz Barsnick <barsn...@gmx.net> wrote: > Hi Andreas, > > 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). The value are native PTS/DTS ticks, so a resolution of 90kHz. So, you agree with something like this? @item offset "The absolute offset value to apply to the PTS/DTS timestamps with a 90kHz resolution." > [...] > > + { "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)? Aah, I feel you like then to full cover all the possible values. Then two changes are required: 1. Use int64 types. 2. Limit offset values from -((2^33)-1) to +((2^33)-1) You agree with this? > > +// TODO: Instead of using absolute timestamp offsets, use frame numbers > > Alternatively? Or perhaps also AV_OPT_TYPE_DURATION? TODO is for later, > anyway... Perhaps I'll remove the TODO comments. The reason to appear is because some other developers in the mailing-list have pointed some interesting ideas for this plugin in the future. I'm not sure to remove it or not. > > + 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. Yes, a small optimization. > > +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. ;-) > Good point! Other bitstream filters have this. I'll update it. Thank you! Regards. A.H. --- _______________________________________________ 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".