| Message ID | 63a22eb9-ce4a-c204-19fe-4704e03bbeb6@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
> Am 05.08.2018 um 09:10 schrieb Gyan Doshi <gyandoshi@gmail.com>: > > <0001-ffmpeg-correct-units-for-raw-pts-in-progress-report.patch> > av_bprintf(&buf_script, "out_time_ms=N/A\n"); If the patch is a good idea you should also change this line. Thank you, Carl Eugen
On 05-08-2018 01:46 PM, Carl Eugen Hoyos wrote: > >> av_bprintf(&buf_script, "out_time_ms=N/A\n"); > > If the patch is a good idea you should also change this line. Fixed locally. Will push soon, if nothing else. Thanks, Gyan
Gyan Doshi (2018-08-05):
> Will push soon, if nothing else.
Have you considered you may be breaking some user's scripts?
Carl Eugen's comment was in no way an approval of your patch. Pushing a
patch without a second positive opinion in less than half a day...
Regards,
On 05-08-2018 02:17 PM, Nicolas George wrote: > Gyan Doshi (2018-08-05): >> Will push soon, if nothing else. > > Have you considered you may be breaking some user's scripts? Ah yes, Sorry, this is used for parsing. What protocol do you suggest for correcting this? Regards, Gyan
On 05-08-2018 02:28 PM, Gyan Doshi wrote: > Ah yes, Sorry, this is used for parsing. What protocol do you suggest > for correcting this? Any thoughts on this? I see that there were two changes made to value fields of the progress report earlier this year - a194e9c4159 and 1322b00060 Regards, Gyan
Gyan Doshi (2018-08-06): > >Ah yes, Sorry, this is used for parsing. What protocol do you suggest for > >correcting this? > > Any thoughts on this? Not really. I suppose you could add the field with the correct name and remove the bogus one later. But your thoughts on this are probably better than mine. > I see that there were two changes made to value fields of the progress > report earlier this year - a194e9c4159 and 1322b00060 It is true, but past mistakes cannot be used to justify new ones. Also, it seems to me theses changes are much less serious: 1322b00060 only changes the precision in a compatible way, while the second only changes anything when the required value is unavailable. Regards,
On 06-08-2018 08:00 PM, Nicolas George wrote: > Not really. I suppose you could add the field with the correct name and > remove the bogus one later. But your thoughts on this are probably > better than mine. That will just defer the breaking change. The options, as I see it, are wontfix, or push with a easily grepped intuitive commit msg for those who like to use nightly builds and want to know why their script is (partially) broken. Best done at the time of a new release. Regards, Gyan
Gyan Doshi (2018-08-08):
> That will just defer the breaking change.
That will leave people time to notice the change and allow old and new
scripts to work during the transition.
Regards,
On 08-08-2018 12:47 AM, Nicolas George wrote: > Gyan Doshi (2018-08-08): >> That will just defer the breaking change. > > That will leave people time to notice the change and allow old and new > scripts to work during the transition. Will do it this way. Gyan
diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c index 55faec8ede..2bc550dfe8 100644 --- a/fftools/ffmpeg.c +++ b/fftools/ffmpeg.c @@ -1797,7 +1797,7 @@ static void print_report(int is_last_report, int64_t timer_start, int64_t cur_ti av_bprintf(&buf_script, "out_time_ms=N/A\n"); av_bprintf(&buf_script, "out_time=N/A\n"); } else { - av_bprintf(&buf_script, "out_time_ms=%"PRId64"\n", pts); + av_bprintf(&buf_script, "out_time_us=%"PRId64"\n", pts); av_bprintf(&buf_script, "out_time=%s%02d:%02d:%02d.%06d\n", hours_sign, hours, mins, secs, us); }
From a45b0ade691816d8037c846f2b773d0ddab74cbe Mon Sep 17 00:00:00 2001 From: Gyan Doshi <ffmpeg@gyani.pro> Date: Sun, 5 Aug 2018 12:34:21 +0530 Subject: [PATCH] ffmpeg: correct units for raw pts in -progress report PTS is in microseconds. Fixes #7345 --- fftools/ffmpeg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)