| Message ID | 20210124130033.1086545-1-linjie.justin.fu@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] fftools/ffmpeg_opt: restore to use codec_info_nb_frames to calculate score | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | success | Make fate finished |
On 1/24/2021 10:00 AM, Linjie Fu wrote: > From: Linjie Fu <linjie.fulinjie@alibaba-inc.com> > > Regression since 87f0c8280. > > If the extradata of a stream could not be extracted correctly, > codec_info_nb_frames would remain zero, while st->event_flag would still > be marked as AVSTREAM_EVENT_FLAG_NEW_PACKETS since it's a new packet > indeed. > > The two expressions could be different in this case, hence restore to > calculate the correct score. > > Fix #9029. codec_info_nb_frames is not public, which is why its usage was removed, so this patch is not ok. A proper fix would perhaps be to reset event_flags after this, as its API says it should. Can you try that and see if it works? > > Signed-off-by: Linjie Fu <linjie.justin.fu@gmail.com> > --- > fftools/ffmpeg_opt.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c > index bf2eb26246..ba813ea7bc 100644 > --- a/fftools/ffmpeg_opt.c > +++ b/fftools/ffmpeg_opt.c > @@ -2228,8 +2228,7 @@ static int open_output_file(OptionsContext *o, const char *filename) > for (i = 0; i < nb_input_streams; i++) { > int score; > ist = input_streams[i]; > - score = ist->st->codecpar->width * ist->st->codecpar->height > - + 100000000 * !!(ist->st->event_flags & AVSTREAM_EVENT_FLAG_NEW_PACKETS) > + score = ist->st->codecpar->width * ist->st->codecpar->height + 100000000*!!ist->st->codec_info_nb_frames > + 5000000*!!(ist->st->disposition & AV_DISPOSITION_DEFAULT); > if (ist->user_set_discard == AVDISCARD_ALL) > continue; >
On Sun, Jan 24, 2021 at 9:38 PM James Almer <jamrial@gmail.com> wrote: > > On 1/24/2021 10:00 AM, Linjie Fu wrote: > > From: Linjie Fu <linjie.fulinjie@alibaba-inc.com> > > > > Regression since 87f0c8280. > > > > If the extradata of a stream could not be extracted correctly, > > codec_info_nb_frames would remain zero, while st->event_flag would still > > be marked as AVSTREAM_EVENT_FLAG_NEW_PACKETS since it's a new packet > > indeed. > > > > The two expressions could be different in this case, hence restore to > > calculate the correct score. > > > > Fix #9029. > > codec_info_nb_frames is not public, which is why its usage was removed, > so this patch is not ok. Okay, thx. > A proper fix would perhaps be to reset event_flags after this, as its > API says it should. Can you try that and see if it works? > Yes it works, and I've tried this before I sent this patch out. However, what makes me hesitant is the definition of AVSTREAM_EVENT_FLAG_NEW_PACKETS , which says this flag indicates "new packets for this stream were read from the file". IIRC the behaviour in this case matches the definition well, hence I didn't reset event_flags to zero at first. Will send it out for discussion as well, thx. - linjie
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c index bf2eb26246..ba813ea7bc 100644 --- a/fftools/ffmpeg_opt.c +++ b/fftools/ffmpeg_opt.c @@ -2228,8 +2228,7 @@ static int open_output_file(OptionsContext *o, const char *filename) for (i = 0; i < nb_input_streams; i++) { int score; ist = input_streams[i]; - score = ist->st->codecpar->width * ist->st->codecpar->height - + 100000000 * !!(ist->st->event_flags & AVSTREAM_EVENT_FLAG_NEW_PACKETS) + score = ist->st->codecpar->width * ist->st->codecpar->height + 100000000*!!ist->st->codec_info_nb_frames + 5000000*!!(ist->st->disposition & AV_DISPOSITION_DEFAULT); if (ist->user_set_discard == AVDISCARD_ALL) continue;