diff mbox

[FFmpeg-devel,1/3] mjpegenc_common: check for codec ID before using avctx->priv_data

Message ID 20170403201330.44240-1-atomnuker@gmail.com
State Accepted
Commit 2c9be3882a03823413945bd9e2d9af33e6e322d5
Headers show

Commit Message

Rostislav Pehlivanov April 3, 2017, 8:13 p.m. UTC
When coding lossless jpeg the priv context will be pointing to LJpegEncContext
rather than MpegEncContext, which the function expects.

Signed-off-by: Rostislav Pehlivanov <atomnuker@gmail.com>
---
 libavcodec/mjpegenc_common.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Comments

Michael Niedermayer April 5, 2017, 7:20 p.m. UTC | #1
On Mon, Apr 03, 2017 at 09:13:28PM +0100, Rostislav Pehlivanov wrote:
> When coding lossless jpeg the priv context will be pointing to LJpegEncContext
> rather than MpegEncContext, which the function expects.
> 
> Signed-off-by: Rostislav Pehlivanov <atomnuker@gmail.com>
> ---
>  libavcodec/mjpegenc_common.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/mjpegenc_common.c b/libavcodec/mjpegenc_common.c
> index 83a9e95766..6d9c982726 100644
> --- a/libavcodec/mjpegenc_common.c
> +++ b/libavcodec/mjpegenc_common.c
> @@ -91,13 +91,17 @@ static void jpeg_table_header(AVCodecContext *avctx, PutBitContext *p,
>  {
>      int i, j, size;
>      uint8_t *ptr;
> -    MpegEncContext *s = avctx->priv_data;
> +    MpegEncContext *s = NULL;
> +
> +    /* Since avctx->priv_data will point to LJpegEncContext in this case */
> +    if (avctx->codec_id != AV_CODEC_ID_LJPEG)
> +        s = avctx->priv_data;
>  
>      if (avctx->codec_id != AV_CODEC_ID_LJPEG) {
>          int matrix_count = 1 + !!memcmp(luma_intra_matrix,
>                                          chroma_intra_matrix,
>                                          sizeof(luma_intra_matrix[0]) * 64);
> -    if (s->force_duplicated_matrix)
> +    if (s && s->force_duplicated_matrix)
>          matrix_count = 2;
>      /* quant matrixes */
>      put_marker(p, DQT);

> @@ -134,7 +138,7 @@ static void jpeg_table_header(AVCodecContext *avctx, PutBitContext *p,
>  
>      // Only MJPEG can have a variable Huffman variable. All other
>      // formats use the default Huffman table.
> -    if (s->out_format == FMT_MJPEG && s->huffman == HUFFMAN_TABLE_OPTIMAL) {
> +    if (s && s->huffman == HUFFMAN_TABLE_OPTIMAL) {
>          size += put_huffman_table(p, 0, 0, s->mjpeg_ctx->bits_dc_luminance,
>                                    s->mjpeg_ctx->val_dc_luminance);
>          size += put_huffman_table(p, 0, 1, s->mjpeg_ctx->bits_dc_chrominance,

ljpeg uses the same huffman tables as mjpeg,the same
ff_mjpeg_encode_dc()

instead of special casing ljpeg it would be better if the identical
huffman stuff would be shared and the same

[...]
Rostislav Pehlivanov April 8, 2017, 11:06 p.m. UTC | #2
On 5 April 2017 at 20:20, Michael Niedermayer <michael@niedermayer.cc>
wrote:

> On Mon, Apr 03, 2017 at 09:13:28PM +0100, Rostislav Pehlivanov wrote:
> > When coding lossless jpeg the priv context will be pointing to
> LJpegEncContext
> > rather than MpegEncContext, which the function expects.
> >
> > Signed-off-by: Rostislav Pehlivanov <atomnuker@gmail.com>
> > ---
> >  libavcodec/mjpegenc_common.c | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/libavcodec/mjpegenc_common.c b/libavcodec/mjpegenc_common.c
> > index 83a9e95766..6d9c982726 100644
> > --- a/libavcodec/mjpegenc_common.c
> > +++ b/libavcodec/mjpegenc_common.c
> > @@ -91,13 +91,17 @@ static void jpeg_table_header(AVCodecContext
> *avctx, PutBitContext *p,
> >  {
> >      int i, j, size;
> >      uint8_t *ptr;
> > -    MpegEncContext *s = avctx->priv_data;
> > +    MpegEncContext *s = NULL;
> > +
> > +    /* Since avctx->priv_data will point to LJpegEncContext in this
> case */
> > +    if (avctx->codec_id != AV_CODEC_ID_LJPEG)
> > +        s = avctx->priv_data;
> >
> >      if (avctx->codec_id != AV_CODEC_ID_LJPEG) {
> >          int matrix_count = 1 + !!memcmp(luma_intra_matrix,
> >                                          chroma_intra_matrix,
> >                                          sizeof(luma_intra_matrix[0]) *
> 64);
> > -    if (s->force_duplicated_matrix)
> > +    if (s && s->force_duplicated_matrix)
> >          matrix_count = 2;
> >      /* quant matrixes */
> >      put_marker(p, DQT);
>
> > @@ -134,7 +138,7 @@ static void jpeg_table_header(AVCodecContext
> *avctx, PutBitContext *p,
> >
> >      // Only MJPEG can have a variable Huffman variable. All other
> >      // formats use the default Huffman table.
> > -    if (s->out_format == FMT_MJPEG && s->huffman ==
> HUFFMAN_TABLE_OPTIMAL) {
> > +    if (s && s->huffman == HUFFMAN_TABLE_OPTIMAL) {
> >          size += put_huffman_table(p, 0, 0, s->mjpeg_ctx->bits_dc_
> luminance,
> >                                    s->mjpeg_ctx->val_dc_luminance);
> >          size += put_huffman_table(p, 0, 1, s->mjpeg_ctx->bits_dc_
> chrominance,
>
> ljpeg uses the same huffman tables as mjpeg,the same
> ff_mjpeg_encode_dc()
>
> instead of special casing ljpeg it would be better if the identical
> huffman stuff would be shared and the same
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Republics decline into democracies and democracies degenerate into
> despotisms. -- Aristotle
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
Pushed to master and release/3.3 as-is for now. I'll send a separate patch
to fix and enable optimized huffman in ljpeg later.
diff mbox

Patch

diff --git a/libavcodec/mjpegenc_common.c b/libavcodec/mjpegenc_common.c
index 83a9e95766..6d9c982726 100644
--- a/libavcodec/mjpegenc_common.c
+++ b/libavcodec/mjpegenc_common.c
@@ -91,13 +91,17 @@  static void jpeg_table_header(AVCodecContext *avctx, PutBitContext *p,
 {
     int i, j, size;
     uint8_t *ptr;
-    MpegEncContext *s = avctx->priv_data;
+    MpegEncContext *s = NULL;
+
+    /* Since avctx->priv_data will point to LJpegEncContext in this case */
+    if (avctx->codec_id != AV_CODEC_ID_LJPEG)
+        s = avctx->priv_data;
 
     if (avctx->codec_id != AV_CODEC_ID_LJPEG) {
         int matrix_count = 1 + !!memcmp(luma_intra_matrix,
                                         chroma_intra_matrix,
                                         sizeof(luma_intra_matrix[0]) * 64);
-    if (s->force_duplicated_matrix)
+    if (s && s->force_duplicated_matrix)
         matrix_count = 2;
     /* quant matrixes */
     put_marker(p, DQT);
@@ -134,7 +138,7 @@  static void jpeg_table_header(AVCodecContext *avctx, PutBitContext *p,
 
     // Only MJPEG can have a variable Huffman variable. All other
     // formats use the default Huffman table.
-    if (s->out_format == FMT_MJPEG && s->huffman == HUFFMAN_TABLE_OPTIMAL) {
+    if (s && s->huffman == HUFFMAN_TABLE_OPTIMAL) {
         size += put_huffman_table(p, 0, 0, s->mjpeg_ctx->bits_dc_luminance,
                                   s->mjpeg_ctx->val_dc_luminance);
         size += put_huffman_table(p, 0, 1, s->mjpeg_ctx->bits_dc_chrominance,