From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 4484143DDA for ; Wed, 12 Oct 2022 16:21:04 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 8DD9968BCE9; Wed, 12 Oct 2022 19:21:01 +0300 (EEST) Received: from mail0.khirnov.net (red.khirnov.net [176.97.15.12]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id B867068BBDB for ; Wed, 12 Oct 2022 19:20:54 +0300 (EEST) Received: from localhost (localhost [IPv6:::1]) by mail0.khirnov.net (Postfix) with ESMTP id 5E1C32404E4 for ; Wed, 12 Oct 2022 18:20:53 +0200 (CEST) Received: from mail0.khirnov.net ([IPv6:::1]) by localhost (mail0.khirnov.net [IPv6:::1]) (amavisd-new, port 10024) with ESMTP id TdUD7D7XGKWO for ; Wed, 12 Oct 2022 18:20:52 +0200 (CEST) Received: from lain.khirnov.net (lain.khirnov.net [IPv6:2001:67c:1138:4306::3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "lain.khirnov.net", Issuer "smtp.khirnov.net SMTP CA" (verified OK)) by mail0.khirnov.net (Postfix) with ESMTPS id A8B7D2400F4 for ; Wed, 12 Oct 2022 18:20:52 +0200 (CEST) Received: by lain.khirnov.net (Postfix, from userid 1000) id 0767B1601B2; Wed, 12 Oct 2022 18:20:50 +0200 (CEST) From: Anton Khirnov To: FFmpeg development discussions and patches In-Reply-To: <20220927044056.2154-1-ffmpeg@gyani.pro> References: <20220927044056.2154-1-ffmpeg@gyani.pro> Mail-Followup-To: FFmpeg development discussions and patches Date: Wed, 12 Oct 2022 18:20:49 +0200 Message-ID: <166559164999.12287.9666902488178933378@lain.khirnov.net> User-Agent: alot/0.8.1 MIME-Version: 1.0 Subject: Re: [FFmpeg-devel] [PATCH] ffmpeg: fix implementation of updated input start time X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: 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".