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.
--
Anton Khirnov
_______________________________________________
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".
