diff mbox

[FFmpeg-devel,12/12] cbs_h264, h264_metadata: Deleting SEI messages never fails

Message ID 20190707231402.46388-2-andreas.rheinhardt@gmail.com
State Accepted
Commit d9418aba66e7f9d32c11d0ee1b8cddaf1e68e1b6
Headers show

Commit Message

Andreas Rheinhardt July 7, 2019, 11:14 p.m. UTC 
Given the recent changes to ff_cbs_delete_unit, it is no longer sensible
to use a return value for ff_cbs_h264_delete_sei_message; instead, use
asserts to ensure that the required conditions are met and remove the
callers' checks for the return value. Also, document said conditions.

An assert that is essentially equivalent to the one used in
ff_cbs_delete_unit has been removed, too.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavcodec/cbs_h264.h          | 11 +++++++----
 libavcodec/cbs_h2645.c         | 11 ++++-------
 libavcodec/h264_metadata_bsf.c | 21 +++++----------------
 3 files changed, 16 insertions(+), 27 deletions(-)

Comments

Mark Thompson July 8, 2019, 10:23 p.m. UTC | #1 
On 08/07/2019 00:14, Andreas Rheinhardt wrote:
> ff_cbs_delete_unit never fails if the index of the unit to delete is
> valid, as it is with all current callers of the function. So just assert
> in ff_cbs_delete_unit that the index is valid and change the return
> value to void in order to remove the callers' checks for whether
> ff_cbs_delete_unit failed.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
>  libavcodec/av1_metadata_bsf.c       |  9 ++-------
>  libavcodec/cbs.c                    | 12 +++++-------
>  libavcodec/cbs.h                    |  8 +++++---
>  libavcodec/cbs_h2645.c              |  6 +++---
>  libavcodec/h264_metadata_bsf.c      |  8 +-------
>  libavcodec/h264_redundant_pps_bsf.c |  4 +---
>  6 files changed, 17 insertions(+), 30 deletions(-)
> 
On 08/07/2019 00:14, Andreas Rheinhardt wrote:
> Given the recent changes to ff_cbs_delete_unit, it is no longer sensible
> to use a return value for ff_cbs_h264_delete_sei_message; instead, use
> asserts to ensure that the required conditions are met and remove the
> callers' checks for the return value. Also, document said conditions.
> 
> An assert that is essentially equivalent to the one used in
> ff_cbs_delete_unit has been removed, too.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
>  libavcodec/cbs_h264.h          | 11 +++++++----
>  libavcodec/cbs_h2645.c         | 11 ++++-------
>  libavcodec/h264_metadata_bsf.c | 21 +++++----------------
>  3 files changed, 16 insertions(+), 27 deletions(-)

LGTM; both applied.

Thanks,

- Mark
diff mbox

Patch

diff --git a/libavcodec/cbs_h264.h b/libavcodec/cbs_h264.h
index a31be298ba..b39e7480c9 100644
--- a/libavcodec/cbs_h264.h
+++ b/libavcodec/cbs_h264.h
@@ -478,10 +478,13 @@  int ff_cbs_h264_add_sei_message(CodedBitstreamContext *ctx,
  *
  * Deletes from nal_unit, which must be an SEI NAL unit.  If this is the
  * last message in nal_unit, also deletes it from access_unit.
+ *
+ * Requires nal_unit to be a unit in access_unit and position to be >= 0
+ * and < the payload count of the SEI nal_unit.
  */
-int ff_cbs_h264_delete_sei_message(CodedBitstreamContext *ctx,
-                                   CodedBitstreamFragment *access_unit,
-                                   CodedBitstreamUnit *nal_unit,
-                                   int position);
+void ff_cbs_h264_delete_sei_message(CodedBitstreamContext *ctx,
+                                    CodedBitstreamFragment *access_unit,
+                                    CodedBitstreamUnit *nal_unit,
+                                    int position);
 
 #endif /* AVCODEC_CBS_H264_H */
diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
index 484b145852..b286269913 100644
--- a/libavcodec/cbs_h2645.c
+++ b/libavcodec/cbs_h2645.c
@@ -1644,10 +1644,10 @@  int ff_cbs_h264_add_sei_message(CodedBitstreamContext *ctx,
     return 0;
 }
 
-int ff_cbs_h264_delete_sei_message(CodedBitstreamContext *ctx,
-                                   CodedBitstreamFragment *au,
-                                   CodedBitstreamUnit *nal,
-                                   int position)
+void ff_cbs_h264_delete_sei_message(CodedBitstreamContext *ctx,
+                                    CodedBitstreamFragment *au,
+                                    CodedBitstreamUnit *nal,
+                                    int position)
 {
     H264RawSEI *sei = nal->content;
 
@@ -1662,7 +1662,6 @@  int ff_cbs_h264_delete_sei_message(CodedBitstreamContext *ctx,
             if (&au->units[i] == nal)
                 break;
         }
-        av_assert0(i < au->nb_units && "NAL unit not in access unit.");
 
         ff_cbs_delete_unit(ctx, au, i);
     } else {
@@ -1673,6 +1672,4 @@  int ff_cbs_h264_delete_sei_message(CodedBitstreamContext *ctx,
                 sei->payload + position + 1,
                 (sei->payload_count - position) * sizeof(*sei->payload));
     }
-
-    return 0;
 }
diff --git a/libavcodec/h264_metadata_bsf.c b/libavcodec/h264_metadata_bsf.c
index e40baa3371..1c1c340d8f 100644
--- a/libavcodec/h264_metadata_bsf.c
+++ b/libavcodec/h264_metadata_bsf.c
@@ -437,15 +437,9 @@  static int h264_metadata_filter(AVBSFContext *bsf, AVPacket *pkt)
 
                 for (j = sei->payload_count - 1; j >= 0; j--) {
                     if (sei->payload[j].payload_type ==
-                        H264_SEI_TYPE_FILLER_PAYLOAD) {
-                        err = ff_cbs_h264_delete_sei_message(ctx->cbc, au,
-                                                             &au->units[i], j);
-                        if (err < 0) {
-                            av_log(bsf, AV_LOG_ERROR, "Failed to delete "
-                                   "filler SEI message.\n");
-                            goto fail;
-                        }
-                    }
+                        H264_SEI_TYPE_FILLER_PAYLOAD)
+                        ff_cbs_h264_delete_sei_message(ctx->cbc, au,
+                                                       &au->units[i], j);
                 }
             }
         }
@@ -469,13 +463,8 @@  static int h264_metadata_filter(AVBSFContext *bsf, AVPacket *pkt)
 
                 if (ctx->display_orientation == REMOVE ||
                     ctx->display_orientation == INSERT) {
-                    err = ff_cbs_h264_delete_sei_message(ctx->cbc, au,
-                                                         &au->units[i], j);
-                    if (err < 0) {
-                        av_log(bsf, AV_LOG_ERROR, "Failed to delete "
-                               "display orientation SEI message.\n");
-                        goto fail;
-                    }
+                    ff_cbs_h264_delete_sei_message(ctx->cbc, au,
+                                                   &au->units[i], j);
                     continue;
                 }