* [FFmpeg-devel] [PATCH] vulkan_av1: Set force_integer_mv on intra frames @ 2024-04-27 13:40 Mark Thompson 2024-04-27 13:47 ` Ronald S. Bultje 0 siblings, 1 reply; 5+ messages in thread From: Mark Thompson @ 2024-04-27 13:40 UTC (permalink / raw) To: FFmpeg development discussions and patches The flag in CBS is the value read from the bitstream (as required for passthrough), but the specification overrides that by setting it immediately after reading if the frame is intra. --- This is already done for DXVA and VDPAU. Also wondering whether this should be done for VAAPI? libavcodec/vulkan_av1.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavcodec/vulkan_av1.c b/libavcodec/vulkan_av1.c index 25ab4ecc70..694e643954 100644 --- a/libavcodec/vulkan_av1.c +++ b/libavcodec/vulkan_av1.c @@ -435,7 +435,7 @@ static int vk_av1_start_frame(AVCodecContext *avctx, .render_and_frame_size_different = frame_header->render_and_frame_size_different, .allow_screen_content_tools = frame_header->allow_screen_content_tools, .is_filter_switchable = frame_header->is_filter_switchable, - .force_integer_mv = frame_header->force_integer_mv, + .force_integer_mv = frame_header->force_integer_mv || !(frame_header->frame_type & 1), .frame_size_override_flag = frame_header->frame_size_override_flag, .buffer_removal_time_present_flag = frame_header->buffer_removal_time_present_flag, .allow_intrabc = frame_header->allow_intrabc, -- 2.43.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] 5+ messages in thread
* Re: [FFmpeg-devel] [PATCH] vulkan_av1: Set force_integer_mv on intra frames 2024-04-27 13:40 [FFmpeg-devel] [PATCH] vulkan_av1: Set force_integer_mv on intra frames Mark Thompson @ 2024-04-27 13:47 ` Ronald S. Bultje 2024-04-27 13:58 ` Mark Thompson 0 siblings, 1 reply; 5+ messages in thread From: Ronald S. Bultje @ 2024-04-27 13:47 UTC (permalink / raw) To: FFmpeg development discussions and patches Hi, On Sat, Apr 27, 2024 at 9:40 AM Mark Thompson <sw@jkqxz.net> wrote: > The flag in CBS is the value read from the bitstream (as required for > passthrough), but the specification overrides that by setting it > immediately after reading if the frame is intra. > --- > This is already done for DXVA and VDPAU. Also wondering whether this > should be done for VAAPI? > > libavcodec/vulkan_av1.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavcodec/vulkan_av1.c b/libavcodec/vulkan_av1.c > index 25ab4ecc70..694e643954 100644 > --- a/libavcodec/vulkan_av1.c > +++ b/libavcodec/vulkan_av1.c > @@ -435,7 +435,7 @@ static int vk_av1_start_frame(AVCodecContext > *avctx, > .render_and_frame_size_different = > frame_header->render_and_frame_size_different, > .allow_screen_content_tools = > frame_header->allow_screen_content_tools, > .is_filter_switchable = frame_header->is_filter_switchable, > - .force_integer_mv = frame_header->force_integer_mv, > + .force_integer_mv = frame_header->force_integer_mv || > !(frame_header->frame_type & 1), > .frame_size_override_flag = > frame_header->frame_size_override_flag, > .buffer_removal_time_present_flag = > frame_header->buffer_removal_time_present_flag, > .allow_intrabc = frame_header->allow_intrabc, > -- > 2.43.0 > Wouldn't it be better to adjust the CBS reader then? Prevents us from having to re-do this in every hw wrapper. Ronald _______________________________________________ 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] 5+ messages in thread
* Re: [FFmpeg-devel] [PATCH] vulkan_av1: Set force_integer_mv on intra frames 2024-04-27 13:47 ` Ronald S. Bultje @ 2024-04-27 13:58 ` Mark Thompson 2024-04-27 14:07 ` Mark Thompson 0 siblings, 1 reply; 5+ messages in thread From: Mark Thompson @ 2024-04-27 13:58 UTC (permalink / raw) To: ffmpeg-devel On 27/04/2024 14:47, Ronald S. Bultje wrote: > Hi, > > On Sat, Apr 27, 2024 at 9:40 AM Mark Thompson <sw@jkqxz.net> wrote: > >> The flag in CBS is the value read from the bitstream (as required for >> passthrough), but the specification overrides that by setting it >> immediately after reading if the frame is intra. >> --- >> This is already done for DXVA and VDPAU. Also wondering whether this >> should be done for VAAPI? >> >> libavcodec/vulkan_av1.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/libavcodec/vulkan_av1.c b/libavcodec/vulkan_av1.c >> index 25ab4ecc70..694e643954 100644 >> --- a/libavcodec/vulkan_av1.c >> +++ b/libavcodec/vulkan_av1.c >> @@ -435,7 +435,7 @@ static int vk_av1_start_frame(AVCodecContext >> *avctx, >> .render_and_frame_size_different = >> frame_header->render_and_frame_size_different, >> .allow_screen_content_tools = >> frame_header->allow_screen_content_tools, >> .is_filter_switchable = frame_header->is_filter_switchable, >> - .force_integer_mv = frame_header->force_integer_mv, >> + .force_integer_mv = frame_header->force_integer_mv || >> !(frame_header->frame_type & 1), >> .frame_size_override_flag = >> frame_header->frame_size_override_flag, >> .buffer_removal_time_present_flag = >> frame_header->buffer_removal_time_present_flag, >> .allow_intrabc = frame_header->allow_intrabc, >> -- >> 2.43.0 >> > > Wouldn't it be better to adjust the CBS reader then? Prevents us from > having to re-do this in every hw wrapper. How would you suggest doing this? We could add a derived field force_integer_mv somewhere else (av1dec?) but that doesn't seem obviously better for use in the "copy stuff from frame header" function. - Mark _______________________________________________ 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] 5+ messages in thread
* Re: [FFmpeg-devel] [PATCH] vulkan_av1: Set force_integer_mv on intra frames 2024-04-27 13:58 ` Mark Thompson @ 2024-04-27 14:07 ` Mark Thompson 2024-04-27 14:58 ` Lynne 0 siblings, 1 reply; 5+ messages in thread From: Mark Thompson @ 2024-04-27 14:07 UTC (permalink / raw) To: ffmpeg-devel On 27/04/2024 14:58, Mark Thompson wrote: > On 27/04/2024 14:47, Ronald S. Bultje wrote: >> Hi, >> >> On Sat, Apr 27, 2024 at 9:40 AM Mark Thompson <sw@jkqxz.net> wrote: >> >>> The flag in CBS is the value read from the bitstream (as required for >>> passthrough), but the specification overrides that by setting it >>> immediately after reading if the frame is intra. >>> --- >>> This is already done for DXVA and VDPAU. Also wondering whether this >>> should be done for VAAPI? >>> >>> libavcodec/vulkan_av1.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/libavcodec/vulkan_av1.c b/libavcodec/vulkan_av1.c >>> index 25ab4ecc70..694e643954 100644 >>> --- a/libavcodec/vulkan_av1.c >>> +++ b/libavcodec/vulkan_av1.c >>> @@ -435,7 +435,7 @@ static int vk_av1_start_frame(AVCodecContext >>> *avctx, >>> .render_and_frame_size_different = >>> frame_header->render_and_frame_size_different, >>> .allow_screen_content_tools = >>> frame_header->allow_screen_content_tools, >>> .is_filter_switchable = frame_header->is_filter_switchable, >>> - .force_integer_mv = frame_header->force_integer_mv, >>> + .force_integer_mv = frame_header->force_integer_mv || >>> !(frame_header->frame_type & 1), >>> .frame_size_override_flag = >>> frame_header->frame_size_override_flag, >>> .buffer_removal_time_present_flag = >>> frame_header->buffer_removal_time_present_flag, >>> .allow_intrabc = frame_header->allow_intrabc, >>> -- >>> 2.43.0 >>> >> >> Wouldn't it be better to adjust the CBS reader then? Prevents us from >> having to re-do this in every hw wrapper. > > How would you suggest doing this? > > We could add a derived field force_integer_mv somewhere else (av1dec?) but that doesn't seem obviously better for use in the "copy stuff from frame header" function. Something like this. Unsure whether this is more or less confusing! libavcodec/av1dec.c | 7 +++++++ libavcodec/av1dec.h | 3 +++ libavcodec/vulkan_av1.c | 2 +- 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c index 79a30a114d..601e3f05e1 100644 --- a/libavcodec/av1dec.c +++ b/libavcodec/av1dec.c @@ -725,6 +725,8 @@ static void av1_frame_replace(AV1Frame *dst, const AV1Frame *src) sizeof(dst->ref_frame_sign_bias)); memcpy(dst->order_hints, src->order_hints, sizeof(dst->order_hints)); + + dst->force_integer_mv = src->force_integer_mv; } static av_cold int av1_decode_free(AVCodecContext *avctx) @@ -1255,6 +1257,11 @@ static int get_current_frame(AVCodecContext *avctx) order_hint_info(s); load_grain_params(s); + s->cur_frame.force_integer_mv = + s->raw_frame_header->force_integer_mv || + s->raw_frame_header->frame_type == AV1_FRAME_KEY || + s->raw_frame_header->frame_type == AV1_FRAME_INTRA_ONLY; + return ret; } diff --git a/libavcodec/av1dec.h b/libavcodec/av1dec.h index 66a732d781..e3016b82a3 100644 --- a/libavcodec/av1dec.h +++ b/libavcodec/av1dec.h @@ -67,6 +67,9 @@ typedef struct AV1Frame { // OrderHints[] when this is the current frame, otherwise // SavedOrderHints[s][] when is the reference frame in slot s. uint8_t order_hints[AV1_TOTAL_REFS_PER_FRAME]; + + // force_integer_mv value at the end of the frame header parsing. + uint8_t force_integer_mv; } AV1Frame; typedef struct TileGroupInfo { diff --git a/libavcodec/vulkan_av1.c b/libavcodec/vulkan_av1.c index 25ab4ecc70..a550215e32 100644 --- a/libavcodec/vulkan_av1.c +++ b/libavcodec/vulkan_av1.c @@ -435,7 +435,7 @@ static int vk_av1_start_frame(AVCodecContext *avctx, .render_and_frame_size_different = frame_header->render_and_frame_size_different, .allow_screen_content_tools = frame_header->allow_screen_content_tools, .is_filter_switchable = frame_header->is_filter_switchable, - .force_integer_mv = frame_header->force_integer_mv, + .force_integer_mv = pic->force_integer_mv, .frame_size_override_flag = frame_header->frame_size_override_flag, .buffer_removal_time_present_flag = frame_header->buffer_removal_time_present_flag, .allow_intrabc = frame_header->allow_intrabc, -- 2.43.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] 5+ messages in thread
* Re: [FFmpeg-devel] [PATCH] vulkan_av1: Set force_integer_mv on intra frames 2024-04-27 14:07 ` Mark Thompson @ 2024-04-27 14:58 ` Lynne 0 siblings, 0 replies; 5+ messages in thread From: Lynne @ 2024-04-27 14:58 UTC (permalink / raw) To: FFmpeg development discussions and patches Apr 27, 2024, 16:07 by sw@jkqxz.net: > On 27/04/2024 14:58, Mark Thompson wrote: > >> On 27/04/2024 14:47, Ronald S. Bultje wrote: >> >>> Hi, >>> >>> On Sat, Apr 27, 2024 at 9:40 AM Mark Thompson <sw@jkqxz.net> wrote: >>> >>>> The flag in CBS is the value read from the bitstream (as required for >>>> passthrough), but the specification overrides that by setting it >>>> immediately after reading if the frame is intra. >>>> --- >>>> This is already done for DXVA and VDPAU. Also wondering whether this >>>> should be done for VAAPI? >>>> >>>> libavcodec/vulkan_av1.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/libavcodec/vulkan_av1.c b/libavcodec/vulkan_av1.c >>>> index 25ab4ecc70..694e643954 100644 >>>> --- a/libavcodec/vulkan_av1.c >>>> +++ b/libavcodec/vulkan_av1.c >>>> @@ -435,7 +435,7 @@ static int vk_av1_start_frame(AVCodecContext >>>> *avctx, >>>> .render_and_frame_size_different = >>>> frame_header->render_and_frame_size_different, >>>> .allow_screen_content_tools = >>>> frame_header->allow_screen_content_tools, >>>> .is_filter_switchable = frame_header->is_filter_switchable, >>>> - .force_integer_mv = frame_header->force_integer_mv, >>>> + .force_integer_mv = frame_header->force_integer_mv || >>>> !(frame_header->frame_type & 1), >>>> .frame_size_override_flag = >>>> frame_header->frame_size_override_flag, >>>> .buffer_removal_time_present_flag = >>>> frame_header->buffer_removal_time_present_flag, >>>> .allow_intrabc = frame_header->allow_intrabc, >>>> -- >>>> 2.43.0 >>>> >>> >>> Wouldn't it be better to adjust the CBS reader then? Prevents us from >>> having to re-do this in every hw wrapper. >>> >> >> How would you suggest doing this? >> >> We could add a derived field force_integer_mv somewhere else (av1dec?) but that doesn't seem obviously better for use in the "copy stuff from frame header" function. >> > > Something like this. Unsure whether this is more or less confusing! > > libavcodec/av1dec.c | 7 +++++++ > libavcodec/av1dec.h | 3 +++ > libavcodec/vulkan_av1.c | 2 +- > 3 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c > index 79a30a114d..601e3f05e1 100644 > --- a/libavcodec/av1dec.c > +++ b/libavcodec/av1dec.c > @@ -725,6 +725,8 @@ static void av1_frame_replace(AV1Frame *dst, const AV1Frame *src) > sizeof(dst->ref_frame_sign_bias)); > memcpy(dst->order_hints, src->order_hints, > sizeof(dst->order_hints)); > + > + dst->force_integer_mv = src->force_integer_mv; > } > > static av_cold int av1_decode_free(AVCodecContext *avctx) > @@ -1255,6 +1257,11 @@ static int get_current_frame(AVCodecContext *avctx) > order_hint_info(s); > load_grain_params(s); > > + s->cur_frame.force_integer_mv = > + s->raw_frame_header->force_integer_mv || > + s->raw_frame_header->frame_type == AV1_FRAME_KEY || > + s->raw_frame_header->frame_type == AV1_FRAME_INTRA_ONLY; > + > return ret; > } > > diff --git a/libavcodec/av1dec.h b/libavcodec/av1dec.h > index 66a732d781..e3016b82a3 100644 > --- a/libavcodec/av1dec.h > +++ b/libavcodec/av1dec.h > @@ -67,6 +67,9 @@ typedef struct AV1Frame { > // OrderHints[] when this is the current frame, otherwise > // SavedOrderHints[s][] when is the reference frame in slot s. > uint8_t order_hints[AV1_TOTAL_REFS_PER_FRAME]; > + > + // force_integer_mv value at the end of the frame header parsing. > + uint8_t force_integer_mv; > } AV1Frame; > > typedef struct TileGroupInfo { > diff --git a/libavcodec/vulkan_av1.c b/libavcodec/vulkan_av1.c > index 25ab4ecc70..a550215e32 100644 > --- a/libavcodec/vulkan_av1.c > +++ b/libavcodec/vulkan_av1.c > @@ -435,7 +435,7 @@ static int vk_av1_start_frame(AVCodecContext *avctx, > .render_and_frame_size_different = frame_header->render_and_frame_size_different, > .allow_screen_content_tools = frame_header->allow_screen_content_tools, > .is_filter_switchable = frame_header->is_filter_switchable, > - .force_integer_mv = frame_header->force_integer_mv, > + .force_integer_mv = pic->force_integer_mv, > .frame_size_override_flag = frame_header->frame_size_override_flag, > .buffer_removal_time_present_flag = frame_header->buffer_removal_time_present_flag, > .allow_intrabc = frame_header->allow_intrabc, > This looks better to me. _______________________________________________ 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] 5+ messages in thread
end of thread, other threads:[~2024-04-27 14:58 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-04-27 13:40 [FFmpeg-devel] [PATCH] vulkan_av1: Set force_integer_mv on intra frames Mark Thompson 2024-04-27 13:47 ` Ronald S. Bultje 2024-04-27 13:58 ` Mark Thompson 2024-04-27 14:07 ` Mark Thompson 2024-04-27 14:58 ` Lynne
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