diff mbox series

[FFmpeg-devel,v2] avcodec/h264dec: Remove ff_h264_draw_horiz_band

Message ID AS8P250MB074475B0CC5B94D642C47EB68FC52@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM
State New
Headers show
Series [FFmpeg-devel,v2] avcodec/h264dec: Remove ff_h264_draw_horiz_band | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Andreas Rheinhardt June 9, 2024, 1:39 a.m. UTC 
The H.264 decoder does not support draw_horiz_band (it does not have
the AV_CODEC_CAP_DRAW_HORIZ_BAND), making ff_h264_draw_horiz_band()
legally dead. The function here always calls draw_horiz_band
in coded order, although the default case is display order.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/d3d12va_h264.c | 12 ++++--------
 libavcodec/dxva2_h264.c   | 13 ++++---------
 libavcodec/h264_slice.c   |  2 --
 libavcodec/h264dec.c      | 39 ---------------------------------------
 libavcodec/h264dec.h      |  2 --
 libavcodec/vaapi_h264.c   | 11 +----------
 libavcodec/vdpau_h264.c   |  2 --
 7 files changed, 9 insertions(+), 72 deletions(-)

Comments

Andreas Rheinhardt June 14, 2024, 11:01 a.m. UTC | #1 
Andreas Rheinhardt:
> The H.264 decoder does not support draw_horiz_band (it does not have
> the AV_CODEC_CAP_DRAW_HORIZ_BAND), making ff_h264_draw_horiz_band()
> legally dead. The function here always calls draw_horiz_band
> in coded order, although the default case is display order.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>  libavcodec/d3d12va_h264.c | 12 ++++--------
>  libavcodec/dxva2_h264.c   | 13 ++++---------
>  libavcodec/h264_slice.c   |  2 --
>  libavcodec/h264dec.c      | 39 ---------------------------------------
>  libavcodec/h264dec.h      |  2 --
>  libavcodec/vaapi_h264.c   | 11 +----------
>  libavcodec/vdpau_h264.c   |  2 --
>  7 files changed, 9 insertions(+), 72 deletions(-)
> 
> diff --git a/libavcodec/d3d12va_h264.c b/libavcodec/d3d12va_h264.c
> index b2fe2955c8..fe23f083df 100644
> --- a/libavcodec/d3d12va_h264.c
> +++ b/libavcodec/d3d12va_h264.c
> @@ -163,14 +163,10 @@ static int d3d12va_h264_end_frame(AVCodecContext *avctx)
>      if (ctx_pic->slice_count <= 0 || ctx_pic->bitstream_size <= 0)
>          return -1;
>  
> -    ret = ff_d3d12va_common_end_frame(avctx, h->cur_pic_ptr->f,
> -                                      &ctx_pic->pp, sizeof(ctx_pic->pp),
> -                                      &ctx_pic->qm, sizeof(ctx_pic->qm),
> -                                      update_input_arguments);
> -    if (!ret)
> -        ff_h264_draw_horiz_band(h, sl, 0, h->avctx->height);
> -
> -    return ret;
> +    return ff_d3d12va_common_end_frame(avctx, h->cur_pic_ptr->f,
> +                                       &ctx_pic->pp, sizeof(ctx_pic->pp),
> +                                       &ctx_pic->qm, sizeof(ctx_pic->qm),
> +                                       update_input_arguments);
>  }
>  
>  static int d3d12va_h264_decode_init(AVCodecContext *avctx)
> diff --git a/libavcodec/dxva2_h264.c b/libavcodec/dxva2_h264.c
> index 0fe4152625..a6fbf61ea9 100644
> --- a/libavcodec/dxva2_h264.c
> +++ b/libavcodec/dxva2_h264.c
> @@ -502,20 +502,15 @@ static int dxva2_h264_decode_slice(AVCodecContext *avctx,
>  static int dxva2_h264_end_frame(AVCodecContext *avctx)
>  {
>      H264Context *h = avctx->priv_data;
> -    H264SliceContext *sl = &h->slice_ctx[0];
>      struct dxva2_picture_context *ctx_pic =
>          h->cur_pic_ptr->hwaccel_picture_private;
> -    int ret;
>  
>      if (ctx_pic->slice_count <= 0 || ctx_pic->bitstream_size <= 0)
>          return -1;
> -    ret = ff_dxva2_common_end_frame(avctx, h->cur_pic_ptr->f,
> -                                    &ctx_pic->pp, sizeof(ctx_pic->pp),
> -                                    &ctx_pic->qm, sizeof(ctx_pic->qm),
> -                                    commit_bitstream_and_slice_buffer);
> -    if (!ret)
> -        ff_h264_draw_horiz_band(h, sl, 0, h->avctx->height);
> -    return ret;
> +    return ff_dxva2_common_end_frame(avctx, h->cur_pic_ptr->f,
> +                                     &ctx_pic->pp, sizeof(ctx_pic->pp),
> +                                     &ctx_pic->qm, sizeof(ctx_pic->qm),
> +                                     commit_bitstream_and_slice_buffer);
>  }
>  
>  #if CONFIG_H264_DXVA2_HWACCEL
> diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
> index ce2c4caca1..ceca1d1046 100644
> --- a/libavcodec/h264_slice.c
> +++ b/libavcodec/h264_slice.c
> @@ -2523,8 +2523,6 @@ static void decode_finish_row(const H264Context *h, H264SliceContext *sl)
>          top    = 0;
>      }
>  
> -    ff_h264_draw_horiz_band(h, sl, top, height);
> -
>      if (h->droppable || h->er.error_occurred)
>          return;
>  
> diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
> index fd23e367b4..a09714b714 100644
> --- a/libavcodec/h264dec.c
> +++ b/libavcodec/h264dec.c
> @@ -30,7 +30,6 @@
>  #include "config_components.h"
>  
>  #include "libavutil/avassert.h"
> -#include "libavutil/emms.h"
>  #include "libavutil/imgutils.h"
>  #include "libavutil/mem.h"
>  #include "libavutil/opt.h"
> @@ -100,44 +99,6 @@ static void h264_er_decode_mb(void *opaque, int ref, int mv_dir, int mv_type,
>      ff_h264_hl_decode_mb(h, &h->slice_ctx[0]);
>  }
>  
> -void ff_h264_draw_horiz_band(const H264Context *h, H264SliceContext *sl,
> -                             int y, int height)
> -{
> -    AVCodecContext *avctx = h->avctx;
> -    const AVFrame   *src  = h->cur_pic.f;
> -    const AVPixFmtDescriptor *desc;
> -    int offset[AV_NUM_DATA_POINTERS];
> -    int vshift;
> -    const int field_pic = h->picture_structure != PICT_FRAME;
> -
> -    if (!avctx->draw_horiz_band)
> -        return;
> -
> -    if (field_pic && h->first_field && !(avctx->slice_flags & SLICE_FLAG_ALLOW_FIELD))
> -        return;
> -
> -    if (field_pic) {
> -        height <<= 1;
> -        y      <<= 1;
> -    }
> -
> -    height = FFMIN(height, avctx->height - y);
> -
> -    desc   = av_pix_fmt_desc_get(avctx->pix_fmt);
> -    vshift = desc->log2_chroma_h;
> -
> -    offset[0] = y * src->linesize[0];
> -    offset[1] =
> -    offset[2] = (y >> vshift) * src->linesize[1];
> -    for (int i = 3; i < AV_NUM_DATA_POINTERS; i++)
> -        offset[i] = 0;
> -
> -    emms_c();
> -
> -    avctx->draw_horiz_band(avctx, src, offset,
> -                           y, h->picture_structure, height);
> -}
> -
>  void ff_h264_free_tables(H264Context *h)
>  {
>      int i;
> diff --git a/libavcodec/h264dec.h b/libavcodec/h264dec.h
> index fc50df90f2..c73c6d085a 100644
> --- a/libavcodec/h264dec.h
> +++ b/libavcodec/h264dec.h
> @@ -677,8 +677,6 @@ void ff_h264_unref_picture(H264Picture *pic);
>  
>  void ff_h264_slice_context_init(H264Context *h, H264SliceContext *sl);
>  
> -void ff_h264_draw_horiz_band(const H264Context *h, H264SliceContext *sl, int y, int height);
> -
>  /**
>   * Submit a slice for decoding.
>   *
> diff --git a/libavcodec/vaapi_h264.c b/libavcodec/vaapi_h264.c
> index 398e92568c..670d3273cb 100644
> --- a/libavcodec/vaapi_h264.c
> +++ b/libavcodec/vaapi_h264.c
> @@ -319,17 +319,8 @@ static int vaapi_h264_end_frame(AVCodecContext *avctx)
>  {
>      const H264Context *h = avctx->priv_data;
>      VAAPIDecodePicture *pic = h->cur_pic_ptr->hwaccel_picture_private;
> -    H264SliceContext *sl = &h->slice_ctx[0];
> -    int ret;
>  
> -    ret = ff_vaapi_decode_issue(avctx, pic);
> -    if (ret < 0)
> -        goto finish;
> -
> -    ff_h264_draw_horiz_band(h, sl, 0, h->avctx->height);
> -
> -finish:
> -    return ret;
> +    return ff_vaapi_decode_issue(avctx, pic);
>  }
>  
>  /** Decode the given H.264 slice with VA API. */
> diff --git a/libavcodec/vdpau_h264.c b/libavcodec/vdpau_h264.c
> index 9c08e4048c..432027f924 100644
> --- a/libavcodec/vdpau_h264.c
> +++ b/libavcodec/vdpau_h264.c
> @@ -201,7 +201,6 @@ static int vdpau_h264_decode_slice(AVCodecContext *avctx,
>  static int vdpau_h264_end_frame(AVCodecContext *avctx)
>  {
>      H264Context *h = avctx->priv_data;
> -    H264SliceContext *sl = &h->slice_ctx[0];
>      H264Picture *pic = h->cur_pic_ptr;
>      struct vdpau_picture_context *pic_ctx = pic->hwaccel_picture_private;
>      int val;
> @@ -210,7 +209,6 @@ static int vdpau_h264_end_frame(AVCodecContext *avctx)
>      if (val < 0)
>          return val;
>  
> -    ff_h264_draw_horiz_band(h, sl, 0, h->avctx->height);
>      return 0;
>  }
>  

Will apply this patch tomorrow unless there are objections.

- Andreas
Kieran Kunhya June 14, 2024, 12:41 p.m. UTC | #2 
On Sun, Jun 9, 2024 at 2:39 AM Andreas Rheinhardt
<andreas.rheinhardt@outlook.com> wrote:
>
> The H.264 decoder does not support draw_horiz_band (it does not have
> the AV_CODEC_CAP_DRAW_HORIZ_BAND), making ff_h264_draw_horiz_band()
> legally dead. The function here always calls draw_horiz_band
> in coded order, although the default case is display order.

Why would you want a low latency decode mode with reordering?
I have no idea about hwaccel, but I believe with low latency encoded
video this does work.

Kieran
Paul B Mahol June 14, 2024, 1:03 p.m. UTC | #3 
On Fri, Jun 14, 2024 at 2:41 PM Kieran Kunhya via ffmpeg-devel <
ffmpeg-devel@ffmpeg.org> wrote:

> On Sun, Jun 9, 2024 at 2:39 AM Andreas Rheinhardt
> <andreas.rheinhardt@outlook.com> wrote:
> >
> > The H.264 decoder does not support draw_horiz_band (it does not have
> > the AV_CODEC_CAP_DRAW_HORIZ_BAND), making ff_h264_draw_horiz_band()
> > legally dead. The function here always calls draw_horiz_band
> > in coded order, although the default case is display order.
>
> Why would you want a low latency decode mode with reordering?
> I have no idea about hwaccel, but I believe with low latency encoded
> video this does work.
>

Since when?

See 0da71265d84b587c7159cd82708ca60ad050dd4c from 2003.

flag remain commented out from then to this day.


> Kieran
> _______________________________________________
> 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".
>
Kieran Kunhya June 14, 2024, 1:27 p.m. UTC | #4 
On Fri, Jun 14, 2024 at 1:53 PM Paul B Mahol <onemda@gmail.com> wrote:
>
>
>
> On Fri, Jun 14, 2024 at 2:41 PM Kieran Kunhya via ffmpeg-devel <ffmpeg-devel@ffmpeg.org> wrote:
>>
>> On Sun, Jun 9, 2024 at 2:39 AM Andreas Rheinhardt
>> <andreas.rheinhardt@outlook.com> wrote:
>> >
>> > The H.264 decoder does not support draw_horiz_band (it does not have
>> > the AV_CODEC_CAP_DRAW_HORIZ_BAND), making ff_h264_draw_horiz_band()
>> > legally dead. The function here always calls draw_horiz_band
>> > in coded order, although the default case is display order.
>>
>> Why would you want a low latency decode mode with reordering?
>> I have no idea about hwaccel, but I believe with low latency encoded
>> video this does work.
>
>
> Since when?
>
> See 0da71265d84b587c7159cd82708ca60ad050dd4c from 2003.
>
> flag remain commented out from then to this day.

It still worked if you controlled the input.

Kieran
diff mbox series

Patch

diff --git a/libavcodec/d3d12va_h264.c b/libavcodec/d3d12va_h264.c
index b2fe2955c8..fe23f083df 100644
--- a/libavcodec/d3d12va_h264.c
+++ b/libavcodec/d3d12va_h264.c
@@ -163,14 +163,10 @@  static int d3d12va_h264_end_frame(AVCodecContext *avctx)
     if (ctx_pic->slice_count <= 0 || ctx_pic->bitstream_size <= 0)
         return -1;
 
-    ret = ff_d3d12va_common_end_frame(avctx, h->cur_pic_ptr->f,
-                                      &ctx_pic->pp, sizeof(ctx_pic->pp),
-                                      &ctx_pic->qm, sizeof(ctx_pic->qm),
-                                      update_input_arguments);
-    if (!ret)
-        ff_h264_draw_horiz_band(h, sl, 0, h->avctx->height);
-
-    return ret;
+    return ff_d3d12va_common_end_frame(avctx, h->cur_pic_ptr->f,
+                                       &ctx_pic->pp, sizeof(ctx_pic->pp),
+                                       &ctx_pic->qm, sizeof(ctx_pic->qm),
+                                       update_input_arguments);
 }
 
 static int d3d12va_h264_decode_init(AVCodecContext *avctx)
diff --git a/libavcodec/dxva2_h264.c b/libavcodec/dxva2_h264.c
index 0fe4152625..a6fbf61ea9 100644
--- a/libavcodec/dxva2_h264.c
+++ b/libavcodec/dxva2_h264.c
@@ -502,20 +502,15 @@  static int dxva2_h264_decode_slice(AVCodecContext *avctx,
 static int dxva2_h264_end_frame(AVCodecContext *avctx)
 {
     H264Context *h = avctx->priv_data;
-    H264SliceContext *sl = &h->slice_ctx[0];
     struct dxva2_picture_context *ctx_pic =
         h->cur_pic_ptr->hwaccel_picture_private;
-    int ret;
 
     if (ctx_pic->slice_count <= 0 || ctx_pic->bitstream_size <= 0)
         return -1;
-    ret = ff_dxva2_common_end_frame(avctx, h->cur_pic_ptr->f,
-                                    &ctx_pic->pp, sizeof(ctx_pic->pp),
-                                    &ctx_pic->qm, sizeof(ctx_pic->qm),
-                                    commit_bitstream_and_slice_buffer);
-    if (!ret)
-        ff_h264_draw_horiz_band(h, sl, 0, h->avctx->height);
-    return ret;
+    return ff_dxva2_common_end_frame(avctx, h->cur_pic_ptr->f,
+                                     &ctx_pic->pp, sizeof(ctx_pic->pp),
+                                     &ctx_pic->qm, sizeof(ctx_pic->qm),
+                                     commit_bitstream_and_slice_buffer);
 }
 
 #if CONFIG_H264_DXVA2_HWACCEL
diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
index ce2c4caca1..ceca1d1046 100644
--- a/libavcodec/h264_slice.c
+++ b/libavcodec/h264_slice.c
@@ -2523,8 +2523,6 @@  static void decode_finish_row(const H264Context *h, H264SliceContext *sl)
         top    = 0;
     }
 
-    ff_h264_draw_horiz_band(h, sl, top, height);
-
     if (h->droppable || h->er.error_occurred)
         return;
 
diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
index fd23e367b4..a09714b714 100644
--- a/libavcodec/h264dec.c
+++ b/libavcodec/h264dec.c
@@ -30,7 +30,6 @@ 
 #include "config_components.h"
 
 #include "libavutil/avassert.h"
-#include "libavutil/emms.h"
 #include "libavutil/imgutils.h"
 #include "libavutil/mem.h"
 #include "libavutil/opt.h"
@@ -100,44 +99,6 @@  static void h264_er_decode_mb(void *opaque, int ref, int mv_dir, int mv_type,
     ff_h264_hl_decode_mb(h, &h->slice_ctx[0]);
 }
 
-void ff_h264_draw_horiz_band(const H264Context *h, H264SliceContext *sl,
-                             int y, int height)
-{
-    AVCodecContext *avctx = h->avctx;
-    const AVFrame   *src  = h->cur_pic.f;
-    const AVPixFmtDescriptor *desc;
-    int offset[AV_NUM_DATA_POINTERS];
-    int vshift;
-    const int field_pic = h->picture_structure != PICT_FRAME;
-
-    if (!avctx->draw_horiz_band)
-        return;
-
-    if (field_pic && h->first_field && !(avctx->slice_flags & SLICE_FLAG_ALLOW_FIELD))
-        return;
-
-    if (field_pic) {
-        height <<= 1;
-        y      <<= 1;
-    }
-
-    height = FFMIN(height, avctx->height - y);
-
-    desc   = av_pix_fmt_desc_get(avctx->pix_fmt);
-    vshift = desc->log2_chroma_h;
-
-    offset[0] = y * src->linesize[0];
-    offset[1] =
-    offset[2] = (y >> vshift) * src->linesize[1];
-    for (int i = 3; i < AV_NUM_DATA_POINTERS; i++)
-        offset[i] = 0;
-
-    emms_c();
-
-    avctx->draw_horiz_band(avctx, src, offset,
-                           y, h->picture_structure, height);
-}
-
 void ff_h264_free_tables(H264Context *h)
 {
     int i;
diff --git a/libavcodec/h264dec.h b/libavcodec/h264dec.h
index fc50df90f2..c73c6d085a 100644
--- a/libavcodec/h264dec.h
+++ b/libavcodec/h264dec.h
@@ -677,8 +677,6 @@  void ff_h264_unref_picture(H264Picture *pic);
 
 void ff_h264_slice_context_init(H264Context *h, H264SliceContext *sl);
 
-void ff_h264_draw_horiz_band(const H264Context *h, H264SliceContext *sl, int y, int height);
-
 /**
  * Submit a slice for decoding.
  *
diff --git a/libavcodec/vaapi_h264.c b/libavcodec/vaapi_h264.c
index 398e92568c..670d3273cb 100644
--- a/libavcodec/vaapi_h264.c
+++ b/libavcodec/vaapi_h264.c
@@ -319,17 +319,8 @@  static int vaapi_h264_end_frame(AVCodecContext *avctx)
 {
     const H264Context *h = avctx->priv_data;
     VAAPIDecodePicture *pic = h->cur_pic_ptr->hwaccel_picture_private;
-    H264SliceContext *sl = &h->slice_ctx[0];
-    int ret;
 
-    ret = ff_vaapi_decode_issue(avctx, pic);
-    if (ret < 0)
-        goto finish;
-
-    ff_h264_draw_horiz_band(h, sl, 0, h->avctx->height);
-
-finish:
-    return ret;
+    return ff_vaapi_decode_issue(avctx, pic);
 }
 
 /** Decode the given H.264 slice with VA API. */
diff --git a/libavcodec/vdpau_h264.c b/libavcodec/vdpau_h264.c
index 9c08e4048c..432027f924 100644
--- a/libavcodec/vdpau_h264.c
+++ b/libavcodec/vdpau_h264.c
@@ -201,7 +201,6 @@  static int vdpau_h264_decode_slice(AVCodecContext *avctx,
 static int vdpau_h264_end_frame(AVCodecContext *avctx)
 {
     H264Context *h = avctx->priv_data;
-    H264SliceContext *sl = &h->slice_ctx[0];
     H264Picture *pic = h->cur_pic_ptr;
     struct vdpau_picture_context *pic_ctx = pic->hwaccel_picture_private;
     int val;
@@ -210,7 +209,6 @@  static int vdpau_h264_end_frame(AVCodecContext *avctx)
     if (val < 0)
         return val;
 
-    ff_h264_draw_horiz_band(h, sl, 0, h->avctx->height);
     return 0;
 }