[FFmpeg-devel] [PATCH 2/5] avcodec/mjpegdec: Don't create unnecessary AVFrame reference

Andreas Rheinhardt andreas.rheinhardt at outlook.com
Thu Apr 14 19:51:57 EEST 2022


James Almer:
> 
> 
> On 4/14/2022 1:43 PM, Andreas Rheinhardt wrote:
>> James Almer:
>>> On 4/14/2022 12:57 PM, Andreas Rheinhardt wrote:
>>>> MJPEG is an intra-codec, so it makes no sense to keep the reference.
>>>> It is basically unused lateron anyway.
>>>
>>> Wouldn't this mean you can remove the AV_GET_BUFFER_FLAG_REF flag from
>>> the ff_get_buffer() call?
>>>
>>
>> Not in case of SMV. The smv code keeps a reference and reuses it. See
>> smv_process_frame.
> 
> Make it AV_GET_BUFFER_FLAG_REF * (s->avctx->codec_id ==
> AV_CODEC_ID_SMVJPEG) then.
> 

Not worth the branch.

>>
>>>>
>>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
>>>> ---
>>>>    libavcodec/mjpegdec.c | 45
>>>> +++++++++++++++++++++----------------------
>>>>    1 file changed, 22 insertions(+), 23 deletions(-)
>>>>
>>>> diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
>>>> index 0e76bf4c26..939dedce33 100644
>>>> --- a/libavcodec/mjpegdec.c
>>>> +++ b/libavcodec/mjpegdec.c
>>>> @@ -2589,8 +2589,7 @@ eoi_parser:
>>>>                      av_freep(&s->hwaccel_picture_private);
>>>>                }
>>>> -            if ((ret = av_frame_ref(frame, s->picture_ptr)) < 0)
>>>> -                return ret;
>>>> +            av_frame_move_ref(frame, s->picture_ptr);
>>>>                s->got_picture = 0;
>>>>                  frame->pkt_dts = s->pkt->dts;
>>>> @@ -2675,9 +2674,9 @@ the_end:
>>>>            if (ret)
>>>>                return ret;
>>>>    -        av_assert0(s->nb_components ==
>>>> av_pix_fmt_count_planes(s->picture_ptr->format));
>>>> +        av_assert0(s->nb_components ==
>>>> av_pix_fmt_count_planes(frame->format));
>>>>            for (p = 0; p<s->nb_components; p++) {
>>>> -            uint8_t *line = s->picture_ptr->data[p];
>>>> +            uint8_t *line = frame->data[p];
>>>>                int w = s->width;
>>>>                int h = s->height;
>>>>                if (!s->upscale_h[p])
>>>> @@ -2737,7 +2736,7 @@ the_end:
>>>>            if (ret)
>>>>                return ret;
>>>>    -        av_assert0(s->nb_components ==
>>>> av_pix_fmt_count_planes(s->picture_ptr->format));
>>>> +        av_assert0(s->nb_components ==
>>>> av_pix_fmt_count_planes(frame->format));
>>>>            for (p = 0; p < s->nb_components; p++) {
>>>>                uint8_t *dst;
>>>>                int w = s->width;
>>>> @@ -2748,10 +2747,10 @@ the_end:
>>>>                    w = AV_CEIL_RSHIFT(w, hshift);
>>>>                    h = AV_CEIL_RSHIFT(h, vshift);
>>>>                }
>>>> -            dst = &((uint8_t *)s->picture_ptr->data[p])[(h - 1) *
>>>> s->linesize[p]];
>>>> +            dst = &frame->data[p][(h - 1) * s->linesize[p]];
>>>>                for (i = h - 1; i; i--) {
>>>> -                uint8_t *src1 = &((uint8_t
>>>> *)s->picture_ptr->data[p])[i * s->upscale_v[p] / (s->upscale_v[p] + 1)
>>>> * s->linesize[p]];
>>>> -                uint8_t *src2 = &((uint8_t
>>>> *)s->picture_ptr->data[p])[(i + 1) * s->upscale_v[p] /
>>>> (s->upscale_v[p] + 1) * s->linesize[p]];
>>>> +                uint8_t *src1 = &frame->data[p][ i      *
>>>> s->upscale_v[p] / (s->upscale_v[p] + 1) * s->linesize[p]];
>>>> +                uint8_t *src2 = &frame->data[p][(i + 1) *
>>>> s->upscale_v[p] / (s->upscale_v[p] + 1) * s->linesize[p]];
>>>>                    if (s->upscale_v[p] != 2 && (src1 == src2 || i == h
>>>> - 1)) {
>>>>                        memcpy(dst, src1, w);
>>>>                    } else {
>>>> @@ -2768,36 +2767,36 @@ the_end:
>>>>            if (ret)
>>>>                return ret;
>>>>    -        av_assert0(s->nb_components ==
>>>> av_pix_fmt_count_planes(s->picture_ptr->format));
>>>> +        av_assert0(s->nb_components ==
>>>> av_pix_fmt_count_planes(frame->format));
>>>>            for (index=0; index<s->nb_components; index++) {
>>>> -            uint8_t *dst = s->picture_ptr->data[index];
>>>> -            int w = s->picture_ptr->width;
>>>> -            int h = s->picture_ptr->height;
>>>> +            uint8_t *dst = frame->data[index];
>>>> +            int w = frame->width;
>>>> +            int h = frame->height;
>>>>                if(index && index<3){
>>>>                    w = AV_CEIL_RSHIFT(w, hshift);
>>>>                    h = AV_CEIL_RSHIFT(h, vshift);
>>>>                }
>>>>                if(dst){
>>>> -                uint8_t *dst2 = dst +
>>>> s->picture_ptr->linesize[index]*(h-1);
>>>> +                uint8_t *dst2 = dst + frame->linesize[index]*(h-1);
>>>>                    for (i=0; i<h/2; i++) {
>>>>                        for (j=0; j<w; j++)
>>>>                            FFSWAP(int, dst[j], dst2[j]);
>>>> -                    dst  += s->picture_ptr->linesize[index];
>>>> -                    dst2 -= s->picture_ptr->linesize[index];
>>>> +                    dst  += frame->linesize[index];
>>>> +                    dst2 -= frame->linesize[index];
>>>>                    }
>>>>                }
>>>>            }
>>>>        }
>>>>        if (s->adobe_transform == 0 && s->avctx->pix_fmt ==
>>>> AV_PIX_FMT_GBRAP) {
>>>> -        int w = s->picture_ptr->width;
>>>> -        int h = s->picture_ptr->height;
>>>> +        int w = frame->width;
>>>> +        int h = frame->height;
>>>>            av_assert0(s->nb_components == 4);
>>>>            for (i=0; i<h; i++) {
>>>>                int j;
>>>>                uint8_t *dst[4];
>>>>                for (index=0; index<4; index++) {
>>>> -                dst[index] =   s->picture_ptr->data[index]
>>>> -                             + s->picture_ptr->linesize[index]*i;
>>>> +                dst[index] =   frame->data[index]
>>>> +                             + frame->linesize[index]*i;
>>>>                }
>>>>                for (j=0; j<w; j++) {
>>>>                    int k = dst[3][j];
>>>> @@ -2812,15 +2811,15 @@ the_end:
>>>>            }
>>>>        }
>>>>        if (s->adobe_transform == 2 && s->avctx->pix_fmt ==
>>>> AV_PIX_FMT_YUVA444P) {
>>>> -        int w = s->picture_ptr->width;
>>>> -        int h = s->picture_ptr->height;
>>>> +        int w = frame->width;
>>>> +        int h = frame->height;
>>>>            av_assert0(s->nb_components == 4);
>>>>            for (i=0; i<h; i++) {
>>>>                int j;
>>>>                uint8_t *dst[4];
>>>>                for (index=0; index<4; index++) {
>>>> -                dst[index] =   s->picture_ptr->data[index]
>>>> -                             + s->picture_ptr->linesize[index]*i;
>>>> +                dst[index] =   frame->data[index]
>>>> +                             + frame->linesize[index]*i;
>>>>                }
>>>>                for (j=0; j<w; j++) {
>>>>                    int k = dst[3][j];
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".



More information about the ffmpeg-devel mailing list