| Message ID | 20240110090547.69151-1-ffmpeg@haasn.xyz |
|---|---|
| State | Accepted |
| Commit | dcc7263b0e7935b6864e43465cfeca423bdf9e77 |
| Headers | show |
| Series | [FFmpeg-devel,1/2] fftools/ffplay: add missing YUV metadata to buffersrc | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
Quoting Niklas Haas (2024-01-10 10:05:46) > From: Niklas Haas <git@haasn.dev> > > Fixes error spam from the `ffplay` tool since commit 2d555dc82d, caused > by an oversight on my part - I didn't notice during development that > `ffplay` goes through its own filtering code path separate from > fftools/ffmpeg_filter.c Wouldn't the same issue affect any other caller?
> On Jan 26, 2024, at 17:36, Anton Khirnov <anton@khirnov.net> wrote: > > Quoting Niklas Haas (2024-01-10 10:05:46) >> From: Niklas Haas <git@haasn.dev> >> >> Fixes error spam from the `ffplay` tool since commit 2d555dc82d, caused >> by an oversight on my part - I didn't notice during development that >> `ffplay` goes through its own filtering code path separate from >> fftools/ffmpeg_filter.c > > Wouldn't the same issue affect any other caller? Firstly, can avfilter support color range/space change from unspecified to a specified value on the first frame? I think it should silent the log most of the cases. Secondly, log once for non-serious case. > > -- > 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".
On Wed, 10 Jan 2024 10:05:46 +0100 Niklas Haas <ffmpeg@haasn.xyz> wrote: > From: Niklas Haas <git@haasn.dev> > > Fixes error spam from the `ffplay` tool since commit 2d555dc82d, caused > by an oversight on my part - I didn't notice during development that > `ffplay` goes through its own filtering code path separate from > fftools/ffmpeg_filter.c > --- > fftools/ffplay.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/fftools/ffplay.c b/fftools/ffplay.c > index 17861e60be..0771326d76 100644 > --- a/fftools/ffplay.c > +++ b/fftools/ffplay.c > @@ -1881,10 +1881,12 @@ static int configure_video_filters(AVFilterGraph *graph, VideoState *is, const c > graph->scale_sws_opts = av_strdup(sws_flags_str); > > snprintf(buffersrc_args, sizeof(buffersrc_args), > - "video_size=%dx%d:pix_fmt=%d:time_base=%d/%d:pixel_aspect=%d/%d", > + "video_size=%dx%d:pix_fmt=%d:time_base=%d/%d:pixel_aspect=%d/%d:" > + "colorspace=%d:range=%d", > frame->width, frame->height, frame->format, > is->video_st->time_base.num, is->video_st->time_base.den, > - codecpar->sample_aspect_ratio.num, FFMAX(codecpar->sample_aspect_ratio.den, 1)); > + codecpar->sample_aspect_ratio.num, FFMAX(codecpar->sample_aspect_ratio.den, 1), > + frame->colorspace, frame->color_range); > if (fr.num && fr.den) > av_strlcatf(buffersrc_args, sizeof(buffersrc_args), ":frame_rate=%d/%d", fr.num, fr.den); > > -- > 2.43.0 > Will merge tomorrow if there is no further objection, especially as it now affects https://trac.ffmpeg.org/ticket/10839
On Fri, 26 Jan 2024 18:17:30 +0800 Zhao Zhili <quinkblack@foxmail.com> wrote: > > > > On Jan 26, 2024, at 17:36, Anton Khirnov <anton@khirnov.net> wrote: > > > > Quoting Niklas Haas (2024-01-10 10:05:46) > >> From: Niklas Haas <git@haasn.dev> > >> > >> Fixes error spam from the `ffplay` tool since commit 2d555dc82d, caused > >> by an oversight on my part - I didn't notice during development that > >> `ffplay` goes through its own filtering code path separate from > >> fftools/ffmpeg_filter.c > > > > Wouldn't the same issue affect any other caller? > > Firstly, can avfilter support color range/space change from unspecified to a specified value > on the first frame? I think it should silent the log most of the cases. > > Secondly, log once for non-serious case. To clarify, commit 94422871fce3b90bebc95f5cae939fbbc4e33224 makes this message drop from WARNING to DEBUG verbosity for subsequent print-outs. So the error spam is no longer a major consideration, however it remains that ffplay needs to set the correct metadata for YUV colorspace negotiation to work inside the ffplay filter graph. (See the issue I linked in my other mail)
diff --git a/fftools/ffplay.c b/fftools/ffplay.c index 17861e60be..0771326d76 100644 --- a/fftools/ffplay.c +++ b/fftools/ffplay.c @@ -1881,10 +1881,12 @@ static int configure_video_filters(AVFilterGraph *graph, VideoState *is, const c graph->scale_sws_opts = av_strdup(sws_flags_str); snprintf(buffersrc_args, sizeof(buffersrc_args), - "video_size=%dx%d:pix_fmt=%d:time_base=%d/%d:pixel_aspect=%d/%d", + "video_size=%dx%d:pix_fmt=%d:time_base=%d/%d:pixel_aspect=%d/%d:" + "colorspace=%d:range=%d", frame->width, frame->height, frame->format, is->video_st->time_base.num, is->video_st->time_base.den, - codecpar->sample_aspect_ratio.num, FFMAX(codecpar->sample_aspect_ratio.den, 1)); + codecpar->sample_aspect_ratio.num, FFMAX(codecpar->sample_aspect_ratio.den, 1), + frame->colorspace, frame->color_range); if (fr.num && fr.den) av_strlcatf(buffersrc_args, sizeof(buffersrc_args), ":frame_rate=%d/%d", fr.num, fr.den);
From: Niklas Haas <git@haasn.dev> Fixes error spam from the `ffplay` tool since commit 2d555dc82d, caused by an oversight on my part - I didn't notice during development that `ffplay` goes through its own filtering code path separate from fftools/ffmpeg_filter.c --- fftools/ffplay.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)