| Message ID | 20190606203145.22217-2-stephan@ecshi.net |
|---|---|
| State | New |
| Headers | show |
On Thu, Jun 06, 2019 at 10:31:45PM +0200, Stephan Hilb wrote: > If `AV_PKT_FLAG_KEY` stays unset on `pkt->flags`, the output stream > stays empty with little information about what is going on. > This change makes it easier to debug the situation for the user who > could then choose to use the `-copyinkf` option. > --- > fftools/ffmpeg.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c > index 01f04103cf..446439e285 100644 > --- a/fftools/ffmpeg.c > +++ b/fftools/ffmpeg.c > @@ -2006,8 +2006,10 @@ static void do_streamcopy(InputStream *ist, OutputStream *ost, const AVPacket *p > } > > if ((!ost->frame_number && !(pkt->flags & AV_PKT_FLAG_KEY)) && > - !ost->copy_initial_nonkeyframes) > + !ost->copy_initial_nonkeyframes) { > + av_log(NULL, AV_LOG_DEBUG, "skipping initial non-keyframe\n"); This should contain some information so the stream (in case of multiple) can be identified [...]
On Thu, Jun 06, 2019 at 22:31:45 +0200, Stephan Hilb wrote: > If `AV_PKT_FLAG_KEY` stays unset on `pkt->flags`, the output stream > stays empty with little information about what is going on. > This change makes it easier to debug the situation for the user who > could then choose to use the `-copyinkf` option. [...] > - !ost->copy_initial_nonkeyframes) > + !ost->copy_initial_nonkeyframes) { > + av_log(NULL, AV_LOG_DEBUG, "skipping initial non-keyframe\n"); > return; Incorrect indentation. More seriously: I welcome this message. There was a question on ffmpeg-user[1] recently where a user provided a file without any keyframes at all, which could be played by ffplay, but not copy-coded by ffmpeg. I suggest this message should be at INFO level, but appear only once. Unfortunately, the file said user provided is no longer available (and I have deleted my copy). I believe ffprobe also suffers from such files and never displays anyhing with -show_frames. In fact, does ffprobe perhaps always begin from, or at least wait for, the first keyframe? Cheers, Moritz [1] http://ffmpeg.org/pipermail/ffmpeg-user/2019-June/044526.html
>> - !ost->copy_initial_nonkeyframes) >> + !ost->copy_initial_nonkeyframes) { >> + av_log(NULL, AV_LOG_DEBUG, "skipping initial >> non-keyframe\n"); return; > > Incorrect indentation. It's actually the same indentation as in other places in the same file, what would be the correct way then? > I suggest this message should be at INFO level, but appear only once. I'm fine with INFO level. Repeated messages get supressed anyways, does that suffice? > I believe ffprobe also suffers from such files and never displays > anyhing with -show_frames. In fact, does ffprobe perhaps always begin > from, or at least wait for, the first keyframe? Cannot really comment on that. For me it happens when stream copying from v4l2, since the keyframe flag is not being set there, Probing works just fine.
On Fri, Jun 07, 2019 at 22:35:03 +0200, Stephan Hilb wrote: > > Incorrect indentation. > > It's actually the same indentation as in other places in the same file, > what would be the correct way then? Sorry, my mind went flaky, you're corrent. > > I suggest this message should be at INFO level, but appear only once. > > I'm fine with INFO level. Repeated messages get supressed anyways, does > that suffice? If it only appears once, then fine. I couldn't test (see below), so couldn't see whether this comes for every skipped frame. > > I believe ffprobe also suffers from such files and never displays > > anyhing with -show_frames. In fact, does ffprobe perhaps always begin > > from, or at least wait for, the first keyframe? > > Cannot really comment on that. For me it happens when stream copying > from v4l2, since the keyframe flag is not being set there, Probing > works just fine. Okay, I may have a look at ffprobe myself. Thanks for the v4l2 hint, I can use that to produce a sample. Moritz
On Fri, Jun 07, 2019 at 10:35:03PM +0200, Stephan Hilb wrote: > >> - !ost->copy_initial_nonkeyframes) > >> + !ost->copy_initial_nonkeyframes) { > >> + av_log(NULL, AV_LOG_DEBUG, "skipping initial > >> non-keyframe\n"); return; > > > > Incorrect indentation. > > It's actually the same indentation as in other places in the same file, > what would be the correct way then? > > > I suggest this message should be at INFO level, but appear only once. > > I'm fine with INFO level. Repeated messages get supressed anyways, does > that suffice? Repeated messages only get supressed if there is no interspaced message if 2 things generate a message per frame, neither will be supressed [...]
> Repeated messages only get supressed if there is no interspaced > message if 2 things generate a message per frame, neither will be > supressed So should I leave it at DEBUG level or implement a custom log_once? The "cur_dts invalid" debug message is currently being printed at least as often.
On Sun, Jun 09, 2019 at 11:26:14AM +0200, Stephan Hilb wrote: > > Repeated messages only get supressed if there is no interspaced > > message if 2 things generate a message per frame, neither will be > > supressed > > So should I leave it at DEBUG level or implement a custom log_once? i agree that the message might be usefull to be seen by the end user. maybe as this is already discussed, the first could be a more vissible level and the following then debug or lower level > The "cur_dts invalid" debug message is currently being printed at least > as often. yes, this should be investigated and fixed by someone ... [...]
diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c index 01f04103cf..446439e285 100644 --- a/fftools/ffmpeg.c +++ b/fftools/ffmpeg.c @@ -2006,8 +2006,10 @@ static void do_streamcopy(InputStream *ist, OutputStream *ost, const AVPacket *p } if ((!ost->frame_number && !(pkt->flags & AV_PKT_FLAG_KEY)) && - !ost->copy_initial_nonkeyframes) + !ost->copy_initial_nonkeyframes) { + av_log(NULL, AV_LOG_DEBUG, "skipping initial non-keyframe\n"); return; + } if (!ost->frame_number && !ost->copy_prior_start) { int64_t comp_start = start_time;