[FFmpeg-devel] [PATCH] avcodec/mjpegenc: support writing ICC profiles

[FFmpeg-devel] [PATCH] avcodec/mjpegenc: support writing ICC profiles

[Niklas Haas]

From: Niklas Haas <git@haasn.dev>

This is mostly straightforward. The major complication is that, as a
result of the 16-bit chunk size limitation, ICC profiles may need to be
split up into multiple chunks.

We also need to make sure to allocate enough extra space in the packet
to fit the ICC profile, so modify both mpegvideo_enc.c and ljpegenc.c to
take into account this extra overhead.

Signed-off-by: Niklas Haas <git@haasn.dev>
---
I will submit a FATE patch and test separately. (We do not currently
have any JPGs with embedded ICC profiles in the test suite)
---
 libavcodec/ljpegenc.c        |  6 +++++-
 libavcodec/mjpegenc.c        |  3 ++-
 libavcodec/mjpegenc_common.c | 41 +++++++++++++++++++++++++++++++++---
 libavcodec/mjpegenc_common.h |  2 +-
 libavcodec/mpegvideo_enc.c   |  3 +++
 5 files changed, 49 insertions(+), 6 deletions(-)

diff --git a/libavcodec/ljpegenc.c b/libavcodec/ljpegenc.c
index e15f448f90..c54450c338 100644
--- a/libavcodec/ljpegenc.c
+++ b/libavcodec/ljpegenc.c
@@ -216,6 +216,7 @@ static int ljpeg_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
 {
     LJpegEncContext *s = avctx->priv_data;
     PutBitContext pb;
+    const AVFrameSideData *sd;
     const int width  = avctx->width;
     const int height = avctx->height;
     const int mb_width  = (width  + s->hsample[0] - 1) / s->hsample[0];
@@ -233,12 +234,15 @@ static int ljpeg_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
                         * s->hsample[0] * s->vsample[0];
     }
 
+    if ((sd = av_frame_get_side_data(pict, AV_FRAME_DATA_ICC_PROFILE)))
+        max_pkt_size += sd->size;
+
     if ((ret = ff_alloc_packet(avctx, pkt, max_pkt_size)) < 0)
         return ret;
 
     init_put_bits(&pb, pkt->data, pkt->size);
 
-    ff_mjpeg_encode_picture_header(avctx, &pb, NULL, &s->scantable,
+    ff_mjpeg_encode_picture_header(avctx, &pb, pict, NULL, &s->scantable,
                                    s->pred, s->matrix, s->matrix);
 
     header_bits = put_bits_count(&pb);
diff --git a/libavcodec/mjpegenc.c b/libavcodec/mjpegenc.c
index 08671b0df7..d7c0c763a1 100644
--- a/libavcodec/mjpegenc.c
+++ b/libavcodec/mjpegenc.c
@@ -77,7 +77,8 @@ static av_cold void init_uni_ac_vlc(const uint8_t huff_size_ac[256],
 
 static void mjpeg_encode_picture_header(MpegEncContext *s)
 {
-    ff_mjpeg_encode_picture_header(s->avctx, &s->pb, s->mjpeg_ctx,
+    const AVFrame *frame = s->picture->f;
+    ff_mjpeg_encode_picture_header(s->avctx, &s->pb, frame, s->mjpeg_ctx,
                                    &s->intra_scantable, 0,
                                    s->intra_matrix, s->chroma_intra_matrix);
 
diff --git a/libavcodec/mjpegenc_common.c b/libavcodec/mjpegenc_common.c
index 995e2b7670..099ed3fce3 100644
--- a/libavcodec/mjpegenc_common.c
+++ b/libavcodec/mjpegenc_common.c
@@ -130,8 +130,10 @@ static void jpeg_table_header(AVCodecContext *avctx, PutBitContext *p,
     AV_WB16(ptr, size);
 }
 
-static void jpeg_put_comments(AVCodecContext *avctx, PutBitContext *p)
+static void jpeg_put_comments(AVCodecContext *avctx, PutBitContext *p,
+                              const AVFrame *frame)
 {
+    const AVFrameSideData *sd;
     int size;
     uint8_t *ptr;
 
@@ -161,6 +163,39 @@ static void jpeg_put_comments(AVCodecContext *avctx, PutBitContext *p)
         put_bits(p, 8, 0); /* thumbnail height */
     }
 
+    /* ICC profile */
+    if ((sd = av_frame_get_side_data(frame, AV_FRAME_DATA_ICC_PROFILE))) {
+        static const uint8_t hdr_size = strlen("ICC_PROFILE")+5;
+        static const uint16_t max_chunk_size = UINT16_MAX - hdr_size;
+        const uint8_t *data = sd->data;
+        size_t remaining = sd->size;
+        size_t nb_chunks = (remaining + max_chunk_size - 1) / max_chunk_size;
+        if (nb_chunks > 255) {
+            av_log(avctx, AV_LOG_WARNING,
+                "Cannot store %zu byte ICC profile: too large for JPEG\n",
+                sd->size);
+            nb_chunks = remaining = 0;
+        }
+
+        for (int i = 0; i < nb_chunks; i++) {
+            size = FFMIN(remaining, max_chunk_size);
+            av_assert1(size > 0);
+            put_marker(p, APP2);
+            put_bits(p, 16, size + hdr_size);
+            ff_put_string(p, "ICC_PROFILE", 1);
+            put_bits(p, 8, i+1);
+            put_bits(p, 8, nb_chunks);
+            flush_put_bits(p);
+            ptr = put_bits_ptr(p);
+            skip_put_bytes(p, size);
+            memcpy(ptr, data, size);
+            remaining -= size;
+            data += size;
+        }
+
+        av_assert1(!remaining);
+    }
+
     /* comment */
     if (!(avctx->flags & AV_CODEC_FLAG_BITEXACT)) {
         put_marker(p, COM);
@@ -213,7 +248,7 @@ void ff_mjpeg_init_hvsample(AVCodecContext *avctx, int hsample[4], int vsample[4
 }
 
 void ff_mjpeg_encode_picture_header(AVCodecContext *avctx, PutBitContext *pb,
-                                    MJpegContext *m,
+                                    const AVFrame *frame, struct MJpegContext *m,
                                     ScanTable *intra_scantable, int pred,
                                     uint16_t luma_intra_matrix[64],
                                     uint16_t chroma_intra_matrix[64])
@@ -233,7 +268,7 @@ void ff_mjpeg_encode_picture_header(AVCodecContext *avctx, PutBitContext *pb,
     if (avctx->codec_id == AV_CODEC_ID_AMV)
         return;
 
-    jpeg_put_comments(avctx, pb);
+    jpeg_put_comments(avctx, pb, frame);
 
     jpeg_table_header(avctx, pb, m, intra_scantable,
                       luma_intra_matrix, chroma_intra_matrix, hsample);
diff --git a/libavcodec/mjpegenc_common.h b/libavcodec/mjpegenc_common.h
index ac753bf153..3c0a7addac 100644
--- a/libavcodec/mjpegenc_common.h
+++ b/libavcodec/mjpegenc_common.h
@@ -30,7 +30,7 @@
 struct MJpegContext;
 
 void ff_mjpeg_encode_picture_header(AVCodecContext *avctx, PutBitContext *pb,
-                                    struct MJpegContext *m,
+                                    const AVFrame *frame, struct MJpegContext *m,
                                     ScanTable *intra_scantable, int pred,
                                     uint16_t luma_intra_matrix[64],
                                     uint16_t chroma_intra_matrix[64]);
diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
index c69114ea15..932dfa34f7 100644
--- a/libavcodec/mpegvideo_enc.c
+++ b/libavcodec/mpegvideo_enc.c
@@ -1686,10 +1686,13 @@ int ff_mpv_encode_picture(AVCodecContext *avctx, AVPacket *pkt,
 
     /* output? */
     if (s->new_picture.f->data[0]) {
+        const AVFrameSideData *sd;
         int growing_buffer = context_count == 1 && !pkt->data && !s->data_partitioning;
         int pkt_size = growing_buffer ? FFMAX(s->mb_width*s->mb_height*64+10000, avctx->internal->byte_buffer_size) - AV_INPUT_BUFFER_PADDING_SIZE
                                               :
                                               s->mb_width*s->mb_height*(MAX_MB_BYTES+100)+10000;
+        if ((sd = av_frame_get_side_data(s->new_picture.f, AV_FRAME_DATA_ICC_PROFILE)))
+            pkt_size += sd->size;
         if ((ret = ff_alloc_packet(avctx, pkt, pkt_size)) < 0)
             return ret;
         if (s->mb_info) {
-- 
2.35.1

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

Re: [FFmpeg-devel] [PATCH] avcodec/mjpegenc: support writing ICC profiles

[Andreas Rheinhardt]

Niklas Haas:
> From: Niklas Haas <git@haasn.dev>
> 
> This is mostly straightforward. The major complication is that, as a
> result of the 16-bit chunk size limitation, ICC profiles may need to be
> split up into multiple chunks.
> 
> We also need to make sure to allocate enough extra space in the packet
> to fit the ICC profile, so modify both mpegvideo_enc.c and ljpegenc.c to
> take into account this extra overhead.
> 
> Signed-off-by: Niklas Haas <git@haasn.dev>
> ---
> I will submit a FATE patch and test separately. (We do not currently
> have any JPGs with embedded ICC profiles in the test suite)

png1/lena-int_rgb24.png

- Andreas
_______________________________________________
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] [PATCH] tests: add jpg-icc test

[Niklas Haas]

From: Niklas Haas <git@haasn.dev>

Transcoding from an existing PNG file that has an embedded ICC profile.
Note that this ICC profile is smaller than 64 kB, so this doesn't test
the APP2 chunk re-arranging code at all.

Signed-off-by: Niklas Haas <git@haasn.dev>
---
 tests/fate/image.mak   |  3 +++
 tests/ref/fate/jpg-icc | 42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+)
 create mode 100644 tests/ref/fate/jpg-icc

diff --git a/tests/fate/image.mak b/tests/fate/image.mak
index 573d398915..8b72890fbe 100644
--- a/tests/fate/image.mak
+++ b/tests/fate/image.mak
@@ -337,6 +337,9 @@ fate-jpg-12bpp: CMD = framecrc -idct simple -i $(TARGET_SAMPLES)/jpg/12bpp.jpg -
 FATE_JPG += fate-jpg-jfif
 fate-jpg-jfif: CMD = framecrc -idct simple -i $(TARGET_SAMPLES)/jpg/20242.jpg
 
+FATE_JPG += fate-jpg-icc
+fate-jpg-icc: CMD = transcode png_pipe $(TARGET_SAMPLES)/png1/lena-int_rgb24.png mjpeg "-vf scale" "" "" "-show_frames"
+
 FATE_JPG-$(call DEMDEC, IMAGE2, MJPEG) += $(FATE_JPG)
 FATE_IMAGE += $(FATE_JPG-yes)
 fate-jpg: $(FATE_JPG-yes)
diff --git a/tests/ref/fate/jpg-icc b/tests/ref/fate/jpg-icc
new file mode 100644
index 0000000000..220146555e
--- /dev/null
+++ b/tests/ref/fate/jpg-icc
@@ -0,0 +1,42 @@
+0a323df5cdfb9574e329b9831be054a6 *tests/data/fate/jpg-icc.mjpeg
+11010 tests/data/fate/jpg-icc.mjpeg
+#tb 0: 1/25
+#media_type 0: video
+#codec_id 0: rawvideo
+#dimensions 0: 128x128
+#sar 0: 1/1
+0,          0,          0,        1,    49152, 0xaac06b42
+[FRAME]
+media_type=video
+stream_index=0
+key_frame=1
+pts=0
+pts_time=0.000000
+pkt_dts=0
+pkt_dts_time=0.000000
+best_effort_timestamp=0
+best_effort_timestamp_time=0.000000
+pkt_duration=1
+pkt_duration_time=0.040000
+pkt_pos=0
+pkt_size=11010
+width=128
+height=128
+pix_fmt=yuvj444p
+sample_aspect_ratio=1:1
+pict_type=I
+coded_picture_number=0
+display_picture_number=0
+interlaced_frame=0
+top_field_first=0
+repeat_pict=0
+color_range=pc
+color_space=bt470bg
+color_primaries=unknown
+color_transfer=unknown
+chroma_location=center
+[SIDE_DATA]
+side_data_type=ICC profile
+size=3144
+[/SIDE_DATA]
+[/FRAME]
-- 
2.35.1

_______________________________________________
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] [PATCH v2] avcodec/mjpegenc: support writing ICC profiles

[Niklas Haas]

From: Niklas Haas <git@haasn.dev>

This is mostly straightforward. The major complication is that, as a
result of the 16-bit chunk size limitation, ICC profiles may need to be
split up into multiple chunks.

We also need to make sure to allocate enough extra space in the packet
to fit the ICC profile, so modify both mpegvideo_enc.c and ljpegenc.c to
take into account this extra overhead.

Also add a FATE transcode test to ensure that the ICC profile gets
written (and read) correctly. Note that this ICC profile is smaller than
64 kB, so this doesn't test the APP2 chunk re-arranging code at all.

Signed-off-by: Niklas Haas <git@haasn.dev>
---
Changes in v2:
- Merge FATE test into this commit
- Fix possible segfault (when no AVFrame is available)
---
 libavcodec/ljpegenc.c        |  6 ++++-
 libavcodec/mjpegenc.c        |  3 ++-
 libavcodec/mjpegenc_common.c | 43 +++++++++++++++++++++++++++++++++---
 libavcodec/mjpegenc_common.h |  2 +-
 libavcodec/mpegvideo_enc.c   |  3 +++
 tests/fate/image.mak         |  3 +++
 tests/ref/fate/jpg-icc       | 42 +++++++++++++++++++++++++++++++++++
 7 files changed, 96 insertions(+), 6 deletions(-)
 create mode 100644 tests/ref/fate/jpg-icc

diff --git a/libavcodec/ljpegenc.c b/libavcodec/ljpegenc.c
index e15f448f90..c54450c338 100644
--- a/libavcodec/ljpegenc.c
+++ b/libavcodec/ljpegenc.c
@@ -216,6 +216,7 @@ static int ljpeg_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
 {
     LJpegEncContext *s = avctx->priv_data;
     PutBitContext pb;
+    const AVFrameSideData *sd;
     const int width  = avctx->width;
     const int height = avctx->height;
     const int mb_width  = (width  + s->hsample[0] - 1) / s->hsample[0];
@@ -233,12 +234,15 @@ static int ljpeg_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
                         * s->hsample[0] * s->vsample[0];
     }
 
+    if ((sd = av_frame_get_side_data(pict, AV_FRAME_DATA_ICC_PROFILE)))
+        max_pkt_size += sd->size;
+
     if ((ret = ff_alloc_packet(avctx, pkt, max_pkt_size)) < 0)
         return ret;
 
     init_put_bits(&pb, pkt->data, pkt->size);
 
-    ff_mjpeg_encode_picture_header(avctx, &pb, NULL, &s->scantable,
+    ff_mjpeg_encode_picture_header(avctx, &pb, pict, NULL, &s->scantable,
                                    s->pred, s->matrix, s->matrix);
 
     header_bits = put_bits_count(&pb);
diff --git a/libavcodec/mjpegenc.c b/libavcodec/mjpegenc.c
index 08671b0df7..d7c0c763a1 100644
--- a/libavcodec/mjpegenc.c
+++ b/libavcodec/mjpegenc.c
@@ -77,7 +77,8 @@ static av_cold void init_uni_ac_vlc(const uint8_t huff_size_ac[256],
 
 static void mjpeg_encode_picture_header(MpegEncContext *s)
 {
-    ff_mjpeg_encode_picture_header(s->avctx, &s->pb, s->mjpeg_ctx,
+    const AVFrame *frame = s->picture->f;
+    ff_mjpeg_encode_picture_header(s->avctx, &s->pb, frame, s->mjpeg_ctx,
                                    &s->intra_scantable, 0,
                                    s->intra_matrix, s->chroma_intra_matrix);
 
diff --git a/libavcodec/mjpegenc_common.c b/libavcodec/mjpegenc_common.c
index 995e2b7670..5594a8d239 100644
--- a/libavcodec/mjpegenc_common.c
+++ b/libavcodec/mjpegenc_common.c
@@ -130,8 +130,10 @@ static void jpeg_table_header(AVCodecContext *avctx, PutBitContext *p,
     AV_WB16(ptr, size);
 }
 
-static void jpeg_put_comments(AVCodecContext *avctx, PutBitContext *p)
+static void jpeg_put_comments(AVCodecContext *avctx, PutBitContext *p,
+                              const AVFrame *frame)
 {
+    const AVFrameSideData *sd = NULL;
     int size;
     uint8_t *ptr;
 
@@ -161,6 +163,41 @@ static void jpeg_put_comments(AVCodecContext *avctx, PutBitContext *p)
         put_bits(p, 8, 0); /* thumbnail height */
     }
 
+    /* ICC profile */
+    if (frame)
+        sd = av_frame_get_side_data(frame, AV_FRAME_DATA_ICC_PROFILE);
+    if (sd && sd->size) {
+        static const uint8_t hdr_size = strlen("ICC_PROFILE")+5;
+        static const uint16_t max_chunk_size = UINT16_MAX - hdr_size;
+        const uint8_t *data = sd->data;
+        size_t remaining = sd->size;
+        size_t nb_chunks = (remaining + max_chunk_size - 1) / max_chunk_size;
+        if (nb_chunks > 255) {
+            av_log(avctx, AV_LOG_WARNING,
+                "Cannot store %zu byte ICC profile: too large for JPEG\n",
+                sd->size);
+            nb_chunks = remaining = 0;
+        }
+
+        for (int i = 0; i < nb_chunks; i++) {
+            size = FFMIN(remaining, max_chunk_size);
+            av_assert1(size > 0);
+            put_marker(p, APP2);
+            put_bits(p, 16, size + hdr_size);
+            ff_put_string(p, "ICC_PROFILE", 1);
+            put_bits(p, 8, i+1);
+            put_bits(p, 8, nb_chunks);
+            flush_put_bits(p);
+            ptr = put_bits_ptr(p);
+            skip_put_bytes(p, size);
+            memcpy(ptr, data, size);
+            remaining -= size;
+            data += size;
+        }
+
+        av_assert1(!remaining);
+    }
+
     /* comment */
     if (!(avctx->flags & AV_CODEC_FLAG_BITEXACT)) {
         put_marker(p, COM);
@@ -213,7 +250,7 @@ void ff_mjpeg_init_hvsample(AVCodecContext *avctx, int hsample[4], int vsample[4
 }
 
 void ff_mjpeg_encode_picture_header(AVCodecContext *avctx, PutBitContext *pb,
-                                    MJpegContext *m,
+                                    const AVFrame *frame, struct MJpegContext *m,
                                     ScanTable *intra_scantable, int pred,
                                     uint16_t luma_intra_matrix[64],
                                     uint16_t chroma_intra_matrix[64])
@@ -233,7 +270,7 @@ void ff_mjpeg_encode_picture_header(AVCodecContext *avctx, PutBitContext *pb,
     if (avctx->codec_id == AV_CODEC_ID_AMV)
         return;
 
-    jpeg_put_comments(avctx, pb);
+    jpeg_put_comments(avctx, pb, frame);
 
     jpeg_table_header(avctx, pb, m, intra_scantable,
                       luma_intra_matrix, chroma_intra_matrix, hsample);
diff --git a/libavcodec/mjpegenc_common.h b/libavcodec/mjpegenc_common.h
index ac753bf153..3c0a7addac 100644
--- a/libavcodec/mjpegenc_common.h
+++ b/libavcodec/mjpegenc_common.h
@@ -30,7 +30,7 @@
 struct MJpegContext;
 
 void ff_mjpeg_encode_picture_header(AVCodecContext *avctx, PutBitContext *pb,
-                                    struct MJpegContext *m,
+                                    const AVFrame *frame, struct MJpegContext *m,
                                     ScanTable *intra_scantable, int pred,
                                     uint16_t luma_intra_matrix[64],
                                     uint16_t chroma_intra_matrix[64]);
diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
index c69114ea15..932dfa34f7 100644
--- a/libavcodec/mpegvideo_enc.c
+++ b/libavcodec/mpegvideo_enc.c
@@ -1686,10 +1686,13 @@ int ff_mpv_encode_picture(AVCodecContext *avctx, AVPacket *pkt,
 
     /* output? */
     if (s->new_picture.f->data[0]) {
+        const AVFrameSideData *sd;
         int growing_buffer = context_count == 1 && !pkt->data && !s->data_partitioning;
         int pkt_size = growing_buffer ? FFMAX(s->mb_width*s->mb_height*64+10000, avctx->internal->byte_buffer_size) - AV_INPUT_BUFFER_PADDING_SIZE
                                               :
                                               s->mb_width*s->mb_height*(MAX_MB_BYTES+100)+10000;
+        if ((sd = av_frame_get_side_data(s->new_picture.f, AV_FRAME_DATA_ICC_PROFILE)))
+            pkt_size += sd->size;
         if ((ret = ff_alloc_packet(avctx, pkt, pkt_size)) < 0)
             return ret;
         if (s->mb_info) {
diff --git a/tests/fate/image.mak b/tests/fate/image.mak
index 573d398915..8b72890fbe 100644
--- a/tests/fate/image.mak
+++ b/tests/fate/image.mak
@@ -337,6 +337,9 @@ fate-jpg-12bpp: CMD = framecrc -idct simple -i $(TARGET_SAMPLES)/jpg/12bpp.jpg -
 FATE_JPG += fate-jpg-jfif
 fate-jpg-jfif: CMD = framecrc -idct simple -i $(TARGET_SAMPLES)/jpg/20242.jpg
 
+FATE_JPG += fate-jpg-icc
+fate-jpg-icc: CMD = transcode png_pipe $(TARGET_SAMPLES)/png1/lena-int_rgb24.png mjpeg "-vf scale" "" "" "-show_frames"
+
 FATE_JPG-$(call DEMDEC, IMAGE2, MJPEG) += $(FATE_JPG)
 FATE_IMAGE += $(FATE_JPG-yes)
 fate-jpg: $(FATE_JPG-yes)
diff --git a/tests/ref/fate/jpg-icc b/tests/ref/fate/jpg-icc
new file mode 100644
index 0000000000..220146555e
--- /dev/null
+++ b/tests/ref/fate/jpg-icc
@@ -0,0 +1,42 @@
+0a323df5cdfb9574e329b9831be054a6 *tests/data/fate/jpg-icc.mjpeg
+11010 tests/data/fate/jpg-icc.mjpeg
+#tb 0: 1/25
+#media_type 0: video
+#codec_id 0: rawvideo
+#dimensions 0: 128x128
+#sar 0: 1/1
+0,          0,          0,        1,    49152, 0xaac06b42
+[FRAME]
+media_type=video
+stream_index=0
+key_frame=1
+pts=0
+pts_time=0.000000
+pkt_dts=0
+pkt_dts_time=0.000000
+best_effort_timestamp=0
+best_effort_timestamp_time=0.000000
+pkt_duration=1
+pkt_duration_time=0.040000
+pkt_pos=0
+pkt_size=11010
+width=128
+height=128
+pix_fmt=yuvj444p
+sample_aspect_ratio=1:1
+pict_type=I
+coded_picture_number=0
+display_picture_number=0
+interlaced_frame=0
+top_field_first=0
+repeat_pict=0
+color_range=pc
+color_space=bt470bg
+color_primaries=unknown
+color_transfer=unknown
+chroma_location=center
+[SIDE_DATA]
+side_data_type=ICC profile
+size=3144
+[/SIDE_DATA]
+[/FRAME]
-- 
2.35.1

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

Re: [FFmpeg-devel] [PATCH v2] avcodec/mjpegenc: support writing ICC profiles

[Andreas Rheinhardt]

Niklas Haas:
> From: Niklas Haas <git@haasn.dev>
> 
> This is mostly straightforward. The major complication is that, as a
> result of the 16-bit chunk size limitation, ICC profiles may need to be
> split up into multiple chunks.
> 
> We also need to make sure to allocate enough extra space in the packet
> to fit the ICC profile, so modify both mpegvideo_enc.c and ljpegenc.c to
> take into account this extra overhead.
> 
> Also add a FATE transcode test to ensure that the ICC profile gets
> written (and read) correctly. Note that this ICC profile is smaller than
> 64 kB, so this doesn't test the APP2 chunk re-arranging code at all.
> 
> Signed-off-by: Niklas Haas <git@haasn.dev>
> ---
> Changes in v2:
> - Merge FATE test into this commit
> - Fix possible segfault (when no AVFrame is available)

When could this ever happen? IMO it can't.

> ---
>  libavcodec/ljpegenc.c        |  6 ++++-
>  libavcodec/mjpegenc.c        |  3 ++-
>  libavcodec/mjpegenc_common.c | 43 +++++++++++++++++++++++++++++++++---
>  libavcodec/mjpegenc_common.h |  2 +-
>  libavcodec/mpegvideo_enc.c   |  3 +++
>  tests/fate/image.mak         |  3 +++
>  tests/ref/fate/jpg-icc       | 42 +++++++++++++++++++++++++++++++++++
>  7 files changed, 96 insertions(+), 6 deletions(-)
>  create mode 100644 tests/ref/fate/jpg-icc
> 
> diff --git a/libavcodec/ljpegenc.c b/libavcodec/ljpegenc.c
> index e15f448f90..c54450c338 100644
> --- a/libavcodec/ljpegenc.c
> +++ b/libavcodec/ljpegenc.c
> @@ -216,6 +216,7 @@ static int ljpeg_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
>  {
>      LJpegEncContext *s = avctx->priv_data;
>      PutBitContext pb;
> +    const AVFrameSideData *sd;
>      const int width  = avctx->width;
>      const int height = avctx->height;
>      const int mb_width  = (width  + s->hsample[0] - 1) / s->hsample[0];
> @@ -233,12 +234,15 @@ static int ljpeg_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
>                          * s->hsample[0] * s->vsample[0];
>      }
>  
> +    if ((sd = av_frame_get_side_data(pict, AV_FRAME_DATA_ICC_PROFILE)))
> +        max_pkt_size += sd->size;

This does not account for the chunk overhead.

> +
>      if ((ret = ff_alloc_packet(avctx, pkt, max_pkt_size)) < 0)
>          return ret;
>  
>      init_put_bits(&pb, pkt->data, pkt->size);
>  
> -    ff_mjpeg_encode_picture_header(avctx, &pb, NULL, &s->scantable,
> +    ff_mjpeg_encode_picture_header(avctx, &pb, pict, NULL, &s->scantable,
>                                     s->pred, s->matrix, s->matrix);
>  
>      header_bits = put_bits_count(&pb);
> diff --git a/libavcodec/mjpegenc.c b/libavcodec/mjpegenc.c
> index 08671b0df7..d7c0c763a1 100644
> --- a/libavcodec/mjpegenc.c
> +++ b/libavcodec/mjpegenc.c
> @@ -77,7 +77,8 @@ static av_cold void init_uni_ac_vlc(const uint8_t huff_size_ac[256],
>  
>  static void mjpeg_encode_picture_header(MpegEncContext *s)
>  {
> -    ff_mjpeg_encode_picture_header(s->avctx, &s->pb, s->mjpeg_ctx,
> +    const AVFrame *frame = s->picture->f;
> +    ff_mjpeg_encode_picture_header(s->avctx, &s->pb, frame, s->mjpeg_ctx,
>                                     &s->intra_scantable, 0,
>                                     s->intra_matrix, s->chroma_intra_matrix);
>  
> diff --git a/libavcodec/mjpegenc_common.c b/libavcodec/mjpegenc_common.c
> index 995e2b7670..5594a8d239 100644
> --- a/libavcodec/mjpegenc_common.c
> +++ b/libavcodec/mjpegenc_common.c
> @@ -130,8 +130,10 @@ static void jpeg_table_header(AVCodecContext *avctx, PutBitContext *p,
>      AV_WB16(ptr, size);
>  }
>  
> -static void jpeg_put_comments(AVCodecContext *avctx, PutBitContext *p)
> +static void jpeg_put_comments(AVCodecContext *avctx, PutBitContext *p,
> +                              const AVFrame *frame)
>  {
> +    const AVFrameSideData *sd = NULL;
>      int size;
>      uint8_t *ptr;
>  
> @@ -161,6 +163,41 @@ static void jpeg_put_comments(AVCodecContext *avctx, PutBitContext *p)
>          put_bits(p, 8, 0); /* thumbnail height */
>      }
>  
> +    /* ICC profile */
> +    if (frame)
> +        sd = av_frame_get_side_data(frame, AV_FRAME_DATA_ICC_PROFILE);
> +    if (sd && sd->size) {
> +        static const uint8_t hdr_size = strlen("ICC_PROFILE")+5;

This is illegal as initializers for static objects need to be constant
expressions or string literals. Which this thing isn't. You are
basically presuming that strlen be evaluated at compile-time.

> +        static const uint16_t max_chunk_size = UINT16_MAX - hdr_size;
> +        const uint8_t *data = sd->data;
> +        size_t remaining = sd->size;
> +        size_t nb_chunks = (remaining + max_chunk_size - 1) / max_chunk_size;

There is a potential overflow here. You could avoid it by checking
whether remaining is > 255 * max_chunk_size.

> +        if (nb_chunks > 255) {
> +            av_log(avctx, AV_LOG_WARNING,
> +                "Cannot store %zu byte ICC profile: too large for JPEG\n",

We use SIZE_SPECIFIER for compatibility with old versions of MSVC. Note
that I do not know whether this is still needed, as all currently
supported MSVC versions might already support #zu.

> +                sd->size);
> +            nb_chunks = remaining = 0;
> +        }
> +
> +        for (int i = 0; i < nb_chunks; i++) {
> +            size = FFMIN(remaining, max_chunk_size);
> +            av_assert1(size > 0);
> +            put_marker(p, APP2);
> +            put_bits(p, 16, size + hdr_size);
> +            ff_put_string(p, "ICC_PROFILE", 1);
> +            put_bits(p, 8, i+1);
> +            put_bits(p, 8, nb_chunks);
> +            flush_put_bits(p);
> +            ptr = put_bits_ptr(p);
> +            skip_put_bytes(p, size);
> +            memcpy(ptr, data, size);
> +            remaining -= size;

Everything that you are doing here is byte-aligned, so I think it would
be cleaner (and less code) to write everything directly and
skip_put_bytes once afterwards.

> +            data += size;
> +        }
> +
> +        av_assert1(!remaining);
> +    }
> +
>      /* comment */
>      if (!(avctx->flags & AV_CODEC_FLAG_BITEXACT)) {
>          put_marker(p, COM);
> @@ -213,7 +250,7 @@ void ff_mjpeg_init_hvsample(AVCodecContext *avctx, int hsample[4], int vsample[4
>  }
>  
>  void ff_mjpeg_encode_picture_header(AVCodecContext *avctx, PutBitContext *pb,
> -                                    MJpegContext *m,
> +                                    const AVFrame *frame, struct MJpegContext *m,
>                                      ScanTable *intra_scantable, int pred,
>                                      uint16_t luma_intra_matrix[64],
>                                      uint16_t chroma_intra_matrix[64])
> @@ -233,7 +270,7 @@ void ff_mjpeg_encode_picture_header(AVCodecContext *avctx, PutBitContext *pb,
>      if (avctx->codec_id == AV_CODEC_ID_AMV)
>          return;
>  
> -    jpeg_put_comments(avctx, pb);
> +    jpeg_put_comments(avctx, pb, frame);
>  
>      jpeg_table_header(avctx, pb, m, intra_scantable,
>                        luma_intra_matrix, chroma_intra_matrix, hsample);
> diff --git a/libavcodec/mjpegenc_common.h b/libavcodec/mjpegenc_common.h
> index ac753bf153..3c0a7addac 100644
> --- a/libavcodec/mjpegenc_common.h
> +++ b/libavcodec/mjpegenc_common.h
> @@ -30,7 +30,7 @@
>  struct MJpegContext;
>  
>  void ff_mjpeg_encode_picture_header(AVCodecContext *avctx, PutBitContext *pb,
> -                                    struct MJpegContext *m,
> +                                    const AVFrame *frame, struct MJpegContext *m,
>                                      ScanTable *intra_scantable, int pred,
>                                      uint16_t luma_intra_matrix[64],
>                                      uint16_t chroma_intra_matrix[64]);
> diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
> index c69114ea15..932dfa34f7 100644
> --- a/libavcodec/mpegvideo_enc.c
> +++ b/libavcodec/mpegvideo_enc.c
> @@ -1686,10 +1686,13 @@ int ff_mpv_encode_picture(AVCodecContext *avctx, AVPacket *pkt,
>  
>      /* output? */
>      if (s->new_picture.f->data[0]) {
> +        const AVFrameSideData *sd;
>          int growing_buffer = context_count == 1 && !pkt->data && !s->data_partitioning;
>          int pkt_size = growing_buffer ? FFMAX(s->mb_width*s->mb_height*64+10000, avctx->internal->byte_buffer_size) - AV_INPUT_BUFFER_PADDING_SIZE
>                                                :
>                                                s->mb_width*s->mb_height*(MAX_MB_BYTES+100)+10000;
> +        if ((sd = av_frame_get_side_data(s->new_picture.f, AV_FRAME_DATA_ICC_PROFILE)))
> +            pkt_size += sd->size;

This does not account for the chunk overhead and it is also not targeted
to MJPEG.

>          if ((ret = ff_alloc_packet(avctx, pkt, pkt_size)) < 0)
>              return ret;
>          if (s->mb_info) {
> diff --git a/tests/fate/image.mak b/tests/fate/image.mak
> index 573d398915..8b72890fbe 100644
> --- a/tests/fate/image.mak
> +++ b/tests/fate/image.mak
> @@ -337,6 +337,9 @@ fate-jpg-12bpp: CMD = framecrc -idct simple -i $(TARGET_SAMPLES)/jpg/12bpp.jpg -
>  FATE_JPG += fate-jpg-jfif
>  fate-jpg-jfif: CMD = framecrc -idct simple -i $(TARGET_SAMPLES)/jpg/20242.jpg
>  
> +FATE_JPG += fate-jpg-icc
> +fate-jpg-icc: CMD = transcode png_pipe $(TARGET_SAMPLES)/png1/lena-int_rgb24.png mjpeg "-vf scale" "" "" "-show_frames"

This needs an ffprobe dependency.

> +
>  FATE_JPG-$(call DEMDEC, IMAGE2, MJPEG) += $(FATE_JPG)
>  FATE_IMAGE += $(FATE_JPG-yes)
>  fate-jpg: $(FATE_JPG-yes)
> diff --git a/tests/ref/fate/jpg-icc b/tests/ref/fate/jpg-icc
> new file mode 100644
> index 0000000000..220146555e
> --- /dev/null
> +++ b/tests/ref/fate/jpg-icc
> @@ -0,0 +1,42 @@
> +0a323df5cdfb9574e329b9831be054a6 *tests/data/fate/jpg-icc.mjpeg
> +11010 tests/data/fate/jpg-icc.mjpeg
> +#tb 0: 1/25
> +#media_type 0: video
> +#codec_id 0: rawvideo
> +#dimensions 0: 128x128
> +#sar 0: 1/1
> +0,          0,          0,        1,    49152, 0xaac06b42
> +[FRAME]
> +media_type=video
> +stream_index=0
> +key_frame=1
> +pts=0
> +pts_time=0.000000
> +pkt_dts=0
> +pkt_dts_time=0.000000
> +best_effort_timestamp=0
> +best_effort_timestamp_time=0.000000
> +pkt_duration=1
> +pkt_duration_time=0.040000
> +pkt_pos=0
> +pkt_size=11010
> +width=128
> +height=128
> +pix_fmt=yuvj444p
> +sample_aspect_ratio=1:1
> +pict_type=I
> +coded_picture_number=0
> +display_picture_number=0
> +interlaced_frame=0
> +top_field_first=0
> +repeat_pict=0
> +color_range=pc
> +color_space=bt470bg
> +color_primaries=unknown
> +color_transfer=unknown
> +chroma_location=center
> +[SIDE_DATA]
> +side_data_type=ICC profile
> +size=3144
> +[/SIDE_DATA]
> +[/FRAME]

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