Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH 1/5] avcodec/mjpegdec: Always reset got_picture at the beginnig of decoding
@ 2022-04-14 15:56 Andreas Rheinhardt
  2022-04-14 15:57 ` [FFmpeg-devel] [PATCH 2/5] avcodec/mjpegdec: Don't create unnecessary AVFrame reference Andreas Rheinhardt
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Andreas Rheinhardt @ 2022-04-14 15:56 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Andreas Rheinhardt

Said field is set when parsing a SOF; yet a picture is only allocated
if skip_frame is != AVDISCARD_ALL. This leads to a crash in the
following case: If a jpeg is split into two parts, the first containing
everything before the scans including the SOF and the second part
containing the rest, and the first part is sent to the decoder with
skip_frame set to AVDISCARD_ALL, got_picture is set, yet no picture
is allocated. If the next part is sent with skip_frame set to
AVDISCARD_NONE, the code presumes that a picture has been allocated,
although it hasn't leading to segfaults.

Fix this by resetting got_picture at the beginning of decoding.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
This patch presumes that there is not use-case for partitioning
the data corresponding to a single AVFrame accross multiple packets.
I am not certain whether this is actually true, in particular
wrt interlaced input where it might be common to put the data for
one field into one packet.
Anyway, no such use is covered by FATE.

 libavcodec/mjpegdec.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
index 32874a5a19..0e76bf4c26 100644
--- a/libavcodec/mjpegdec.c
+++ b/libavcodec/mjpegdec.c
@@ -2419,6 +2419,7 @@ int ff_mjpeg_receive_frame(AVCodecContext *avctx, AVFrame *frame)
     av_dict_free(&s->exif_metadata);
     av_freep(&s->stereo3d);
     s->adobe_transform = -1;
+    s->got_picture = 0;
 
     if (s->iccnum != 0)
         reset_icc_profile(s);
@@ -2578,7 +2579,6 @@ eoi_parser:
                     break;
             }
             if (avctx->skip_frame == AVDISCARD_ALL) {
-                s->got_picture = 0;
                 ret = AVERROR(EAGAIN);
                 goto the_end_no_picture;
             }
@@ -2651,7 +2651,6 @@ skip:
     av_log(avctx, AV_LOG_FATAL, "No JPEG data found in image\n");
     return AVERROR_INVALIDDATA;
 fail:
-    s->got_picture = 0;
     return ret;
 the_end:
 
@@ -2987,10 +2986,9 @@ av_cold int ff_mjpeg_decode_end(AVCodecContext *avctx)
     return 0;
 }
 
-static void decode_flush(AVCodecContext *avctx)
+static void smv_decode_flush(AVCodecContext *avctx)
 {
     MJpegDecodeContext *s = avctx->priv_data;
-    s->got_picture = 0;
 
     s->smv_next_frame = 0;
     av_frame_unref(s->smv_frame);
@@ -3021,7 +3019,6 @@ const FFCodec ff_mjpeg_decoder = {
     .init           = ff_mjpeg_decode_init,
     .close          = ff_mjpeg_decode_end,
     FF_CODEC_RECEIVE_FRAME_CB(ff_mjpeg_receive_frame),
-    .flush          = decode_flush,
     .p.capabilities = AV_CODEC_CAP_DR1,
     .p.max_lowres   = 3,
     .p.priv_class   = &mjpegdec_class,
@@ -3049,7 +3046,6 @@ const FFCodec ff_thp_decoder = {
     .init           = ff_mjpeg_decode_init,
     .close          = ff_mjpeg_decode_end,
     FF_CODEC_RECEIVE_FRAME_CB(ff_mjpeg_receive_frame),
-    .flush          = decode_flush,
     .p.capabilities = AV_CODEC_CAP_DR1,
     .p.max_lowres   = 3,
     .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE | FF_CODEC_CAP_INIT_CLEANUP |
@@ -3067,7 +3063,7 @@ const FFCodec ff_smvjpeg_decoder = {
     .init           = ff_mjpeg_decode_init,
     .close          = ff_mjpeg_decode_end,
     FF_CODEC_RECEIVE_FRAME_CB(ff_mjpeg_receive_frame),
-    .flush          = decode_flush,
+    .flush          = smv_decode_flush,
     .p.capabilities = AV_CODEC_CAP_DR1,
     .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE | FF_CODEC_CAP_EXPORTS_CROPPING |
                       FF_CODEC_CAP_SETS_PKT_DTS | FF_CODEC_CAP_INIT_CLEANUP,
-- 
2.32.0

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [FFmpeg-devel] [PATCH 2/5] avcodec/mjpegdec: Don't create unnecessary AVFrame reference
  2022-04-14 15:56 [FFmpeg-devel] [PATCH 1/5] avcodec/mjpegdec: Always reset got_picture at the beginnig of decoding Andreas Rheinhardt
@ 2022-04-14 15:57 ` Andreas Rheinhardt
  2022-04-14 16:39   ` James Almer
  2022-04-14 15:57 ` [FFmpeg-devel] [PATCH 3/5] avcodec/mjpegdec: Avoid copying data when flipping image Andreas Rheinhardt
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Andreas Rheinhardt @ 2022-04-14 15:57 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Andreas Rheinhardt

MJPEG is an intra-codec, so it makes no sense to keep the reference.
It is basically unused lateron anyway.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@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];
-- 
2.32.0

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [FFmpeg-devel] [PATCH 3/5] avcodec/mjpegdec: Avoid copying data when flipping image
  2022-04-14 15:56 [FFmpeg-devel] [PATCH 1/5] avcodec/mjpegdec: Always reset got_picture at the beginnig of decoding Andreas Rheinhardt
  2022-04-14 15:57 ` [FFmpeg-devel] [PATCH 2/5] avcodec/mjpegdec: Don't create unnecessary AVFrame reference Andreas Rheinhardt
@ 2022-04-14 15:57 ` Andreas Rheinhardt
  2022-04-17 12:57   ` Michael Niedermayer
  2022-04-14 15:57 ` [FFmpeg-devel] [PATCH 4/5] avcodec/mjpegbdec: Don't create unnecessary AVFrame reference Andreas Rheinhardt
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Andreas Rheinhardt @ 2022-04-14 15:57 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Andreas Rheinhardt

Basically reverts af15c17daa5d8d2940c0263ba4d3ecec761c11ee.
Flipping a picture by modifying the pointers is so common
that even users of direct rendering should take it into account.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/mjpegdec.c | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
index 939dedce33..33beda12ea 100644
--- a/libavcodec/mjpegdec.c
+++ b/libavcodec/mjpegdec.c
@@ -2762,28 +2762,18 @@ the_end:
         }
     }
     if (s->flipped && !s->rgb) {
-        int j;
         ret = av_pix_fmt_get_chroma_sub_sample(s->avctx->pix_fmt, &hshift, &vshift);
         if (ret)
             return ret;
 
         av_assert0(s->nb_components == av_pix_fmt_count_planes(frame->format));
         for (index=0; index<s->nb_components; index++) {
-            uint8_t *dst = frame->data[index];
-            int w = frame->width;
             int h = frame->height;
-            if(index && index<3){
-                w = AV_CEIL_RSHIFT(w, hshift);
+            if (index && index < 3)
                 h = AV_CEIL_RSHIFT(h, vshift);
-            }
-            if(dst){
-                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  += frame->linesize[index];
-                    dst2 -= frame->linesize[index];
-                }
+            if (frame->data[index]) {
+                frame->data[index]     += (h - 1) * frame->linesize[index];
+                frame->linesize[index] *= -1;
             }
         }
     }
-- 
2.32.0

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [FFmpeg-devel] [PATCH 4/5] avcodec/mjpegbdec: Don't create unnecessary AVFrame reference
  2022-04-14 15:56 [FFmpeg-devel] [PATCH 1/5] avcodec/mjpegdec: Always reset got_picture at the beginnig of decoding Andreas Rheinhardt
  2022-04-14 15:57 ` [FFmpeg-devel] [PATCH 2/5] avcodec/mjpegdec: Don't create unnecessary AVFrame reference Andreas Rheinhardt
  2022-04-14 15:57 ` [FFmpeg-devel] [PATCH 3/5] avcodec/mjpegdec: Avoid copying data when flipping image Andreas Rheinhardt
@ 2022-04-14 15:57 ` Andreas Rheinhardt
  2022-04-14 15:57 ` [FFmpeg-devel] [PATCH 5/5] avcodec/mjpegbdec: Don't use GetBit-API for byte-aligned reads Andreas Rheinhardt
  2022-04-15 22:15 ` [FFmpeg-devel] [PATCH 1/5] avcodec/mjpegdec: Always reset got_picture at the beginnig of decoding Michael Niedermayer
  4 siblings, 0 replies; 11+ messages in thread
From: Andreas Rheinhardt @ 2022-04-14 15:57 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Andreas Rheinhardt

MJPEG-B is an intra-codec, so it makes no sense to keep the reference.
It is unused lateron anyway.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/mjpegbdec.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavcodec/mjpegbdec.c b/libavcodec/mjpegbdec.c
index 8c1809e5fb..46b9eb377e 100644
--- a/libavcodec/mjpegbdec.c
+++ b/libavcodec/mjpegbdec.c
@@ -144,8 +144,8 @@ read_header:
         return buf_size;
     }
 
-    if ((ret = av_frame_ref(rframe, s->picture_ptr)) < 0)
-        return ret;
+    av_frame_move_ref(rframe, s->picture_ptr);
+    s->got_picture = 0;
     *got_frame = 1;
 
     if (!s->lossless && avctx->debug & FF_DEBUG_QP) {
-- 
2.32.0

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [FFmpeg-devel] [PATCH 5/5] avcodec/mjpegbdec: Don't use GetBit-API for byte-aligned reads
  2022-04-14 15:56 [FFmpeg-devel] [PATCH 1/5] avcodec/mjpegdec: Always reset got_picture at the beginnig of decoding Andreas Rheinhardt
                   ` (2 preceding siblings ...)
  2022-04-14 15:57 ` [FFmpeg-devel] [PATCH 4/5] avcodec/mjpegbdec: Don't create unnecessary AVFrame reference Andreas Rheinhardt
@ 2022-04-14 15:57 ` Andreas Rheinhardt
  2022-04-15 22:15 ` [FFmpeg-devel] [PATCH 1/5] avcodec/mjpegdec: Always reset got_picture at the beginnig of decoding Michael Niedermayer
  4 siblings, 0 replies; 11+ messages in thread
From: Andreas Rheinhardt @ 2022-04-14 15:57 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Andreas Rheinhardt

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/mjpegbdec.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/libavcodec/mjpegbdec.c b/libavcodec/mjpegbdec.c
index 46b9eb377e..50fff2562b 100644
--- a/libavcodec/mjpegbdec.c
+++ b/libavcodec/mjpegbdec.c
@@ -27,12 +27,15 @@
 #include <inttypes.h>
 
 #include "avcodec.h"
+#include "bytestream.h"
 #include "codec_internal.h"
 #include "mjpeg.h"
 #include "mjpegdec.h"
 
-static uint32_t read_offs(AVCodecContext *avctx, GetBitContext *gb, uint32_t size, const char *err_msg){
-    uint32_t offs= get_bits_long(gb, 32);
+static uint32_t read_offs(AVCodecContext *avctx, GetByteContext *gb,
+                          uint32_t size, const char *err_msg)
+{
+    uint32_t offs = bytestream2_get_be32u(gb);
     if(offs >= size){
         av_log(avctx, AV_LOG_WARNING, err_msg, offs, size);
         return 0;
@@ -47,7 +50,7 @@ static int mjpegb_decode_frame(AVCodecContext *avctx, AVFrame *rframe,
     int buf_size = avpkt->size;
     MJpegDecodeContext *s = avctx->priv_data;
     const uint8_t *buf_end, *buf_ptr;
-    GetBitContext hgb; /* for the header */
+    GetByteContext hgb; /* for the header */
     uint32_t dqt_offs, dht_offs, sof_offs, sos_offs, second_field_offs;
     uint32_t field_size, sod_offs;
     int ret;
@@ -64,21 +67,22 @@ read_header:
     s->restart_count = 0;
     s->mjpb_skiptosod = 0;
 
-    if (buf_end - buf_ptr >= 1 << 28)
+    if (buf_end - buf_ptr >= 1 << 28 ||
+        buf_end - buf_ptr < 40 /* header size */)
         return AVERROR_INVALIDDATA;
 
-    init_get_bits(&hgb, buf_ptr, /*buf_size*/(buf_end - buf_ptr)*8);
+    bytestream2_init(&hgb, buf_ptr, buf_end - buf_ptr);
 
-    skip_bits(&hgb, 32); /* reserved zeros */
+    bytestream2_skipu(&hgb, 4); /* reserved zeros */
 
-    if (get_bits_long(&hgb, 32) != MKBETAG('m','j','p','g')) {
+    if (bytestream2_get_be32u(&hgb) != MKBETAG('m','j','p','g')) {
         av_log(avctx, AV_LOG_WARNING, "not mjpeg-b (bad fourcc)\n");
         return AVERROR_INVALIDDATA;
     }
 
-    field_size = get_bits_long(&hgb, 32); /* field size */
+    field_size = bytestream2_get_be32u(&hgb); /* field size */
     av_log(avctx, AV_LOG_DEBUG, "field size: 0x%"PRIx32"\n", field_size);
-    skip_bits(&hgb, 32); /* padded field size */
+    bytestream2_skipu(&hgb, 4); /* padded field size */
     second_field_offs = read_offs(avctx, &hgb, buf_end - buf_ptr, "second_field_offs is %d and size is %d\n");
     av_log(avctx, AV_LOG_DEBUG, "second field offs: 0x%"PRIx32"\n",
            second_field_offs);
-- 
2.32.0

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [FFmpeg-devel] [PATCH 2/5] avcodec/mjpegdec: Don't create unnecessary AVFrame reference
  2022-04-14 15:57 ` [FFmpeg-devel] [PATCH 2/5] avcodec/mjpegdec: Don't create unnecessary AVFrame reference Andreas Rheinhardt
@ 2022-04-14 16:39   ` James Almer
  2022-04-14 16:43     ` Andreas Rheinhardt
  0 siblings, 1 reply; 11+ messages in thread
From: James Almer @ 2022-04-14 16:39 UTC (permalink / raw)
  To: ffmpeg-devel

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?

> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@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@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [FFmpeg-devel] [PATCH 2/5] avcodec/mjpegdec: Don't create unnecessary AVFrame reference
  2022-04-14 16:39   ` James Almer
@ 2022-04-14 16:43     ` Andreas Rheinhardt
  2022-04-14 16:46       ` James Almer
  0 siblings, 1 reply; 11+ messages in thread
From: Andreas Rheinhardt @ 2022-04-14 16:43 UTC (permalink / raw)
  To: ffmpeg-devel

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.

>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@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@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [FFmpeg-devel] [PATCH 2/5] avcodec/mjpegdec: Don't create unnecessary AVFrame reference
  2022-04-14 16:43     ` Andreas Rheinhardt
@ 2022-04-14 16:46       ` James Almer
  2022-04-14 16:51         ` Andreas Rheinhardt
  0 siblings, 1 reply; 11+ messages in thread
From: James Almer @ 2022-04-14 16:46 UTC (permalink / raw)
  To: ffmpeg-devel



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.

> 
>>>
>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@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@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [FFmpeg-devel] [PATCH 2/5] avcodec/mjpegdec: Don't create unnecessary AVFrame reference
  2022-04-14 16:46       ` James Almer
@ 2022-04-14 16:51         ` Andreas Rheinhardt
  0 siblings, 0 replies; 11+ messages in thread
From: Andreas Rheinhardt @ 2022-04-14 16:51 UTC (permalink / raw)
  To: ffmpeg-devel

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@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@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> 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".

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/5] avcodec/mjpegdec: Always reset got_picture at the beginnig of decoding
  2022-04-14 15:56 [FFmpeg-devel] [PATCH 1/5] avcodec/mjpegdec: Always reset got_picture at the beginnig of decoding Andreas Rheinhardt
                   ` (3 preceding siblings ...)
  2022-04-14 15:57 ` [FFmpeg-devel] [PATCH 5/5] avcodec/mjpegbdec: Don't use GetBit-API for byte-aligned reads Andreas Rheinhardt
@ 2022-04-15 22:15 ` Michael Niedermayer
  4 siblings, 0 replies; 11+ messages in thread
From: Michael Niedermayer @ 2022-04-15 22:15 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


[-- Attachment #1.1: Type: text/plain, Size: 1588 bytes --]

On Thu, Apr 14, 2022 at 05:56:30PM +0200, Andreas Rheinhardt wrote:
> Said field is set when parsing a SOF; yet a picture is only allocated
> if skip_frame is != AVDISCARD_ALL. This leads to a crash in the
> following case: If a jpeg is split into two parts, the first containing
> everything before the scans including the SOF and the second part
> containing the rest, and the first part is sent to the decoder with
> skip_frame set to AVDISCARD_ALL, got_picture is set, yet no picture
> is allocated. If the next part is sent with skip_frame set to
> AVDISCARD_NONE, the code presumes that a picture has been allocated,
> although it hasn't leading to segfaults.
> 
> Fix this by resetting got_picture at the beginning of decoding.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
> This patch presumes that there is not use-case for partitioning
> the data corresponding to a single AVFrame accross multiple packets.
> I am not certain whether this is actually true, in particular
> wrt interlaced input where it might be common to put the data for
> one field into one packet.
> Anyway, no such use is covered by FATE.

This changes timestamps slightly for:
./ffmpeg -an -i ~/tickets/1915/m_noint.avi -an -bitexact -f framecrc -t 1 -

not sure thats intended

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Into a blind darkness they enter who follow after the Ignorance,
they as if into a greater darkness enter who devote themselves
to the Knowledge alone. -- Isha Upanishad

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [FFmpeg-devel] [PATCH 3/5] avcodec/mjpegdec: Avoid copying data when flipping image
  2022-04-14 15:57 ` [FFmpeg-devel] [PATCH 3/5] avcodec/mjpegdec: Avoid copying data when flipping image Andreas Rheinhardt
@ 2022-04-17 12:57   ` Michael Niedermayer
  0 siblings, 0 replies; 11+ messages in thread
From: Michael Niedermayer @ 2022-04-17 12:57 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


[-- Attachment #1.1: Type: text/plain, Size: 782 bytes --]

On Thu, Apr 14, 2022 at 05:57:38PM +0200, Andreas Rheinhardt wrote:
> Basically reverts af15c17daa5d8d2940c0263ba4d3ecec761c11ee.
> Flipping a picture by modifying the pointers is so common
> that even users of direct rendering should take it into account.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>  libavcodec/mjpegdec.c | 18 ++++--------------
>  1 file changed, 4 insertions(+), 14 deletions(-)

LGTM

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The day soldiers stop bringing you their problems is the day you have stopped 
leading them. They have either lost confidence that you can help or concluded 
you do not care. Either case is a failure of leadership. - Colin Powell

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-04-17 12:57 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-14 15:56 [FFmpeg-devel] [PATCH 1/5] avcodec/mjpegdec: Always reset got_picture at the beginnig of decoding Andreas Rheinhardt
2022-04-14 15:57 ` [FFmpeg-devel] [PATCH 2/5] avcodec/mjpegdec: Don't create unnecessary AVFrame reference Andreas Rheinhardt
2022-04-14 16:39   ` James Almer
2022-04-14 16:43     ` Andreas Rheinhardt
2022-04-14 16:46       ` James Almer
2022-04-14 16:51         ` Andreas Rheinhardt
2022-04-14 15:57 ` [FFmpeg-devel] [PATCH 3/5] avcodec/mjpegdec: Avoid copying data when flipping image Andreas Rheinhardt
2022-04-17 12:57   ` Michael Niedermayer
2022-04-14 15:57 ` [FFmpeg-devel] [PATCH 4/5] avcodec/mjpegbdec: Don't create unnecessary AVFrame reference Andreas Rheinhardt
2022-04-14 15:57 ` [FFmpeg-devel] [PATCH 5/5] avcodec/mjpegbdec: Don't use GetBit-API for byte-aligned reads Andreas Rheinhardt
2022-04-15 22:15 ` [FFmpeg-devel] [PATCH 1/5] avcodec/mjpegdec: Always reset got_picture at the beginnig of decoding Michael Niedermayer

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git