* [FFmpeg-devel] [PATCH] av1dec: handle dimension changes via get_format
@ 2023-06-20 23:36 airlied
2023-06-21 16:34 ` Leo Izen
2023-06-21 16:36 ` James Almer
0 siblings, 2 replies; 7+ messages in thread
From: airlied @ 2023-06-20 23:36 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Dave Airlie
From: Dave Airlie <airlied@redhat.com>
av1-1-b8-03-sizeup.ivf on vulkan causes gpu hangs as none of the
images get resized when dimensions change, this detects the dim
change and calls the get_format to reinit the context.
---
libavcodec/av1dec.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
index e7f98a6c81..1cec328563 100644
--- a/libavcodec/av1dec.c
+++ b/libavcodec/av1dec.c
@@ -721,6 +721,7 @@ static av_cold int av1_decode_free(AVCodecContext *avctx)
}
static int set_context_with_sequence(AVCodecContext *avctx,
+ int *dim_change,
const AV1RawSequenceHeader *seq)
{
int width = seq->max_frame_width_minus_1 + 1;
@@ -753,6 +754,8 @@ static int set_context_with_sequence(AVCodecContext *avctx,
int ret = ff_set_dimensions(avctx, width, height);
if (ret < 0)
return ret;
+ if (dim_change)
+ *dim_change = 1;
}
avctx->sample_aspect_ratio = (AVRational) { 1, 1 };
@@ -859,7 +862,7 @@ static av_cold int av1_decode_init(AVCodecContext *avctx)
goto end;
}
- ret = set_context_with_sequence(avctx, seq);
+ ret = set_context_with_sequence(avctx, NULL, seq);
if (ret < 0) {
av_log(avctx, AV_LOG_WARNING, "Failed to set decoder context.\n");
goto end;
@@ -1202,7 +1205,7 @@ static int av1_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame)
CodedBitstreamUnit *unit = &s->current_obu.units[i];
AV1RawOBU *obu = unit->content;
const AV1RawOBUHeader *header;
-
+ int dim_change = 0;
if (!obu)
continue;
@@ -1220,7 +1223,8 @@ static int av1_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame)
s->raw_seq = &obu->obu.sequence_header;
- ret = set_context_with_sequence(avctx, s->raw_seq);
+ dim_change = 0;
+ ret = set_context_with_sequence(avctx, &dim_change, s->raw_seq);
if (ret < 0) {
av_log(avctx, AV_LOG_ERROR, "Failed to set context.\n");
s->raw_seq = NULL;
@@ -1229,7 +1233,7 @@ static int av1_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame)
s->operating_point_idc = s->raw_seq->operating_point_idc[s->operating_point];
- if (s->pix_fmt == AV_PIX_FMT_NONE) {
+ if (s->pix_fmt == AV_PIX_FMT_NONE || dim_change) {
ret = get_pixel_format(avctx);
if (ret < 0) {
av_log(avctx, AV_LOG_ERROR,
--
2.41.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".
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [FFmpeg-devel] [PATCH] av1dec: handle dimension changes via get_format
2023-06-20 23:36 [FFmpeg-devel] [PATCH] av1dec: handle dimension changes via get_format airlied
@ 2023-06-21 16:34 ` Leo Izen
2023-06-21 16:36 ` James Almer
1 sibling, 0 replies; 7+ messages in thread
From: Leo Izen @ 2023-06-21 16:34 UTC (permalink / raw)
To: ffmpeg-devel
On 6/20/23 19:36, airlied@gmail.com wrote:
> From: Dave Airlie <airlied@redhat.com>
>
> av1-1-b8-03-sizeup.ivf on vulkan causes gpu hangs as none of the
> images get resized when dimensions change, this detects the dim
> change and calls the get_format to reinit the context.
> ---
> libavcodec/av1dec.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
> index e7f98a6c81..1cec328563 100644
> --- a/libavcodec/av1dec.c
> +++ b/libavcodec/av1dec.c
> @@ -721,6 +721,7 @@ static av_cold int av1_decode_free(AVCodecContext *avctx)
> }
>
> static int set_context_with_sequence(AVCodecContext *avctx,
> + int *dim_change,
I would add a comment above this function documenting that dim_change
may be NULL, in which case it will be ignored. This is not a functional
issue, it's just comments = readable = good.
Also, nitpick, but this fits on the previous line (77 wide, less than
80). Ignore this if you think a new line is more readable, just keep in
mind.
> const AV1RawSequenceHeader *seq)
> {
> int width = seq->max_frame_width_minus_1 + 1;
> @@ -753,6 +754,8 @@ static int set_context_with_sequence(AVCodecContext *avctx,
> int ret = ff_set_dimensions(avctx, width, height);
> if (ret < 0)
> return ret;
> + if (dim_change)
> + *dim_change = 1;
> }
> avctx->sample_aspect_ratio = (AVRational) { 1, 1 };
>
> @@ -859,7 +862,7 @@ static av_cold int av1_decode_init(AVCodecContext *avctx)
> goto end;
> }
>
> - ret = set_context_with_sequence(avctx, seq);
> + ret = set_context_with_sequence(avctx, NULL, seq);
> if (ret < 0) {
> av_log(avctx, AV_LOG_WARNING, "Failed to set decoder context.\n");
> goto end;
> @@ -1202,7 +1205,7 @@ static int av1_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame)
> CodedBitstreamUnit *unit = &s->current_obu.units[i];
> AV1RawOBU *obu = unit->content;
> const AV1RawOBUHeader *header;
> -
> + int dim_change = 0;
> if (!obu)
> continue;
>
> @@ -1220,7 +1223,8 @@ static int av1_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame)
>
> s->raw_seq = &obu->obu.sequence_header;
>
> - ret = set_context_with_sequence(avctx, s->raw_seq);
> + dim_change = 0;
You don't need to initialize this variable to zero here, because you
initialized it to zero when you declared it. If you prefer to leave this
here, then remove the `= 0;` upon declaration.
> + ret = set_context_with_sequence(avctx, &dim_change, s->raw_seq);
> if (ret < 0) {
> av_log(avctx, AV_LOG_ERROR, "Failed to set context.\n");
> s->raw_seq = NULL;
> @@ -1229,7 +1233,7 @@ static int av1_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame)
>
> s->operating_point_idc = s->raw_seq->operating_point_idc[s->operating_point];
>
> - if (s->pix_fmt == AV_PIX_FMT_NONE) {
> + if (s->pix_fmt == AV_PIX_FMT_NONE || dim_change) {
> ret = get_pixel_format(avctx);
> if (ret < 0) {
> av_log(avctx, AV_LOG_ERROR,
- Leo Izen (Traneptora)
_______________________________________________
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".
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [FFmpeg-devel] [PATCH] av1dec: handle dimension changes via get_format
2023-06-20 23:36 [FFmpeg-devel] [PATCH] av1dec: handle dimension changes via get_format airlied
2023-06-21 16:34 ` Leo Izen
@ 2023-06-21 16:36 ` James Almer
2023-06-21 21:15 ` Dave Airlie
1 sibling, 1 reply; 7+ messages in thread
From: James Almer @ 2023-06-21 16:36 UTC (permalink / raw)
To: ffmpeg-devel
On 6/20/2023 8:36 PM, airlied@gmail.com wrote:
> From: Dave Airlie <airlied@redhat.com>
>
> av1-1-b8-03-sizeup.ivf on vulkan causes gpu hangs as none of the
> images get resized when dimensions change, this detects the dim
> change and calls the get_format to reinit the context.
> ---
> libavcodec/av1dec.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
> index e7f98a6c81..1cec328563 100644
> --- a/libavcodec/av1dec.c
> +++ b/libavcodec/av1dec.c
> @@ -721,6 +721,7 @@ static av_cold int av1_decode_free(AVCodecContext *avctx)
> }
>
> static int set_context_with_sequence(AVCodecContext *avctx,
> + int *dim_change,
> const AV1RawSequenceHeader *seq)
> {
> int width = seq->max_frame_width_minus_1 + 1;
> @@ -753,6 +754,8 @@ static int set_context_with_sequence(AVCodecContext *avctx,
> int ret = ff_set_dimensions(avctx, width, height);
> if (ret < 0)
> return ret;
> + if (dim_change)
> + *dim_change = 1;
> }
> avctx->sample_aspect_ratio = (AVRational) { 1, 1 };
>
> @@ -859,7 +862,7 @@ static av_cold int av1_decode_init(AVCodecContext *avctx)
> goto end;
> }
>
> - ret = set_context_with_sequence(avctx, seq);
> + ret = set_context_with_sequence(avctx, NULL, seq);
> if (ret < 0) {
> av_log(avctx, AV_LOG_WARNING, "Failed to set decoder context.\n");
> goto end;
> @@ -1202,7 +1205,7 @@ static int av1_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame)
> CodedBitstreamUnit *unit = &s->current_obu.units[i];
> AV1RawOBU *obu = unit->content;
> const AV1RawOBUHeader *header;
> -
> + int dim_change = 0;
> if (!obu)
> continue;
>
> @@ -1220,7 +1223,8 @@ static int av1_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame)
>
> s->raw_seq = &obu->obu.sequence_header;
>
> - ret = set_context_with_sequence(avctx, s->raw_seq);
> + dim_change = 0;
> + ret = set_context_with_sequence(avctx, &dim_change, s->raw_seq);
> if (ret < 0) {
> av_log(avctx, AV_LOG_ERROR, "Failed to set context.\n");
> s->raw_seq = NULL;
> @@ -1229,7 +1233,7 @@ static int av1_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame)
>
> s->operating_point_idc = s->raw_seq->operating_point_idc[s->operating_point];
>
> - if (s->pix_fmt == AV_PIX_FMT_NONE) {
> + if (s->pix_fmt == AV_PIX_FMT_NONE || dim_change) {
> ret = get_pixel_format(avctx);
Dimensions can change between frames without a seq header showing up to
change the max_width/height values. get_pixel_format() would need to be
called on frame headers instead.
> if (ret < 0) {
> av_log(avctx, AV_LOG_ERROR,
_______________________________________________
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".
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [FFmpeg-devel] [PATCH] av1dec: handle dimension changes via get_format
2023-06-21 16:36 ` James Almer
@ 2023-06-21 21:15 ` Dave Airlie
2023-06-21 21:36 ` James Almer
0 siblings, 1 reply; 7+ messages in thread
From: Dave Airlie @ 2023-06-21 21:15 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On Thu, 22 Jun 2023 at 02:36, James Almer <jamrial@gmail.com> wrote:
>
> On 6/20/2023 8:36 PM, airlied@gmail.com wrote:
> > From: Dave Airlie <airlied@redhat.com>
> >
> > av1-1-b8-03-sizeup.ivf on vulkan causes gpu hangs as none of the
> > images get resized when dimensions change, this detects the dim
> > change and calls the get_format to reinit the context.
> > ---
> > libavcodec/av1dec.c | 12 ++++++++----
> > 1 file changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
> > index e7f98a6c81..1cec328563 100644
> > --- a/libavcodec/av1dec.c
> > +++ b/libavcodec/av1dec.c
> > @@ -721,6 +721,7 @@ static av_cold int av1_decode_free(AVCodecContext *avctx)
> > }
> >
> > static int set_context_with_sequence(AVCodecContext *avctx,
> > + int *dim_change,
> > const AV1RawSequenceHeader *seq)
> > {
> > int width = seq->max_frame_width_minus_1 + 1;
> > @@ -753,6 +754,8 @@ static int set_context_with_sequence(AVCodecContext *avctx,
> > int ret = ff_set_dimensions(avctx, width, height);
> > if (ret < 0)
> > return ret;
> > + if (dim_change)
> > + *dim_change = 1;
> > }
> > avctx->sample_aspect_ratio = (AVRational) { 1, 1 };
> >
> > @@ -859,7 +862,7 @@ static av_cold int av1_decode_init(AVCodecContext *avctx)
> > goto end;
> > }
> >
> > - ret = set_context_with_sequence(avctx, seq);
> > + ret = set_context_with_sequence(avctx, NULL, seq);
> > if (ret < 0) {
> > av_log(avctx, AV_LOG_WARNING, "Failed to set decoder context.\n");
> > goto end;
> > @@ -1202,7 +1205,7 @@ static int av1_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame)
> > CodedBitstreamUnit *unit = &s->current_obu.units[i];
> > AV1RawOBU *obu = unit->content;
> > const AV1RawOBUHeader *header;
> > -
> > + int dim_change = 0;
> > if (!obu)
> > continue;
> >
> > @@ -1220,7 +1223,8 @@ static int av1_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame)
> >
> > s->raw_seq = &obu->obu.sequence_header;
> >
> > - ret = set_context_with_sequence(avctx, s->raw_seq);
> > + dim_change = 0;
> > + ret = set_context_with_sequence(avctx, &dim_change, s->raw_seq);
> > if (ret < 0) {
> > av_log(avctx, AV_LOG_ERROR, "Failed to set context.\n");
> > s->raw_seq = NULL;
> > @@ -1229,7 +1233,7 @@ static int av1_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame)
> >
> > s->operating_point_idc = s->raw_seq->operating_point_idc[s->operating_point];
> >
> > - if (s->pix_fmt == AV_PIX_FMT_NONE) {
> > + if (s->pix_fmt == AV_PIX_FMT_NONE || dim_change) {
> > ret = get_pixel_format(avctx);
>
> Dimensions can change between frames without a seq header showing up to
> change the max_width/height values. get_pixel_format() would need to be
> called on frame headers instead.
It can but my reading of the spec is that it is illegal to go beyond
the max in sequence header.
6.8 Frame header OBU semantics
6.8.4 Frame size semantics
"
It is a requirement of bitstream conformance that frame_width_minus_1
is less than or equal to max_frame_width_minus_1.
It is a requirement of bitstream conformance that frame_height_minus_1
is less than or equal to max_frame_height_minus_1.
"
Dave.
_______________________________________________
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".
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [FFmpeg-devel] [PATCH] av1dec: handle dimension changes via get_format
2023-06-21 21:15 ` Dave Airlie
@ 2023-06-21 21:36 ` James Almer
2023-06-21 21:42 ` Dave Airlie
0 siblings, 1 reply; 7+ messages in thread
From: James Almer @ 2023-06-21 21:36 UTC (permalink / raw)
To: ffmpeg-devel
On 6/21/2023 6:15 PM, Dave Airlie wrote:
> On Thu, 22 Jun 2023 at 02:36, James Almer <jamrial@gmail.com> wrote:
>>
>> On 6/20/2023 8:36 PM, airlied@gmail.com wrote:
>>> From: Dave Airlie <airlied@redhat.com>
>>>
>>> av1-1-b8-03-sizeup.ivf on vulkan causes gpu hangs as none of the
>>> images get resized when dimensions change, this detects the dim
>>> change and calls the get_format to reinit the context.
>>> ---
>>> libavcodec/av1dec.c | 12 ++++++++----
>>> 1 file changed, 8 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
>>> index e7f98a6c81..1cec328563 100644
>>> --- a/libavcodec/av1dec.c
>>> +++ b/libavcodec/av1dec.c
>>> @@ -721,6 +721,7 @@ static av_cold int av1_decode_free(AVCodecContext *avctx)
>>> }
>>>
>>> static int set_context_with_sequence(AVCodecContext *avctx,
>>> + int *dim_change,
>>> const AV1RawSequenceHeader *seq)
>>> {
>>> int width = seq->max_frame_width_minus_1 + 1;
>>> @@ -753,6 +754,8 @@ static int set_context_with_sequence(AVCodecContext *avctx,
>>> int ret = ff_set_dimensions(avctx, width, height);
>>> if (ret < 0)
>>> return ret;
>>> + if (dim_change)
>>> + *dim_change = 1;
>>> }
>>> avctx->sample_aspect_ratio = (AVRational) { 1, 1 };
>>>
>>> @@ -859,7 +862,7 @@ static av_cold int av1_decode_init(AVCodecContext *avctx)
>>> goto end;
>>> }
>>>
>>> - ret = set_context_with_sequence(avctx, seq);
>>> + ret = set_context_with_sequence(avctx, NULL, seq);
>>> if (ret < 0) {
>>> av_log(avctx, AV_LOG_WARNING, "Failed to set decoder context.\n");
>>> goto end;
>>> @@ -1202,7 +1205,7 @@ static int av1_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame)
>>> CodedBitstreamUnit *unit = &s->current_obu.units[i];
>>> AV1RawOBU *obu = unit->content;
>>> const AV1RawOBUHeader *header;
>>> -
>>> + int dim_change = 0;
>>> if (!obu)
>>> continue;
>>>
>>> @@ -1220,7 +1223,8 @@ static int av1_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame)
>>>
>>> s->raw_seq = &obu->obu.sequence_header;
>>>
>>> - ret = set_context_with_sequence(avctx, s->raw_seq);
>>> + dim_change = 0;
>>> + ret = set_context_with_sequence(avctx, &dim_change, s->raw_seq);
>>> if (ret < 0) {
>>> av_log(avctx, AV_LOG_ERROR, "Failed to set context.\n");
>>> s->raw_seq = NULL;
>>> @@ -1229,7 +1233,7 @@ static int av1_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame)
>>>
>>> s->operating_point_idc = s->raw_seq->operating_point_idc[s->operating_point];
>>>
>>> - if (s->pix_fmt == AV_PIX_FMT_NONE) {
>>> + if (s->pix_fmt == AV_PIX_FMT_NONE || dim_change) {
>>> ret = get_pixel_format(avctx);
>>
>> Dimensions can change between frames without a seq header showing up to
>> change the max_width/height values. get_pixel_format() would need to be
>> called on frame headers instead.
>
> It can but my reading of the spec is that it is illegal to go beyond
> the max in sequence header.
>
> 6.8 Frame header OBU semantics
> 6.8.4 Frame size semantics
>
> "
> It is a requirement of bitstream conformance that frame_width_minus_1
> is less than or equal to max_frame_width_minus_1.
> It is a requirement of bitstream conformance that frame_height_minus_1
> is less than or equal to max_frame_height_minus_1.
> "
>
> Dave.
So Vulkan always allocates buffers for the max allowed dimensions and
not for the current frame's dimensions?
_______________________________________________
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".
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [FFmpeg-devel] [PATCH] av1dec: handle dimension changes via get_format
2023-06-21 21:36 ` James Almer
@ 2023-06-21 21:42 ` Dave Airlie
2023-06-21 21:44 ` James Almer
0 siblings, 1 reply; 7+ messages in thread
From: Dave Airlie @ 2023-06-21 21:42 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On Thu, 22 Jun 2023 at 07:36, James Almer <jamrial@gmail.com> wrote:
>
> On 6/21/2023 6:15 PM, Dave Airlie wrote:
> > On Thu, 22 Jun 2023 at 02:36, James Almer <jamrial@gmail.com> wrote:
> >>
> >> On 6/20/2023 8:36 PM, airlied@gmail.com wrote:
> >>> From: Dave Airlie <airlied@redhat.com>
> >>>
> >>> av1-1-b8-03-sizeup.ivf on vulkan causes gpu hangs as none of the
> >>> images get resized when dimensions change, this detects the dim
> >>> change and calls the get_format to reinit the context.
> >>> ---
> >>> libavcodec/av1dec.c | 12 ++++++++----
> >>> 1 file changed, 8 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
> >>> index e7f98a6c81..1cec328563 100644
> >>> --- a/libavcodec/av1dec.c
> >>> +++ b/libavcodec/av1dec.c
> >>> @@ -721,6 +721,7 @@ static av_cold int av1_decode_free(AVCodecContext *avctx)
> >>> }
> >>>
> >>> static int set_context_with_sequence(AVCodecContext *avctx,
> >>> + int *dim_change,
> >>> const AV1RawSequenceHeader *seq)
> >>> {
> >>> int width = seq->max_frame_width_minus_1 + 1;
> >>> @@ -753,6 +754,8 @@ static int set_context_with_sequence(AVCodecContext *avctx,
> >>> int ret = ff_set_dimensions(avctx, width, height);
> >>> if (ret < 0)
> >>> return ret;
> >>> + if (dim_change)
> >>> + *dim_change = 1;
> >>> }
> >>> avctx->sample_aspect_ratio = (AVRational) { 1, 1 };
> >>>
> >>> @@ -859,7 +862,7 @@ static av_cold int av1_decode_init(AVCodecContext *avctx)
> >>> goto end;
> >>> }
> >>>
> >>> - ret = set_context_with_sequence(avctx, seq);
> >>> + ret = set_context_with_sequence(avctx, NULL, seq);
> >>> if (ret < 0) {
> >>> av_log(avctx, AV_LOG_WARNING, "Failed to set decoder context.\n");
> >>> goto end;
> >>> @@ -1202,7 +1205,7 @@ static int av1_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame)
> >>> CodedBitstreamUnit *unit = &s->current_obu.units[i];
> >>> AV1RawOBU *obu = unit->content;
> >>> const AV1RawOBUHeader *header;
> >>> -
> >>> + int dim_change = 0;
> >>> if (!obu)
> >>> continue;
> >>>
> >>> @@ -1220,7 +1223,8 @@ static int av1_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame)
> >>>
> >>> s->raw_seq = &obu->obu.sequence_header;
> >>>
> >>> - ret = set_context_with_sequence(avctx, s->raw_seq);
> >>> + dim_change = 0;
> >>> + ret = set_context_with_sequence(avctx, &dim_change, s->raw_seq);
> >>> if (ret < 0) {
> >>> av_log(avctx, AV_LOG_ERROR, "Failed to set context.\n");
> >>> s->raw_seq = NULL;
> >>> @@ -1229,7 +1233,7 @@ static int av1_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame)
> >>>
> >>> s->operating_point_idc = s->raw_seq->operating_point_idc[s->operating_point];
> >>>
> >>> - if (s->pix_fmt == AV_PIX_FMT_NONE) {
> >>> + if (s->pix_fmt == AV_PIX_FMT_NONE || dim_change) {
> >>> ret = get_pixel_format(avctx);
> >>
> >> Dimensions can change between frames without a seq header showing up to
> >> change the max_width/height values. get_pixel_format() would need to be
> >> called on frame headers instead.
> >
> > It can but my reading of the spec is that it is illegal to go beyond
> > the max in sequence header.
> >
> > 6.8 Frame header OBU semantics
> > 6.8.4 Frame size semantics
> >
> > "
> > It is a requirement of bitstream conformance that frame_width_minus_1
> > is less than or equal to max_frame_width_minus_1.
> > It is a requirement of bitstream conformance that frame_height_minus_1
> > is less than or equal to max_frame_height_minus_1.
> > "
> >
> > Dave.
>
> So Vulkan always allocates buffers for the max allowed dimensions and
> not for the current frame's dimensions?
It doesn't seem to be in vulkan specific code:
av1dec.c:set_context_with_sequence
int width = seq->max_frame_width_minus_1 + 1;
int height = seq->max_frame_height_minus_1 + 1;
is where it sets the values later used to allocate the frames.
Dave.
_______________________________________________
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".
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [FFmpeg-devel] [PATCH] av1dec: handle dimension changes via get_format
2023-06-21 21:42 ` Dave Airlie
@ 2023-06-21 21:44 ` James Almer
0 siblings, 0 replies; 7+ messages in thread
From: James Almer @ 2023-06-21 21:44 UTC (permalink / raw)
To: ffmpeg-devel
On 6/21/2023 6:42 PM, Dave Airlie wrote:
> On Thu, 22 Jun 2023 at 07:36, James Almer <jamrial@gmail.com> wrote:
>>
>> On 6/21/2023 6:15 PM, Dave Airlie wrote:
>>> On Thu, 22 Jun 2023 at 02:36, James Almer <jamrial@gmail.com> wrote:
>>>>
>>>> On 6/20/2023 8:36 PM, airlied@gmail.com wrote:
>>>>> From: Dave Airlie <airlied@redhat.com>
>>>>>
>>>>> av1-1-b8-03-sizeup.ivf on vulkan causes gpu hangs as none of the
>>>>> images get resized when dimensions change, this detects the dim
>>>>> change and calls the get_format to reinit the context.
>>>>> ---
>>>>> libavcodec/av1dec.c | 12 ++++++++----
>>>>> 1 file changed, 8 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
>>>>> index e7f98a6c81..1cec328563 100644
>>>>> --- a/libavcodec/av1dec.c
>>>>> +++ b/libavcodec/av1dec.c
>>>>> @@ -721,6 +721,7 @@ static av_cold int av1_decode_free(AVCodecContext *avctx)
>>>>> }
>>>>>
>>>>> static int set_context_with_sequence(AVCodecContext *avctx,
>>>>> + int *dim_change,
>>>>> const AV1RawSequenceHeader *seq)
>>>>> {
>>>>> int width = seq->max_frame_width_minus_1 + 1;
>>>>> @@ -753,6 +754,8 @@ static int set_context_with_sequence(AVCodecContext *avctx,
>>>>> int ret = ff_set_dimensions(avctx, width, height);
>>>>> if (ret < 0)
>>>>> return ret;
>>>>> + if (dim_change)
>>>>> + *dim_change = 1;
>>>>> }
>>>>> avctx->sample_aspect_ratio = (AVRational) { 1, 1 };
>>>>>
>>>>> @@ -859,7 +862,7 @@ static av_cold int av1_decode_init(AVCodecContext *avctx)
>>>>> goto end;
>>>>> }
>>>>>
>>>>> - ret = set_context_with_sequence(avctx, seq);
>>>>> + ret = set_context_with_sequence(avctx, NULL, seq);
>>>>> if (ret < 0) {
>>>>> av_log(avctx, AV_LOG_WARNING, "Failed to set decoder context.\n");
>>>>> goto end;
>>>>> @@ -1202,7 +1205,7 @@ static int av1_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame)
>>>>> CodedBitstreamUnit *unit = &s->current_obu.units[i];
>>>>> AV1RawOBU *obu = unit->content;
>>>>> const AV1RawOBUHeader *header;
>>>>> -
>>>>> + int dim_change = 0;
>>>>> if (!obu)
>>>>> continue;
>>>>>
>>>>> @@ -1220,7 +1223,8 @@ static int av1_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame)
>>>>>
>>>>> s->raw_seq = &obu->obu.sequence_header;
>>>>>
>>>>> - ret = set_context_with_sequence(avctx, s->raw_seq);
>>>>> + dim_change = 0;
>>>>> + ret = set_context_with_sequence(avctx, &dim_change, s->raw_seq);
>>>>> if (ret < 0) {
>>>>> av_log(avctx, AV_LOG_ERROR, "Failed to set context.\n");
>>>>> s->raw_seq = NULL;
>>>>> @@ -1229,7 +1233,7 @@ static int av1_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame)
>>>>>
>>>>> s->operating_point_idc = s->raw_seq->operating_point_idc[s->operating_point];
>>>>>
>>>>> - if (s->pix_fmt == AV_PIX_FMT_NONE) {
>>>>> + if (s->pix_fmt == AV_PIX_FMT_NONE || dim_change) {
>>>>> ret = get_pixel_format(avctx);
>>>>
>>>> Dimensions can change between frames without a seq header showing up to
>>>> change the max_width/height values. get_pixel_format() would need to be
>>>> called on frame headers instead.
>>>
>>> It can but my reading of the spec is that it is illegal to go beyond
>>> the max in sequence header.
>>>
>>> 6.8 Frame header OBU semantics
>>> 6.8.4 Frame size semantics
>>>
>>> "
>>> It is a requirement of bitstream conformance that frame_width_minus_1
>>> is less than or equal to max_frame_width_minus_1.
>>> It is a requirement of bitstream conformance that frame_height_minus_1
>>> is less than or equal to max_frame_height_minus_1.
>>> "
>>>
>>> Dave.
>>
>> So Vulkan always allocates buffers for the max allowed dimensions and
>> not for the current frame's dimensions?
>
> It doesn't seem to be in vulkan specific code:
>
> av1dec.c:set_context_with_sequence
> int width = seq->max_frame_width_minus_1 + 1;
> int height = seq->max_frame_height_minus_1 + 1;
>
> is where it sets the values later used to allocate the frames.
>
> Dave.
I see a call to ff_set_dimensions() in
update_context_with_frame_header() which uses frame header dimensions.
_______________________________________________
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".
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-06-21 21:44 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-20 23:36 [FFmpeg-devel] [PATCH] av1dec: handle dimension changes via get_format airlied
2023-06-21 16:34 ` Leo Izen
2023-06-21 16:36 ` James Almer
2023-06-21 21:15 ` Dave Airlie
2023-06-21 21:36 ` James Almer
2023-06-21 21:42 ` Dave Airlie
2023-06-21 21:44 ` James Almer
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git