On 3/7/2024 9:18 AM, Anton Khirnov wrote:
Quoting Andreas Rheinhardt (2024-03-07 12:19:28)
Anton Khirnov:
Quoting Andreas Rheinhardt (2024-03-04 14:36:09)
Anton Khirnov:
From: Niklas Haas <g...@haasn.dev>
For consistency, even though this cannot be overriden at the packet
level.
---
libavcodec/mpeg12dec.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
index 3a2f17e508..aa116336dd 100644
--- a/libavcodec/mpeg12dec.c
+++ b/libavcodec/mpeg12dec.c
@@ -2531,15 +2531,17 @@ static int mpeg_decode_frame(AVCodecContext *avctx,
AVFrame *picture,
if (s->timecode_frame_start != -1 && *got_output) {
char tcbuf[AV_TIMECODE_STR_SIZE];
- AVFrameSideData *tcside = av_frame_new_side_data(picture,
-
AV_FRAME_DATA_GOP_TIMECODE,
- sizeof(int64_t));
- if (!tcside)
- return AVERROR(ENOMEM);
- memcpy(tcside->data, &s->timecode_frame_start, sizeof(int64_t));
+ AVFrameSideData *tcside;
+ ret = ff_frame_new_side_data(avctx, picture,
AV_FRAME_DATA_GOP_TIMECODE,
+ sizeof(int64_t), &tcside);
+ if (ret < 0)
+ return ret;
+ if (tcside) {
+ memcpy(tcside->data, &s->timecode_frame_start,
sizeof(int64_t));
- av_timecode_make_mpeg_tc_string(tcbuf, s->timecode_frame_start);
- av_dict_set(&picture->metadata, "timecode", tcbuf, 0);
+ av_timecode_make_mpeg_tc_string(tcbuf,
s->timecode_frame_start);
+ av_dict_set(&picture->metadata, "timecode", tcbuf, 0);
+ }
s->timecode_frame_start = -1;
}
-1 to everything that is only done for consistency.
I prefer consistency here, otherwise the decoder authors have to choose
which function to use, and they are often not aware of the precise
implications of thise choice. Better to always use just one function.
It adds unnecessary checks and given that internal API is updated more
frequently it is likely to lead to unnecessary further changes lateron.
Furthermore, mjpeg still emits an allocation failure error message
without knowing whether it was really allocation failure.
"Could not allocate frame side data" seems appropriate to me, it really
is what happened, whatever the actual reason is.
Finally, if we really believed decoder authors to be that uninformed, we
should remove ff_get_buffer() from decoders altogether and only use the
ff_thread_get_buffer() wrapper.
I'd be in favor, fewer functions is better.
I wouldn't. It adds an extra layer of abstraction for no real gain. And
the name in this case is very specific and self explanatory in how it's
different from the alternative.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
