Re: [FFmpeg-devel] [PATCH 23/29] avcodec/mpeg12dec: use ff_frame_new_side_data

[Anton Khirnov <anton@khirnov.net>]

Quoting Andreas Rheinhardt (2024-03-04 14:36:09)
> Anton Khirnov:
> > From: Niklas Haas <git@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.

-- 
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-request@ffmpeg.org with subject "unsubscribe".