Message ID | LLkcXIUui8Sk98nsBMs-u01h58EcsnAkRLkDHuroEt65QJLLH56WwahnlxoJas4oUQXW_WXz8HliZRMwF_P0jznXcRLdwkmAgw6etDV1Ws4=@protonmail.com |
---|---|
State | New |
Headers | show |
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
Hi Moritz, ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ On Monday, 5 de August de 2019 17:31, Moritz Barsnick <barsnick@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. ---
Hi Andreas! On 2019-08-05 17:25 +0000, Andreas Håkon wrote: > On Monday, 5 de August de 2019 17:31, Moritz Barsnick <barsnick@gmx.net> wrote: > [...] > > > +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! Sorry for the noise. IIRC the naming was previously discussed. Can't remember the details right now. Unfortunately I don't have any good name suggestions too :( This is no objections, but at least to me the name timer sounds like some autonomous unit/process that can interrupt/call your process on defined/configured/subscribed intervals. Maybe it's just me... ...what do others think? Alexander
Hi Alexander, ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ On Wednesday, 7 de August de 2019 0:24, Alexander Strasser <eclipse7@gmx.net> wrote: > Hi Andreas! > > On 2019-08-05 17:25 +0000, Andreas Håkon wrote: > > > On Monday, 5 de August de 2019 17:31, Moritz Barsnick barsnick@gmx.net wrote: > > [...] > > > > > +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! > > Sorry for the noise. IIRC the naming was previously discussed. > Can't remember the details right now. Unfortunately I don't have > any good name suggestions too :( > > This is no objections, but at least to me the name timer sounds > like some autonomous unit/process that can interrupt/call your > process on defined/configured/subscribed intervals. Maybe it's > just me... > > ...what do others think? It's nitial name was "retimer". After a suggestion I changed to "timer". Other options can be "timestamp", "synchronizer", etc. However, they're all a little vague. Therefore, I ask someone to present an appropriate name taking into account the objective: shifting the PTS/DTS marks of a stream. You like "timeshift"? Regards. A.H. ---
Hi Alexander, ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ On Wednesday, 7 de August de 2019 10:01, Andreas Håkon <andreas.hakon@protonmail.com> wrote: > Hi Alexander, > > ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ > On Wednesday, 7 de August de 2019 0:24, Alexander Strasser eclipse7@gmx.net wrote: > > > Hi Andreas! > > On 2019-08-05 17:25 +0000, Andreas Håkon wrote: > > > > > On Monday, 5 de August de 2019 17:31, Moritz Barsnick barsnick@gmx.net wrote: > > > > [...] > > > > Sorry for the noise. IIRC the naming was previously discussed. > > Can't remember the details right now. Unfortunately I don't have > > any good name suggestions too :( > > This is no objections, but at least to me the name timer sounds > > like some autonomous unit/process that can interrupt/call your > > process on defined/configured/subscribed intervals. Maybe it's > > just me... > > ...what do others think? > > It's nitial name was "retimer". After a suggestion I changed to "timer". > Other options can be "timestamp", "synchronizer", etc. > However, they're all a little vague. > > Therefore, I ask someone to present an appropriate name taking > into account the objective: shifting the PTS/DTS marks of a stream. > > You like "timeshift"? Check new version: https://patchwork.ffmpeg.org/patch/14302/ Regards. A.H. ---
diff --git a/doc/bitstream_filters.texi b/doc/bitstream_filters.texi index 50a1679..2d92eeb 100644 --- a/doc/bitstream_filters.texi +++ b/doc/bitstream_filters.texi @@ -644,6 +644,16 @@ codec) with metadata headers. See also the @ref{mov2textsub} filter. +@section timer + +Apply an offset to the PTS/DTS timestamps. + +@table @option +@item offset +The offset value to apply to the PTS/DTS timestamps. + +@end table + @section trace_headers Log trace output containing all syntax elements in the coded stream diff --git a/libavcodec/Makefile b/libavcodec/Makefile index 3cd73fb..2c2f018 100644 --- a/libavcodec/Makefile +++ b/libavcodec/Makefile @@ -1096,6 +1096,7 @@ OBJS-$(CONFIG_NULL_BSF) += null_bsf.o OBJS-$(CONFIG_PRORES_METADATA_BSF) += prores_metadata_bsf.o OBJS-$(CONFIG_REMOVE_EXTRADATA_BSF) += remove_extradata_bsf.o OBJS-$(CONFIG_TEXT2MOVSUB_BSF) += movsub_bsf.o +OBJS-$(CONFIG_TIMER_BSF) += timer_bsf.o OBJS-$(CONFIG_TRACE_HEADERS_BSF) += trace_headers_bsf.o OBJS-$(CONFIG_TRUEHD_CORE_BSF) += truehd_core_bsf.o mlp_parse.o mlp.o OBJS-$(CONFIG_VP9_METADATA_BSF) += vp9_metadata_bsf.o diff --git a/libavcodec/bitstream_filters.c b/libavcodec/bitstream_filters.c index 4630039..be6af05 100644 --- a/libavcodec/bitstream_filters.c +++ b/libavcodec/bitstream_filters.c @@ -51,6 +51,7 @@ extern const AVBitStreamFilter ff_null_bsf; extern const AVBitStreamFilter ff_prores_metadata_bsf; extern const AVBitStreamFilter ff_remove_extradata_bsf; extern const AVBitStreamFilter ff_text2movsub_bsf; +extern const AVBitStreamFilter ff_timer_bsf; extern const AVBitStreamFilter ff_trace_headers_bsf; extern const AVBitStreamFilter ff_truehd_core_bsf; extern const AVBitStreamFilter ff_vp9_metadata_bsf; diff --git a/libavcodec/timer_bsf.c b/libavcodec/timer_bsf.c new file mode 100644 index 0000000..3f1c7f6 --- /dev/null +++ b/libavcodec/timer_bsf.c @@ -0,0 +1,86 @@ +/* + * This file is part of FFmpeg. + * + * FFmpeg is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * FFmpeg is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with FFmpeg; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +/** + * @file + * This bitstream filter applies an offset to the PTS/DTS timestamps. + * + */ + +// TODO: Control time wrapping +// TODO: Instead of using absolute timestamp offsets, use frame numbers + +#include "avcodec.h" +#include "bsf.h" + +#include "libavutil/opt.h" + +typedef struct TimerContext { + const AVClass *class; + int offset; + int first_debug_done; +} TimerContext; + +static int timer_filter(AVBSFContext *ctx, AVPacket *pkt) +{ + int ret; + TimerContext *s = ctx->priv_data; + int64_t opts,odts; + + ret = ff_bsf_get_packet_ref(ctx, pkt); + if (ret < 0) + return ret; + + opts = pkt->pts; + if (pkt->pts != AV_NOPTS_VALUE) { + pkt->pts += s->offset; + } + odts = pkt->dts; + if (pkt->dts != AV_NOPTS_VALUE) { + pkt->dts += s->offset; + } + + 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; + + return 0; +} + +#define OFFSET(x) offsetof(TimerContext, x) +#define FLAGS (AV_OPT_FLAG_VIDEO_PARAM|AV_OPT_FLAG_AUDIO_PARAM|AV_OPT_FLAG_SUBTITLE_PARAM|AV_OPT_FLAG_BSF_PARAM) +static const AVOption options[] = { + { "offset", NULL, OFFSET(offset), AV_OPT_TYPE_INT, { .i64 = 0 }, INT_MIN, INT_MAX, FLAGS }, + { NULL }, +}; + +static const AVClass timer_class = { + .class_name = "timer", + .item_name = av_default_item_name, + .option = options, + .version = LIBAVUTIL_VERSION_INT, +}; + +const AVBitStreamFilter ff_timer_bsf = { + .name = "timer", + .priv_data_size = sizeof(TimerContext), + .priv_class = &timer_class, + .filter = timer_filter, +};