diff mbox series

[FFmpeg-devel,23/38] avcodec/hevcdec: respect side data preference

Message ID 20240223143115.16521-24-anton@khirnov.net
State New
Headers show
Series [FFmpeg-devel,01/38] lavu/opt: cosmetics, change option flags to (1 << N) style | expand

Checks

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

Commit Message

Anton Khirnov Feb. 23, 2024, 1:58 p.m. UTC
From: Niklas Haas <git@haasn.dev>

If the time code side data is overridden by the packet level, we also
make sure not to update `out->metadata` to a mismatched timecode.

For HDR side data, we unfortunately need to omit a return check because
the new function does not distinguish between OOM and overridden side
data.
---
 libavcodec/hevcdec.c | 37 +++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 18 deletions(-)

Comments

Marton Balint Feb. 23, 2024, 7:11 p.m. UTC | #1
On Fri, 23 Feb 2024, Anton Khirnov wrote:

> From: Niklas Haas <git@haasn.dev>
>
> If the time code side data is overridden by the packet level, we also
> make sure not to update `out->metadata` to a mismatched timecode.
>
> For HDR side data, we unfortunately need to omit a return check because
> the new function does not distinguish between OOM and overridden side
> data.

I don't think this is acceptable. If the API is not capable 
of differentiating between OOM and overridden side data, then 
the API should be fixed.

Regards,
Marton
diff mbox series

Patch

diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
index 17c6a9212f..0cb2577b39 100644
--- a/libavcodec/hevcdec.c
+++ b/libavcodec/hevcdec.c
@@ -2788,24 +2788,27 @@  static int set_side_data(HEVCContext *s)
     if (s->sei.timecode.present) {
         uint32_t *tc_sd;
         char tcbuf[AV_TIMECODE_STR_SIZE];
-        AVFrameSideData *tcside = av_frame_new_side_data(out, AV_FRAME_DATA_S12M_TIMECODE,
-                                                         sizeof(uint32_t) * 4);
-        if (!tcside)
-            return AVERROR(ENOMEM);
+        AVFrameSideData *tcside;
+        ret = ff_frame_new_side_data(s->avctx, out, AV_FRAME_DATA_S12M_TIMECODE,
+                                     sizeof(uint32_t) * 4, &tcside);
+        if (ret < 0)
+            return ret;
 
-        tc_sd = (uint32_t*)tcside->data;
-        tc_sd[0] = s->sei.timecode.num_clock_ts;
+        if (tcside) {
+            tc_sd = (uint32_t*)tcside->data;
+            tc_sd[0] = s->sei.timecode.num_clock_ts;
 
-        for (int i = 0; i < tc_sd[0]; i++) {
-            int drop = s->sei.timecode.cnt_dropped_flag[i];
-            int   hh = s->sei.timecode.hours_value[i];
-            int   mm = s->sei.timecode.minutes_value[i];
-            int   ss = s->sei.timecode.seconds_value[i];
-            int   ff = s->sei.timecode.n_frames[i];
+            for (int i = 0; i < tc_sd[0]; i++) {
+                int drop = s->sei.timecode.cnt_dropped_flag[i];
+                int   hh = s->sei.timecode.hours_value[i];
+                int   mm = s->sei.timecode.minutes_value[i];
+                int   ss = s->sei.timecode.seconds_value[i];
+                int   ff = s->sei.timecode.n_frames[i];
 
-            tc_sd[i + 1] = av_timecode_get_smpte(s->avctx->framerate, drop, hh, mm, ss, ff);
-            av_timecode_make_smpte_tc_string2(tcbuf, s->avctx->framerate, tc_sd[i + 1], 0, 0);
-            av_dict_set(&out->metadata, "timecode", tcbuf, 0);
+                tc_sd[i + 1] = av_timecode_get_smpte(s->avctx->framerate, drop, hh, mm, ss, ff);
+                av_timecode_make_smpte_tc_string2(tcbuf, s->avctx->framerate, tc_sd[i + 1], 0, 0);
+                av_dict_set(&out->metadata, "timecode", tcbuf, 0);
+            }
         }
 
         s->sei.timecode.num_clock_ts = 0;
@@ -2816,10 +2819,8 @@  static int set_side_data(HEVCContext *s)
         if (!info_ref)
             return AVERROR(ENOMEM);
 
-        if (!av_frame_new_side_data_from_buf(out, AV_FRAME_DATA_DYNAMIC_HDR_PLUS, info_ref)) {
+        if (!ff_frame_new_side_data_from_buf(s->avctx, out, AV_FRAME_DATA_DYNAMIC_HDR_PLUS, info_ref))
             av_buffer_unref(&info_ref);
-            return AVERROR(ENOMEM);
-        }
     }
 
     if (s->rpu_buf) {