| Message ID | 20210921123254.1712-1-jamrial@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] ffmpeg_filter: don't try to autorotate frames with hwaccel pixel formats | expand |
| Context | Check | Description |
|---|---|---|
| andriy/configurex86 | warning | Failed to apply patch |
| andriy/configureppc | warning | Failed to apply patch |
> -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of James Almer > Sent: Tuesday, September 21, 2021 5:33 AM > To: ffmpeg-devel@ffmpeg.org > Subject: [FFmpeg-devel] [PATCH] ffmpeg_filter: don't try to autorotate frames with hwaccel pixel formats > > The transpose, rotate, hflip, and vflip filters don't support them. > Fixes ticket #9432. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > I'm surprised nobody tried to decode an mp4 video like those coming from a > cellphone camera using a hwaccel decoder. This would have been noticed much > earlier. > > fftools/ffmpeg_filter.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c > index da0d4faf54..cc3dc33246 100644 > --- a/fftools/ffmpeg_filter.c > +++ b/fftools/ffmpeg_filter.c > @@ -699,6 +699,7 @@ static int configure_input_video_filter(FilterGraph *fg, InputFilter *ifilter, > { > AVFilterContext *last_filter; > const AVFilter *buffer_filt = avfilter_get_by_name("buffer"); > + const AVPixFmtDescriptor *desc; > InputStream *ist = ifilter->ist; > InputFile *f = input_files[ist->file_index]; > AVRational tb = ist->framerate.num ? av_inv_q(ist->framerate) : > @@ -756,7 +757,9 @@ static int configure_input_video_filter(FilterGraph *fg, InputFilter *ifilter, > av_freep(&par); > last_filter = ifilter->filter; > > - if (ist->autorotate) { > + desc = av_pix_fmt_desc_get(ifilter->format); > + // TODO: insert hwaccel enabled filters like transpose_vaapi into the graph > + if (ist->autorotate && desc && !(desc->flags & AV_PIX_FMT_FLAG_HWACCEL)) { > int32_t *displaymatrix = ifilter->displaymatrix; > double theta; > > -- > 2.33.0 > LGTM and verified WFM with vaapi hwaccel. > _______________________________________________ > 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".
James Almer (12021-09-21): > The transpose, rotate, hflip, and vflip filters don't support them. > Fixes ticket #9432. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > I'm surprised nobody tried to decode an mp4 video like those coming from a > cellphone camera using a hwaccel decoder. This would have been noticed much > earlier. > > fftools/ffmpeg_filter.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c > index da0d4faf54..cc3dc33246 100644 > --- a/fftools/ffmpeg_filter.c > +++ b/fftools/ffmpeg_filter.c > @@ -699,6 +699,7 @@ static int configure_input_video_filter(FilterGraph *fg, InputFilter *ifilter, > { > AVFilterContext *last_filter; > const AVFilter *buffer_filt = avfilter_get_by_name("buffer"); > + const AVPixFmtDescriptor *desc; > InputStream *ist = ifilter->ist; > InputFile *f = input_files[ist->file_index]; > AVRational tb = ist->framerate.num ? av_inv_q(ist->framerate) : > @@ -756,7 +757,9 @@ static int configure_input_video_filter(FilterGraph *fg, InputFilter *ifilter, > av_freep(&par); > last_filter = ifilter->filter; > > - if (ist->autorotate) { > + desc = av_pix_fmt_desc_get(ifilter->format); > + // TODO: insert hwaccel enabled filters like transpose_vaapi into the graph > + if (ist->autorotate && desc && !(desc->flags & AV_PIX_FMT_FLAG_HWACCEL)) { I am rather unsure about the "&& desc". buffersink requires a valid pixel format. If the code arrives there without one, either there is a bug somewhere or there should be a clearer failure earlier. So I think "av_assert0(desc);" would be more adequate. > int32_t *displaymatrix = ifilter->displaymatrix; > double theta; > Regards,
On 9/21/2021 10:07 AM, Nicolas George wrote: > James Almer (12021-09-21): >> The transpose, rotate, hflip, and vflip filters don't support them. >> Fixes ticket #9432. >> >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> I'm surprised nobody tried to decode an mp4 video like those coming from a >> cellphone camera using a hwaccel decoder. This would have been noticed much >> earlier. >> >> fftools/ffmpeg_filter.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c >> index da0d4faf54..cc3dc33246 100644 >> --- a/fftools/ffmpeg_filter.c >> +++ b/fftools/ffmpeg_filter.c >> @@ -699,6 +699,7 @@ static int configure_input_video_filter(FilterGraph *fg, InputFilter *ifilter, >> { >> AVFilterContext *last_filter; >> const AVFilter *buffer_filt = avfilter_get_by_name("buffer"); >> + const AVPixFmtDescriptor *desc; >> InputStream *ist = ifilter->ist; >> InputFile *f = input_files[ist->file_index]; >> AVRational tb = ist->framerate.num ? av_inv_q(ist->framerate) : >> @@ -756,7 +757,9 @@ static int configure_input_video_filter(FilterGraph *fg, InputFilter *ifilter, >> av_freep(&par); >> last_filter = ifilter->filter; >> >> - if (ist->autorotate) { > >> + desc = av_pix_fmt_desc_get(ifilter->format); >> + // TODO: insert hwaccel enabled filters like transpose_vaapi into the graph >> + if (ist->autorotate && desc && !(desc->flags & AV_PIX_FMT_FLAG_HWACCEL)) { > > I am rather unsure about the "&& desc". buffersink requires a valid > pixel format. If the code arrives there without one, either there is a > bug somewhere or there should be a clearer failure earlier. > > So I think "av_assert0(desc);" would be more adequate. Will apply with that change. Thanks. > >> int32_t *displaymatrix = ifilter->displaymatrix; >> double theta; >> > > Regards, > > > _______________________________________________ > 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 Sep 21, 2021, at 8:32 PM, James Almer <jamrial@gmail.com> wrote: > > The transpose, rotate, hflip, and vflip filters don't support them. > Fixes ticket #9432. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > I'm surprised nobody tried to decode an mp4 video like those coming from a > cellphone camera using a hwaccel decoder. This would have been noticed much > earlier. Actually before cf1746d7 "ffmpeg_videotoolbox: skip memory copy if hwaccel_output_format match”, autorotate works for videotoolbox in an inefficiency way. I thought it was by design. > > fftools/ffmpeg_filter.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c > index da0d4faf54..cc3dc33246 100644 > --- a/fftools/ffmpeg_filter.c > +++ b/fftools/ffmpeg_filter.c > @@ -699,6 +699,7 @@ static int configure_input_video_filter(FilterGraph *fg, InputFilter *ifilter, > { > AVFilterContext *last_filter; > const AVFilter *buffer_filt = avfilter_get_by_name("buffer"); > + const AVPixFmtDescriptor *desc; > InputStream *ist = ifilter->ist; > InputFile *f = input_files[ist->file_index]; > AVRational tb = ist->framerate.num ? av_inv_q(ist->framerate) : > @@ -756,7 +757,9 @@ static int configure_input_video_filter(FilterGraph *fg, InputFilter *ifilter, > av_freep(&par); > last_filter = ifilter->filter; > > - if (ist->autorotate) { > + desc = av_pix_fmt_desc_get(ifilter->format); > + // TODO: insert hwaccel enabled filters like transpose_vaapi into the graph > + if (ist->autorotate && desc && !(desc->flags & AV_PIX_FMT_FLAG_HWACCEL)) { > int32_t *displaymatrix = ifilter->displaymatrix; > double theta; > > -- > 2.33.0 > > _______________________________________________ > 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". >
> -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of James Almer > Sent: Tuesday, 21 September 2021 14:33 > To: ffmpeg-devel@ffmpeg.org > Subject: [FFmpeg-devel] [PATCH] ffmpeg_filter: don't try to autorotate frames > with hwaccel pixel formats > > The transpose, rotate, hflip, and vflip filters don't support them. > Fixes ticket #9432. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > I'm surprised nobody tried to decode an mp4 video like those coming from a > cellphone camera using a hwaccel decoder. This would have been noticed much > earlier. I'm aware of this issue, but I think it would be better to auto-insert the corresponding hw rotation filters, as a user should be able to expect the same result, no matter whether using sw or hw filtering. It is already possible to specify -autorotate 0 to prevent autorotation, which applies to all cases equally. softworkz
diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c index da0d4faf54..cc3dc33246 100644 --- a/fftools/ffmpeg_filter.c +++ b/fftools/ffmpeg_filter.c @@ -699,6 +699,7 @@ static int configure_input_video_filter(FilterGraph *fg, InputFilter *ifilter, { AVFilterContext *last_filter; const AVFilter *buffer_filt = avfilter_get_by_name("buffer"); + const AVPixFmtDescriptor *desc; InputStream *ist = ifilter->ist; InputFile *f = input_files[ist->file_index]; AVRational tb = ist->framerate.num ? av_inv_q(ist->framerate) : @@ -756,7 +757,9 @@ static int configure_input_video_filter(FilterGraph *fg, InputFilter *ifilter, av_freep(&par); last_filter = ifilter->filter; - if (ist->autorotate) { + desc = av_pix_fmt_desc_get(ifilter->format); + // TODO: insert hwaccel enabled filters like transpose_vaapi into the graph + if (ist->autorotate && desc && !(desc->flags & AV_PIX_FMT_FLAG_HWACCEL)) { int32_t *displaymatrix = ifilter->displaymatrix; double theta;
The transpose, rotate, hflip, and vflip filters don't support them. Fixes ticket #9432. Signed-off-by: James Almer <jamrial@gmail.com> --- I'm surprised nobody tried to decode an mp4 video like those coming from a cellphone camera using a hwaccel decoder. This would have been noticed much earlier. fftools/ffmpeg_filter.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)