On Fri, May 22, 2020 at 04:51:05PM -0300, James Almer wrote:
> On 5/22/2020 4:22 PM, Michael Niedermayer wrote:
> > On Fri, May 22, 2020 at 02:32:07PM -0300, James Almer wrote:
> >> On 5/22/2020 1:56 PM, Anton Khirnov wrote:
> >>> ---
> >>> libavcodec/decode.c | 8 ++++----
> >>> 1 file changed, 4 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> >>> index f3327d74af..dbecdf3f46 100644
> >>> --- a/libavcodec/decode.c
> >>> +++ b/libavcodec/decode.c
> >>> @@ -586,6 +586,7 @@ static int
> >>> decode_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame)
> >>> int attribute_align_arg avcodec_send_packet(AVCodecContext *avctx, const
> >>> AVPacket *avpkt)
> >>> {
> >>> AVCodecInternal *avci = avctx->internal;
> >>> + AVPacket buffer_pkt = { NULL };
> >>> int ret;
> >>>
> >>> if (!avcodec_is_open(avctx) || !av_codec_is_decoder(avctx->codec))
> >>> @@ -597,16 +598,15 @@ int attribute_align_arg
> >>> avcodec_send_packet(AVCodecContext *avctx, const AVPacke
> >>> if (avpkt && !avpkt->size && avpkt->data)
> >>> return AVERROR(EINVAL);
> >>>
> >>> - av_packet_unref(avci->buffer_pkt);
> >>> if (avpkt && (avpkt->data || avpkt->side_data_elems)) {
> >>> - ret = av_packet_ref(avci->buffer_pkt, avpkt);
> >>> + ret = av_packet_ref(&buffer_pkt, avpkt);
> >>> if (ret < 0)
> >>> return ret;
> >>> }
> >>>
> >>> - ret = av_bsf_send_packet(avci->bsf, avci->buffer_pkt);
> >>> + ret = av_bsf_send_packet(avci->bsf, &buffer_pkt);
> >>> if (ret < 0) {
> >>> - av_packet_unref(avci->buffer_pkt);
> >>> + av_packet_unref(&buffer_pkt);
> >>> return ret;
> >>> }
> >>
> >> What's the gain here? You're not removing the context variable since
> >> it's used in the encode framework as well, and you're introducing a
> >
> >> stack AVPacket (that, while harmless, is not even properly initialized).
> >
> > It would be nice if we could avoid all such code, so that we can extend
> > AVPacket like other structs without Major bumping
>
> Yes, some relatively recent commits went in the direction of removing as
> much AVPacket on stack/heap usage as possible precisely to remove
> sizeof(AVPacket) from the ABI, but at least within lavc it's not a problem.sizeof() may be ok in libavcodec (though still IMHO not great style) the other issue is NULL initialization. We may want to add new fields which need to be initialized to a non 0 default. timestamps and AV_NOPTS_VALUE come to mind as examples of such fields thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB It is what and why we do it that matters, not just one of them.
signature.asc
Description: PGP signature
_______________________________________________ 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".
