* [FFmpeg-devel] [PATCH] ffmpeg: fix implementation of updated input start time
@ 2022-09-27 4:40 Gyan Doshi
2022-10-02 19:46 ` Gyan Doshi
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Gyan Doshi @ 2022-09-27 4:40 UTC (permalink / raw)
To: ffmpeg-devel
The current adjustment of input start times just adjusts the tsoffset.
And it does so, by resetting the tsoffset to nullify the new start time.
This leads to breakage of -copyts, ignoring of user_ts_offset, breaking
of -isync as well as breaking wrap correction.
Fixed by taking cognizance of these parameters, and by correcting start
times just before sync offsets are applied.
---
fftools/ffmpeg.c | 24 +-------------------
fftools/ffmpeg.h | 5 ++++-
fftools/ffmpeg_demux.c | 4 ++--
fftools/ffmpeg_opt.c | 50 +++++++++++++++++++++++++++++++++++++-----
4 files changed, 52 insertions(+), 31 deletions(-)
diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 0e1477299d..fabb0fb952 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -1898,7 +1898,7 @@ static void do_streamcopy(InputStream *ist, OutputStream *ost, const AVPacket *p
start_time = 0;
if (copy_ts) {
start_time += f->start_time != AV_NOPTS_VALUE ? f->start_time : 0;
- start_time += start_at_zero ? 0 : f->ctx->start_time;
+ start_time += start_at_zero ? 0 : f->enabled_start_time;
}
if (ist->pts >= f->recording_time + start_time) {
close_output_stream(ost);
@@ -3323,28 +3323,6 @@ static int transcode_init(void)
input_streams[j + ifile->ist_index]->start = av_gettime_relative();
}
- // Correct starttime based on the enabled streams
- for (i = 0; i < nb_input_files; i++) {
- InputFile *ifile = input_files[i];
- AVFormatContext *is = ifile->ctx;
- int64_t new_start_time = INT64_MAX;
-
- if (is->start_time == AV_NOPTS_VALUE ||
- !(is->iformat->flags & AVFMT_TS_DISCONT))
- continue;
-
- for (int j = 0; j < is->nb_streams; j++) {
- AVStream *st = is->streams[j];
- if(st->discard == AVDISCARD_ALL || st->start_time == AV_NOPTS_VALUE)
- continue;
- new_start_time = FFMIN(new_start_time, av_rescale_q(st->start_time, st->time_base, AV_TIME_BASE_Q));
- }
- if (new_start_time > is->start_time) {
- av_log(is, AV_LOG_VERBOSE, "Correcting start time by %"PRId64"\n", new_start_time - is->start_time);
- ifile->ts_offset = -new_start_time;
- }
- }
-
/* init input streams */
for (i = 0; i < nb_input_streams; i++)
if ((ret = init_input_stream(i, error, sizeof(error))) < 0)
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index ede0b2bd96..b93c2427b6 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -439,7 +439,10 @@ typedef struct InputFile {
AVRational time_base; /* time base of the duration */
int64_t input_ts_offset;
int input_sync_ref;
-
+ /**
+ * Effective format start time based on enabled streams.
+ */
+ int64_t enabled_start_time;
int64_t ts_offset;
/**
* Extra timestamp offset added by discontinuity handling.
diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
index 6e89f5999a..6125181f81 100644
--- a/fftools/ffmpeg_demux.c
+++ b/fftools/ffmpeg_demux.c
@@ -120,7 +120,7 @@ static int seek_to_start(InputFile *ifile)
static void ts_fixup(InputFile *ifile, AVPacket *pkt, int *repeat_pict)
{
InputStream *ist = input_streams[ifile->ist_index + pkt->stream_index];
- const int64_t start_time = ifile->ctx->start_time;
+ const int64_t start_time = ifile->enabled_start_time;
int64_t duration;
if (debug_ts) {
@@ -367,7 +367,7 @@ int ifile_get_packet(InputFile *f, AVPacket **pkt)
if (f->readrate || f->rate_emu) {
int i;
int64_t file_start = copy_ts * (
- (f->ctx->start_time != AV_NOPTS_VALUE ? f->ctx->start_time * !start_at_zero : 0) +
+ (f->enabled_start_time != AV_NOPTS_VALUE ? f->enabled_start_time * !start_at_zero : 0) +
(f->start_time != AV_NOPTS_VALUE ? f->start_time : 0)
);
float scale = f->rate_emu ? 1.0 : f->readrate;
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index 5febe319e4..8df322c31c 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -293,6 +293,44 @@ static int parse_and_set_vsync(const char *arg, int *vsync_var, int file_idx, in
return 0;
}
+/* Correct input file start times based on enabled streams */
+static int correct_input_start_times(void)
+{
+ for (int i = 0; i < nb_input_files; i++) {
+ InputFile *ifile = input_files[i];
+ AVFormatContext *is = ifile->ctx;
+ int64_t new_start_time = INT64_MAX, diff, abs_start_seek;
+
+ ifile->enabled_start_time = is->start_time;
+
+ if (is->start_time == AV_NOPTS_VALUE ||
+ !(is->iformat->flags & AVFMT_TS_DISCONT))
+ continue;
+
+ for (int j = 0; j < is->nb_streams; j++) {
+ AVStream *st = is->streams[j];
+ if(st->discard == AVDISCARD_ALL || st->start_time == AV_NOPTS_VALUE)
+ continue;
+ new_start_time = FFMIN(new_start_time, av_rescale_q(st->start_time, st->time_base, AV_TIME_BASE_Q));
+ }
+
+ diff = new_start_time - is->start_time;
+ if (diff) {
+ av_log(NULL, AV_LOG_VERBOSE, "Correcting start time of Input #%d by %"PRId64" us.\n", i, diff);
+ ifile->enabled_start_time = new_start_time;
+ if (copy_ts && start_at_zero)
+ ifile->ts_offset = -new_start_time;
+ else if (!copy_ts) {
+ abs_start_seek = is->start_time + (ifile->start_time != AV_NOPTS_VALUE) ? ifile->start_time : 0;
+ ifile->ts_offset = abs_start_seek > new_start_time ? -abs_start_seek : -new_start_time;
+ }
+ ifile->ts_offset += ifile->input_ts_offset;
+ }
+ }
+
+ return 0;
+}
+
static int apply_sync_offsets(void)
{
for (int i = 0; i < nb_input_files; i++) {
@@ -321,9 +359,9 @@ static int apply_sync_offsets(void)
if (self->ctx->start_time_realtime != AV_NOPTS_VALUE && ref->ctx->start_time_realtime != AV_NOPTS_VALUE) {
self_start_time = self->ctx->start_time_realtime;
ref_start_time = ref->ctx->start_time_realtime;
- } else if (self->ctx->start_time != AV_NOPTS_VALUE && ref->ctx->start_time != AV_NOPTS_VALUE) {
- self_start_time = self->ctx->start_time;
- ref_start_time = ref->ctx->start_time;
+ } else if (self->enabled_start_time != AV_NOPTS_VALUE && ref->enabled_start_time != AV_NOPTS_VALUE) {
+ self_start_time = self->enabled_start_time;
+ ref_start_time = ref->enabled_start_time;
} else {
start_times_set = 0;
}
@@ -3747,8 +3785,6 @@ int ffmpeg_parse_options(int argc, char **argv)
goto fail;
}
- apply_sync_offsets();
-
/* create the complex filtergraphs */
ret = init_complex_filters();
if (ret < 0) {
@@ -3763,6 +3799,10 @@ int ffmpeg_parse_options(int argc, char **argv)
goto fail;
}
+ correct_input_start_times();
+
+ apply_sync_offsets();
+
check_filter_outputs();
fail:
--
2.36.1
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [FFmpeg-devel] [PATCH] ffmpeg: fix implementation of updated input start time
2022-09-27 4:40 [FFmpeg-devel] [PATCH] ffmpeg: fix implementation of updated input start time Gyan Doshi
@ 2022-10-02 19:46 ` Gyan Doshi
2022-10-04 9:11 ` Anton Khirnov
2022-10-12 16:20 ` Anton Khirnov
2022-10-14 13:19 ` Jan Ekström
2 siblings, 1 reply; 8+ messages in thread
From: Gyan Doshi @ 2022-10-02 19:46 UTC (permalink / raw)
To: ffmpeg-devel
On 2022-09-27 10:10 am, Gyan Doshi wrote:
> The current adjustment of input start times just adjusts the tsoffset.
> And it does so, by resetting the tsoffset to nullify the new start time.
> This leads to breakage of -copyts, ignoring of user_ts_offset, breaking
> of -isync as well as breaking wrap correction.
>
> Fixed by taking cognizance of these parameters, and by correcting start
> times just before sync offsets are applied.
Comments?
> ---
> fftools/ffmpeg.c | 24 +-------------------
> fftools/ffmpeg.h | 5 ++++-
> fftools/ffmpeg_demux.c | 4 ++--
> fftools/ffmpeg_opt.c | 50 +++++++++++++++++++++++++++++++++++++-----
> 4 files changed, 52 insertions(+), 31 deletions(-)
>
> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> index 0e1477299d..fabb0fb952 100644
> --- a/fftools/ffmpeg.c
> +++ b/fftools/ffmpeg.c
> @@ -1898,7 +1898,7 @@ static void do_streamcopy(InputStream *ist, OutputStream *ost, const AVPacket *p
> start_time = 0;
> if (copy_ts) {
> start_time += f->start_time != AV_NOPTS_VALUE ? f->start_time : 0;
> - start_time += start_at_zero ? 0 : f->ctx->start_time;
> + start_time += start_at_zero ? 0 : f->enabled_start_time;
> }
> if (ist->pts >= f->recording_time + start_time) {
> close_output_stream(ost);
> @@ -3323,28 +3323,6 @@ static int transcode_init(void)
> input_streams[j + ifile->ist_index]->start = av_gettime_relative();
> }
>
> - // Correct starttime based on the enabled streams
> - for (i = 0; i < nb_input_files; i++) {
> - InputFile *ifile = input_files[i];
> - AVFormatContext *is = ifile->ctx;
> - int64_t new_start_time = INT64_MAX;
> -
> - if (is->start_time == AV_NOPTS_VALUE ||
> - !(is->iformat->flags & AVFMT_TS_DISCONT))
> - continue;
> -
> - for (int j = 0; j < is->nb_streams; j++) {
> - AVStream *st = is->streams[j];
> - if(st->discard == AVDISCARD_ALL || st->start_time == AV_NOPTS_VALUE)
> - continue;
> - new_start_time = FFMIN(new_start_time, av_rescale_q(st->start_time, st->time_base, AV_TIME_BASE_Q));
> - }
> - if (new_start_time > is->start_time) {
> - av_log(is, AV_LOG_VERBOSE, "Correcting start time by %"PRId64"\n", new_start_time - is->start_time);
> - ifile->ts_offset = -new_start_time;
> - }
> - }
> -
> /* init input streams */
> for (i = 0; i < nb_input_streams; i++)
> if ((ret = init_input_stream(i, error, sizeof(error))) < 0)
> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
> index ede0b2bd96..b93c2427b6 100644
> --- a/fftools/ffmpeg.h
> +++ b/fftools/ffmpeg.h
> @@ -439,7 +439,10 @@ typedef struct InputFile {
> AVRational time_base; /* time base of the duration */
> int64_t input_ts_offset;
> int input_sync_ref;
> -
> + /**
> + * Effective format start time based on enabled streams.
> + */
> + int64_t enabled_start_time;
> int64_t ts_offset;
> /**
> * Extra timestamp offset added by discontinuity handling.
> diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
> index 6e89f5999a..6125181f81 100644
> --- a/fftools/ffmpeg_demux.c
> +++ b/fftools/ffmpeg_demux.c
> @@ -120,7 +120,7 @@ static int seek_to_start(InputFile *ifile)
> static void ts_fixup(InputFile *ifile, AVPacket *pkt, int *repeat_pict)
> {
> InputStream *ist = input_streams[ifile->ist_index + pkt->stream_index];
> - const int64_t start_time = ifile->ctx->start_time;
> + const int64_t start_time = ifile->enabled_start_time;
> int64_t duration;
>
> if (debug_ts) {
> @@ -367,7 +367,7 @@ int ifile_get_packet(InputFile *f, AVPacket **pkt)
> if (f->readrate || f->rate_emu) {
> int i;
> int64_t file_start = copy_ts * (
> - (f->ctx->start_time != AV_NOPTS_VALUE ? f->ctx->start_time * !start_at_zero : 0) +
> + (f->enabled_start_time != AV_NOPTS_VALUE ? f->enabled_start_time * !start_at_zero : 0) +
> (f->start_time != AV_NOPTS_VALUE ? f->start_time : 0)
> );
> float scale = f->rate_emu ? 1.0 : f->readrate;
> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
> index 5febe319e4..8df322c31c 100644
> --- a/fftools/ffmpeg_opt.c
> +++ b/fftools/ffmpeg_opt.c
> @@ -293,6 +293,44 @@ static int parse_and_set_vsync(const char *arg, int *vsync_var, int file_idx, in
> return 0;
> }
>
> +/* Correct input file start times based on enabled streams */
> +static int correct_input_start_times(void)
> +{
> + for (int i = 0; i < nb_input_files; i++) {
> + InputFile *ifile = input_files[i];
> + AVFormatContext *is = ifile->ctx;
> + int64_t new_start_time = INT64_MAX, diff, abs_start_seek;
> +
> + ifile->enabled_start_time = is->start_time;
> +
> + if (is->start_time == AV_NOPTS_VALUE ||
> + !(is->iformat->flags & AVFMT_TS_DISCONT))
> + continue;
> +
> + for (int j = 0; j < is->nb_streams; j++) {
> + AVStream *st = is->streams[j];
> + if(st->discard == AVDISCARD_ALL || st->start_time == AV_NOPTS_VALUE)
> + continue;
> + new_start_time = FFMIN(new_start_time, av_rescale_q(st->start_time, st->time_base, AV_TIME_BASE_Q));
> + }
> +
> + diff = new_start_time - is->start_time;
> + if (diff) {
> + av_log(NULL, AV_LOG_VERBOSE, "Correcting start time of Input #%d by %"PRId64" us.\n", i, diff);
> + ifile->enabled_start_time = new_start_time;
> + if (copy_ts && start_at_zero)
> + ifile->ts_offset = -new_start_time;
> + else if (!copy_ts) {
> + abs_start_seek = is->start_time + (ifile->start_time != AV_NOPTS_VALUE) ? ifile->start_time : 0;
> + ifile->ts_offset = abs_start_seek > new_start_time ? -abs_start_seek : -new_start_time;
> + }
> + ifile->ts_offset += ifile->input_ts_offset;
> + }
> + }
> +
> + return 0;
> +}
> +
> static int apply_sync_offsets(void)
> {
> for (int i = 0; i < nb_input_files; i++) {
> @@ -321,9 +359,9 @@ static int apply_sync_offsets(void)
> if (self->ctx->start_time_realtime != AV_NOPTS_VALUE && ref->ctx->start_time_realtime != AV_NOPTS_VALUE) {
> self_start_time = self->ctx->start_time_realtime;
> ref_start_time = ref->ctx->start_time_realtime;
> - } else if (self->ctx->start_time != AV_NOPTS_VALUE && ref->ctx->start_time != AV_NOPTS_VALUE) {
> - self_start_time = self->ctx->start_time;
> - ref_start_time = ref->ctx->start_time;
> + } else if (self->enabled_start_time != AV_NOPTS_VALUE && ref->enabled_start_time != AV_NOPTS_VALUE) {
> + self_start_time = self->enabled_start_time;
> + ref_start_time = ref->enabled_start_time;
> } else {
> start_times_set = 0;
> }
> @@ -3747,8 +3785,6 @@ int ffmpeg_parse_options(int argc, char **argv)
> goto fail;
> }
>
> - apply_sync_offsets();
> -
> /* create the complex filtergraphs */
> ret = init_complex_filters();
> if (ret < 0) {
> @@ -3763,6 +3799,10 @@ int ffmpeg_parse_options(int argc, char **argv)
> goto fail;
> }
>
> + correct_input_start_times();
> +
> + apply_sync_offsets();
> +
> check_filter_outputs();
>
> fail:
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [FFmpeg-devel] [PATCH] ffmpeg: fix implementation of updated input start time
2022-10-02 19:46 ` Gyan Doshi
@ 2022-10-04 9:11 ` Anton Khirnov
2022-10-12 6:35 ` Gyan Doshi
0 siblings, 1 reply; 8+ messages in thread
From: Anton Khirnov @ 2022-10-04 9:11 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Quoting Gyan Doshi (2022-10-02 21:46:59)
>
>
> On 2022-09-27 10:10 am, Gyan Doshi wrote:
> > The current adjustment of input start times just adjusts the tsoffset.
> > And it does so, by resetting the tsoffset to nullify the new start time.
> > This leads to breakage of -copyts, ignoring of user_ts_offset, breaking
> > of -isync as well as breaking wrap correction.
> >
> > Fixed by taking cognizance of these parameters, and by correcting start
> > times just before sync offsets are applied.
>
> Comments?
sorry, was busy
will look at this in a few days
--
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [FFmpeg-devel] [PATCH] ffmpeg: fix implementation of updated input start time
2022-10-04 9:11 ` Anton Khirnov
@ 2022-10-12 6:35 ` Gyan Doshi
0 siblings, 0 replies; 8+ messages in thread
From: Gyan Doshi @ 2022-10-12 6:35 UTC (permalink / raw)
To: ffmpeg-devel
On 2022-10-04 02:41 pm, Anton Khirnov wrote:
> Quoting Gyan Doshi (2022-10-02 21:46:59)
>>
>> On 2022-09-27 10:10 am, Gyan Doshi wrote:
>>> The current adjustment of input start times just adjusts the tsoffset.
>>> And it does so, by resetting the tsoffset to nullify the new start time.
>>> This leads to breakage of -copyts, ignoring of user_ts_offset, breaking
>>> of -isync as well as breaking wrap correction.
>>>
>>> Fixed by taking cognizance of these parameters, and by correcting start
>>> times just before sync offsets are applied.
>> Comments?
> sorry, was busy
>
> will look at this in a few days
Ping.
Regards,
Gyan
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [FFmpeg-devel] [PATCH] ffmpeg: fix implementation of updated input start time
2022-09-27 4:40 [FFmpeg-devel] [PATCH] ffmpeg: fix implementation of updated input start time Gyan Doshi
2022-10-02 19:46 ` Gyan Doshi
@ 2022-10-12 16:20 ` Anton Khirnov
2022-10-14 4:22 ` Gyan Doshi
2022-10-14 13:19 ` Jan Ekström
2 siblings, 1 reply; 8+ messages in thread
From: Anton Khirnov @ 2022-10-12 16:20 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Quoting Gyan Doshi (2022-09-27 06:40:56)
> The current adjustment of input start times just adjusts the tsoffset.
> And it does so, by resetting the tsoffset to nullify the new start time.
> This leads to breakage of -copyts, ignoring of user_ts_offset, breaking
^^^^^^^^^^^^^^
no such variable
> of -isync as well as breaking wrap correction.
Given all these options that are supposed to interact with each other in
highly nonobvious ways, it would be great to have tests for the use
cases you're fixing here.
> Fixed by taking cognizance of these parameters, and by correcting start
> times just before sync offsets are applied.
> ---
> fftools/ffmpeg.c | 24 +-------------------
> fftools/ffmpeg.h | 5 ++++-
> fftools/ffmpeg_demux.c | 4 ++--
> fftools/ffmpeg_opt.c | 50 +++++++++++++++++++++++++++++++++++++-----
> 4 files changed, 52 insertions(+), 31 deletions(-)
>
> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> index 0e1477299d..fabb0fb952 100644
> --- a/fftools/ffmpeg.c
> +++ b/fftools/ffmpeg.c
> @@ -1898,7 +1898,7 @@ static void do_streamcopy(InputStream *ist, OutputStream *ost, const AVPacket *p
> start_time = 0;
> if (copy_ts) {
> start_time += f->start_time != AV_NOPTS_VALUE ? f->start_time : 0;
> - start_time += start_at_zero ? 0 : f->ctx->start_time;
> + start_time += start_at_zero ? 0 : f->enabled_start_time;
> }
> if (ist->pts >= f->recording_time + start_time) {
> close_output_stream(ost);
> @@ -3323,28 +3323,6 @@ static int transcode_init(void)
> input_streams[j + ifile->ist_index]->start = av_gettime_relative();
> }
>
> - // Correct starttime based on the enabled streams
> - for (i = 0; i < nb_input_files; i++) {
> - InputFile *ifile = input_files[i];
> - AVFormatContext *is = ifile->ctx;
> - int64_t new_start_time = INT64_MAX;
> -
> - if (is->start_time == AV_NOPTS_VALUE ||
> - !(is->iformat->flags & AVFMT_TS_DISCONT))
> - continue;
> -
> - for (int j = 0; j < is->nb_streams; j++) {
> - AVStream *st = is->streams[j];
> - if(st->discard == AVDISCARD_ALL || st->start_time == AV_NOPTS_VALUE)
> - continue;
> - new_start_time = FFMIN(new_start_time, av_rescale_q(st->start_time, st->time_base, AV_TIME_BASE_Q));
> - }
> - if (new_start_time > is->start_time) {
> - av_log(is, AV_LOG_VERBOSE, "Correcting start time by %"PRId64"\n", new_start_time - is->start_time);
> - ifile->ts_offset = -new_start_time;
> - }
> - }
> -
The change would be more readable if you first moved this block into a
separate function in a separate patch.
Also, apply_sync_offsets() and correct_input_start_times() both modify
ts_offset in complicated ways. IMO it makes more sense to have both
loops in one function.
> /* init input streams */
> for (i = 0; i < nb_input_streams; i++)
> if ((ret = init_input_stream(i, error, sizeof(error))) < 0)
> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
> index ede0b2bd96..b93c2427b6 100644
> --- a/fftools/ffmpeg.h
> +++ b/fftools/ffmpeg.h
> @@ -439,7 +439,10 @@ typedef struct InputFile {
> AVRational time_base; /* time base of the duration */
> int64_t input_ts_offset;
> int input_sync_ref;
> -
> + /**
> + * Effective format start time based on enabled streams.
> + */
> + int64_t enabled_start_time;
Not a big fan of the name. Maybe start_time_effective would be better.
Or maybe even rename InputFile.start_time into start_time_user and reuse
the start_time name for the new variable.
--
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [FFmpeg-devel] [PATCH] ffmpeg: fix implementation of updated input start time
2022-10-12 16:20 ` Anton Khirnov
@ 2022-10-14 4:22 ` Gyan Doshi
0 siblings, 0 replies; 8+ messages in thread
From: Gyan Doshi @ 2022-10-14 4:22 UTC (permalink / raw)
To: ffmpeg-devel
On 2022-10-12 09:50 pm, Anton Khirnov wrote:
> Quoting Gyan Doshi (2022-09-27 06:40:56)
>> The current adjustment of input start times just adjusts the tsoffset.
>> And it does so, by resetting the tsoffset to nullify the new start time.
>> This leads to breakage of -copyts, ignoring of user_ts_offset, breaking
> ^^^^^^^^^^^^^^
> no such variable
Will correct.
>> of -isync as well as breaking wrap correction.
> Given all these options that are supposed to interact with each other in
> highly nonobvious ways, it would be great to have tests for the use
> cases you're fixing here.
These options don't really interact with each other. They all derive
from or reference the start time.
Since ctx->start_time can't be changed outside lavf, we need to point to
an fftools field. Basically, that's it.
>> Fixed by taking cognizance of these parameters, and by correcting start
>> times just before sync offsets are applied.
>> ---
>> fftools/ffmpeg.c | 24 +-------------------
>> fftools/ffmpeg.h | 5 ++++-
>> fftools/ffmpeg_demux.c | 4 ++--
>> fftools/ffmpeg_opt.c | 50 +++++++++++++++++++++++++++++++++++++-----
>> 4 files changed, 52 insertions(+), 31 deletions(-)
>>
>> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
>> index 0e1477299d..fabb0fb952 100644
>> --- a/fftools/ffmpeg.c
>> +++ b/fftools/ffmpeg.c
>> @@ -1898,7 +1898,7 @@ static void do_streamcopy(InputStream *ist, OutputStream *ost, const AVPacket *p
>> start_time = 0;
>> if (copy_ts) {
>> start_time += f->start_time != AV_NOPTS_VALUE ? f->start_time : 0;
>> - start_time += start_at_zero ? 0 : f->ctx->start_time;
>> + start_time += start_at_zero ? 0 : f->enabled_start_time;
>> }
>> if (ist->pts >= f->recording_time + start_time) {
>> close_output_stream(ost);
>> @@ -3323,28 +3323,6 @@ static int transcode_init(void)
>> input_streams[j + ifile->ist_index]->start = av_gettime_relative();
>> }
>>
>> - // Correct starttime based on the enabled streams
>> - for (i = 0; i < nb_input_files; i++) {
>> - InputFile *ifile = input_files[i];
>> - AVFormatContext *is = ifile->ctx;
>> - int64_t new_start_time = INT64_MAX;
>> -
>> - if (is->start_time == AV_NOPTS_VALUE ||
>> - !(is->iformat->flags & AVFMT_TS_DISCONT))
>> - continue;
>> -
>> - for (int j = 0; j < is->nb_streams; j++) {
>> - AVStream *st = is->streams[j];
>> - if(st->discard == AVDISCARD_ALL || st->start_time == AV_NOPTS_VALUE)
>> - continue;
>> - new_start_time = FFMIN(new_start_time, av_rescale_q(st->start_time, st->time_base, AV_TIME_BASE_Q));
>> - }
>> - if (new_start_time > is->start_time) {
>> - av_log(is, AV_LOG_VERBOSE, "Correcting start time by %"PRId64"\n", new_start_time - is->start_time);
>> - ifile->ts_offset = -new_start_time;
>> - }
>> - }
>> -
> The change would be more readable if you first moved this block into a
> separate function in a separate patch.
Will do.
>
> Also, apply_sync_offsets() and correct_input_start_times() both modify
> ts_offset in complicated ways. IMO it makes more sense to have both
> loops in one function.
I'd prefer a parent container with these two as child functions. I like
the one purpose per function - cleaner for any future changes.
>> /* init input streams */
>> for (i = 0; i < nb_input_streams; i++)
>> if ((ret = init_input_stream(i, error, sizeof(error))) < 0)
>> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
>> index ede0b2bd96..b93c2427b6 100644
>> --- a/fftools/ffmpeg.h
>> +++ b/fftools/ffmpeg.h
>> @@ -439,7 +439,10 @@ typedef struct InputFile {
>> AVRational time_base; /* time base of the duration */
>> int64_t input_ts_offset;
>> int input_sync_ref;
>> -
>> + /**
>> + * Effective format start time based on enabled streams.
>> + */
>> + int64_t enabled_start_time;
> Not a big fan of the name. Maybe start_time_effective would be better.
> Or maybe even rename InputFile.start_time into start_time_user and reuse
> the start_time name for the new variable.
I like start_time_effective . Will do.
Regards,
Gyan
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [FFmpeg-devel] [PATCH] ffmpeg: fix implementation of updated input start time
2022-09-27 4:40 [FFmpeg-devel] [PATCH] ffmpeg: fix implementation of updated input start time Gyan Doshi
2022-10-02 19:46 ` Gyan Doshi
2022-10-12 16:20 ` Anton Khirnov
@ 2022-10-14 13:19 ` Jan Ekström
2022-10-14 13:37 ` Gyan Doshi
2 siblings, 1 reply; 8+ messages in thread
From: Jan Ekström @ 2022-10-14 13:19 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On Tue, Sep 27, 2022 at 7:41 AM Gyan Doshi <ffmpeg@gyani.pro> wrote:
>
> The current adjustment of input start times just adjusts the tsoffset.
> And it does so, by resetting the tsoffset to nullify the new start time.
> This leads to breakage of -copyts, ignoring of user_ts_offset, breaking
> of -isync as well as breaking wrap correction.
>
> Fixed by taking cognizance of these parameters, and by correcting start
> times just before sync offsets are applied.
Out of interest and sorry for not testing this myself: does this
improve the current state of affairs with itsoffset with discontinuous
inputs like MPEG-TS?
Until now the offset was put together with a whole bunch of other
things, which led to logic which was supposed to only start the input
at PTS=0 to also take out the configured itsoffset at the same time
(since both the user defined offset as well as the format start time
offset were both chunked into the same thing).
Cheers,
Jan
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [FFmpeg-devel] [PATCH] ffmpeg: fix implementation of updated input start time
2022-10-14 13:19 ` Jan Ekström
@ 2022-10-14 13:37 ` Gyan Doshi
0 siblings, 0 replies; 8+ messages in thread
From: Gyan Doshi @ 2022-10-14 13:37 UTC (permalink / raw)
To: ffmpeg-devel
On 2022-10-14 06:49 pm, Jan Ekström wrote:
> On Tue, Sep 27, 2022 at 7:41 AM Gyan Doshi <ffmpeg@gyani.pro> wrote:
>> The current adjustment of input start times just adjusts the tsoffset.
>> And it does so, by resetting the tsoffset to nullify the new start time.
>> This leads to breakage of -copyts, ignoring of user_ts_offset, breaking
>> of -isync as well as breaking wrap correction.
>>
>> Fixed by taking cognizance of these parameters, and by correcting start
>> times just before sync offsets are applied.
> Out of interest and sorry for not testing this myself: does this
> improve the current state of affairs with itsoffset with discontinuous
> inputs like MPEG-TS?
I just recompute the tsoffset the same way but with the effective new
start time.
> Until now the offset was put together with a whole bunch of other
> things, which led to logic which was supposed to only start the input
> at PTS=0 to also take out the configured itsoffset at the same time
> (since both the user defined offset as well as the format start time
> offset were both chunked into the same thing).
That's not my experience.
If the itsoffset is 5 and the first packet timestamp is 40, then only
the 40 is negated and the packet is relayed with a ts of 5.
Isn't that what's expected?
Regards,
Gyan
>
> Cheers,
> Jan
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-10-14 13:38 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-27 4:40 [FFmpeg-devel] [PATCH] ffmpeg: fix implementation of updated input start time Gyan Doshi
2022-10-02 19:46 ` Gyan Doshi
2022-10-04 9:11 ` Anton Khirnov
2022-10-12 6:35 ` Gyan Doshi
2022-10-12 16:20 ` Anton Khirnov
2022-10-14 4:22 ` Gyan Doshi
2022-10-14 13:19 ` Jan Ekström
2022-10-14 13:37 ` Gyan Doshi
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git