| Message ID | 20200514210349.12825-2-cus@passwd.hu |
|---|---|
| State | Accepted |
| Commit | b4bcae4e0e9afee734b7d9d80fd350bc27d88cea |
| Headers | show |
| Series | [FFmpeg-devel,1/2] Revert "avfilter/vf_minterpolate: if metadata lavfi.scd.mafd exists, we'll use it first" | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
Am Do., 14. Mai 2020 um 23:04 Uhr schrieb Marton Balint <cus@passwd.hu>: > > This reverts commit 339593ca90cb3e05d659ec99a1479904ec742294. > > Fixes null pointer dereference. I did not look into the patches, and maybe you could add how the null pointer dereferences are reproducible but if these patches fix crashes, they should be pushed, other fixes can still be discussed. Carl Eugen
On Thu, 14 May 2020, Carl Eugen Hoyos wrote: > Am Do., 14. Mai 2020 um 23:04 Uhr schrieb Marton Balint <cus@passwd.hu>: >> >> This reverts commit 339593ca90cb3e05d659ec99a1479904ec742294. >> >> Fixes null pointer dereference. > > I did not look into the patches, and maybe you could add how the > null pointer dereferences are reproducible but if these patches fix > crashes, they should be pushed, other fixes can still be discussed. Actual crashes depends on compiler and compiler options. I reproduced it with -O0, but if you take a look at FATE, you will see that there are multiple failures. Regards, Marton
On Thu, May 14, 2020 at 11:51:29PM +0200, Marton Balint wrote: > > > On Thu, 14 May 2020, Carl Eugen Hoyos wrote: > > > Am Do., 14. Mai 2020 um 23:04 Uhr schrieb Marton Balint <cus@passwd.hu>: > > > > > > This reverts commit 339593ca90cb3e05d659ec99a1479904ec742294. > > > > > > Fixes null pointer dereference. > > > > I did not look into the patches, and maybe you could add how the > > null pointer dereferences are reproducible but if these patches fix > > crashes, they should be pushed, other fixes can still be discussed. > > Actual crashes depends on compiler and compiler options. I reproduced it > with -O0, but if you take a look at FATE, you will see that there are > multiple failures. Sorry, LGTM, I think I have run make fate pass on both Mac and Linux x86 system. By the old email, this patchset have give comments and reviewed so I push it after 12hour. I'm not clear about the rules, please update the developer document clearly. In fact, I'm glad to waiting your request time if anybody request, just say "hold on". > > Regards, > Marton > _______________________________________________ > 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".
On Thu, May 14, 2020 at 11:03:49PM +0200, Marton Balint wrote: > This reverts commit 339593ca90cb3e05d659ec99a1479904ec742294. > > Fixes null pointer dereference. > > Signed-off-by: Marton Balint <cus@passwd.hu> > --- > libavfilter/vf_framerate.c | 15 ++++----------- > 1 file changed, 4 insertions(+), 11 deletions(-) > > diff --git a/libavfilter/vf_framerate.c b/libavfilter/vf_framerate.c > index 8d16998457..6c8d01c94b 100644 > --- a/libavfilter/vf_framerate.c > +++ b/libavfilter/vf_framerate.c > @@ -71,20 +71,13 @@ static double get_scene_score(AVFilterContext *ctx, AVFrame *crnt, AVFrame *next > > if (crnt->height == next->height && > crnt->width == next->width) { > - AVDictionaryEntry *e_mafd = NULL; > uint64_t sad; > - double mafd = HUGE_VAL, diff; > - char *tail = NULL; > + double mafd, diff; > > ff_dlog(ctx, "get_scene_score() process\n"); > - e_mafd = av_dict_get(next->metadata, "lavfi.scd.mafd", NULL, AV_DICT_MATCH_CASE); > - if (e_mafd) > - mafd = strtod(e_mafd->value, &tail); just have time to look at the issue, I prefer to add one extra checking for !tail to fix it, it's better than revert. I'll post patch for the fixes. > - if (*tail || mafd == HUGE_VAL) { > - s->sad(crnt->data[0], crnt->linesize[0], next->data[0], next->linesize[0], crnt->width, crnt->height, &sad); > - emms_c(); > - mafd = (double)sad * 100.0 / (crnt->width * crnt->height) / (1 << s->bitdepth); > - } > + s->sad(crnt->data[0], crnt->linesize[0], next->data[0], next->linesize[0], crnt->width, crnt->height, &sad); > + emms_c(); > + mafd = (double)sad * 100.0 / (crnt->width * crnt->height) / (1 << s->bitdepth); > diff = fabs(mafd - s->prev_mafd); > ret = av_clipf(FFMIN(mafd, diff), 0, 100.0); > s->prev_mafd = mafd; > -- > 2.16.4 > > _______________________________________________ > 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".
On Fri, 15 May 2020, Limin Wang wrote: > On Thu, May 14, 2020 at 11:03:49PM +0200, Marton Balint wrote: >> This reverts commit 339593ca90cb3e05d659ec99a1479904ec742294. >> >> Fixes null pointer dereference. > >> >> Signed-off-by: Marton Balint <cus@passwd.hu> >> --- >> libavfilter/vf_framerate.c | 15 ++++----------- >> 1 file changed, 4 insertions(+), 11 deletions(-) >> >> diff --git a/libavfilter/vf_framerate.c b/libavfilter/vf_framerate.c >> index 8d16998457..6c8d01c94b 100644 >> --- a/libavfilter/vf_framerate.c >> +++ b/libavfilter/vf_framerate.c >> @@ -71,20 +71,13 @@ static double get_scene_score(AVFilterContext *ctx, AVFrame *crnt, AVFrame *next >> >> if (crnt->height == next->height && >> crnt->width == next->width) { >> - AVDictionaryEntry *e_mafd = NULL; >> uint64_t sad; >> - double mafd = HUGE_VAL, diff; >> - char *tail = NULL; >> + double mafd, diff; >> >> ff_dlog(ctx, "get_scene_score() process\n"); >> - e_mafd = av_dict_get(next->metadata, "lavfi.scd.mafd", NULL, AV_DICT_MATCH_CASE); >> - if (e_mafd) >> - mafd = strtod(e_mafd->value, &tail); > > just have time to look at the issue, I prefer to add one extra checking for !tail to fix > it, it's better than revert. I'll post patch for the fixes. Applied the revert patches for now, because it seems there are other changes requested. Also the documentation should also mention that scene change detection depends on metadata. Regards, Marton
diff --git a/libavfilter/vf_framerate.c b/libavfilter/vf_framerate.c index 8d16998457..6c8d01c94b 100644 --- a/libavfilter/vf_framerate.c +++ b/libavfilter/vf_framerate.c @@ -71,20 +71,13 @@ static double get_scene_score(AVFilterContext *ctx, AVFrame *crnt, AVFrame *next if (crnt->height == next->height && crnt->width == next->width) { - AVDictionaryEntry *e_mafd = NULL; uint64_t sad; - double mafd = HUGE_VAL, diff; - char *tail = NULL; + double mafd, diff; ff_dlog(ctx, "get_scene_score() process\n"); - e_mafd = av_dict_get(next->metadata, "lavfi.scd.mafd", NULL, AV_DICT_MATCH_CASE); - if (e_mafd) - mafd = strtod(e_mafd->value, &tail); - if (*tail || mafd == HUGE_VAL) { - s->sad(crnt->data[0], crnt->linesize[0], next->data[0], next->linesize[0], crnt->width, crnt->height, &sad); - emms_c(); - mafd = (double)sad * 100.0 / (crnt->width * crnt->height) / (1 << s->bitdepth); - } + s->sad(crnt->data[0], crnt->linesize[0], next->data[0], next->linesize[0], crnt->width, crnt->height, &sad); + emms_c(); + mafd = (double)sad * 100.0 / (crnt->width * crnt->height) / (1 << s->bitdepth); diff = fabs(mafd - s->prev_mafd); ret = av_clipf(FFMIN(mafd, diff), 0, 100.0); s->prev_mafd = mafd;
This reverts commit 339593ca90cb3e05d659ec99a1479904ec742294. Fixes null pointer dereference. Signed-off-by: Marton Balint <cus@passwd.hu> --- libavfilter/vf_framerate.c | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-)