Re: [FFmpeg-devel] [PATCH] Media 100i decoder

[Paul B Mahol <onemda@gmail.com>]

On 9/22/22, Andreas Rheinhardt <andreas.rheinhardt@outlook.com> wrote:
> Paul B Mahol:
>> Subject: [PATCH] avcodec: add Media 100i decoder
>>
>> Signed-off-by: Paul B Mahol <onemda@gmail.com>
>> ---
>>  libavcodec/Makefile     |   1 +
>>  libavcodec/allcodecs.c  |   1 +
>>  libavcodec/codec_desc.c |   7 ++
>>  libavcodec/codec_id.h   |   1 +
>>  libavcodec/media100.c   | 216 ++++++++++++++++++++++++++++++++++++++++
>>  libavformat/isom_tags.c |   4 +
>>  6 files changed, 230 insertions(+)
>>  create mode 100644 libavcodec/media100.c
>>
>> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
>> index b9aa6efaac..45702551b3 100644
>> --- a/libavcodec/Makefile
>> +++ b/libavcodec/Makefile
>> @@ -468,6 +468,7 @@ OBJS-$(CONFIG_MACE6_DECODER)           += mace.o
>>  OBJS-$(CONFIG_MAGICYUV_DECODER)        += magicyuv.o
>>  OBJS-$(CONFIG_MAGICYUV_ENCODER)        += magicyuvenc.o
>>  OBJS-$(CONFIG_MDEC_DECODER)            += mdec.o mpeg12.o mpeg12data.o
>> +OBJS-$(CONFIG_MEDIA100_DECODER)        += media100.o
>>  OBJS-$(CONFIG_METASOUND_DECODER)       += metasound.o metasound_data.o \
>>                                            twinvq.o
>>  OBJS-$(CONFIG_MICRODVD_DECODER)        += microdvddec.o ass.o
>> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
>> index fc88e25fda..1966f61770 100644
>> --- a/libavcodec/allcodecs.c
>> +++ b/libavcodec/allcodecs.c
>> @@ -194,6 +194,7 @@ extern const FFCodec ff_m101_decoder;
>>  extern const FFCodec ff_magicyuv_encoder;
>>  extern const FFCodec ff_magicyuv_decoder;
>>  extern const FFCodec ff_mdec_decoder;
>> +extern const FFCodec ff_media100_decoder;
>>  extern const FFCodec ff_mimic_decoder;
>>  extern const FFCodec ff_mjpeg_encoder;
>>  extern const FFCodec ff_mjpeg_decoder;
>> diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
>> index e8e1529401..a58b3f9780 100644
>> --- a/libavcodec/codec_desc.c
>> +++ b/libavcodec/codec_desc.c
>> @@ -1909,6 +1909,13 @@ static const AVCodecDescriptor codec_descriptors[]
>> = {
>>          .long_name = NULL_IF_CONFIG_SMALL("WBMP (Wireless Application
>> Protocol Bitmap) image"),
>>          .props     = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSLESS,
>>      },
>> +    {
>> +        .id        = AV_CODEC_ID_MEDIA100,
>> +        .type      = AVMEDIA_TYPE_VIDEO,
>> +        .name      = "media100",
>> +        .long_name = NULL_IF_CONFIG_SMALL("Media 100i"),
>> +        .props     = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSY,
>> +    },
>>
>>      /* various PCM "codecs" */
>>      {
>> diff --git a/libavcodec/codec_id.h b/libavcodec/codec_id.h
>> index 9c01ea9750..dc79c95b46 100644
>> --- a/libavcodec/codec_id.h
>> +++ b/libavcodec/codec_id.h
>> @@ -318,6 +318,7 @@ enum AVCodecID {
>>      AV_CODEC_ID_PHM,
>>      AV_CODEC_ID_RADIANCE_HDR,
>>      AV_CODEC_ID_WBMP,
>> +    AV_CODEC_ID_MEDIA100,
>>
>>      /* various PCM "codecs" */
>>      AV_CODEC_ID_FIRST_AUDIO = 0x10000,     ///< A dummy id pointing at
>> the start of audio codecs
>> diff --git a/libavcodec/media100.c b/libavcodec/media100.c
>> new file mode 100644
>> index 0000000000..2c7bb3cf93
>> --- /dev/null
>> +++ b/libavcodec/media100.c
>> @@ -0,0 +1,216 @@
>> +/*
>> + * Media 100 decoder
>> + * Copyright (c) 2022 Paul B Mahol
>> + *
>> + * This file is part of FFmpeg.
>> + *
>> + * FFmpeg is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2.1 of the License, or (at your option) any later version.
>> + *
>> + * FFmpeg is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with FFmpeg; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>> 02110-1301 USA
>> + */
>> +
>> +/**
>> + * @file
>> + * Media 100 decoder.
>> + */
>> +
>> +#include <inttypes.h>
>
> I think you don't even need stdint.h, let alone inttypes.h.
>
>> +
>> +#include "libavutil/intreadwrite.h"
>> +#include "avcodec.h"
>> +#include "bytestream.h"
>> +#include "codec_internal.h"
>> +
>> +typedef struct Media100Context {
>> +    AVCodecContext *avctx;   // wrapper context for mjpegb
>> +} Media100Context;
>> +
>> +static av_cold int media100_decode_init(AVCodecContext *avctx)
>> +{
>> +    Media100Context *ctx = avctx->priv_data;
>> +    const AVCodec *codec;
>> +    int ret;
>> +
>> +    codec = avcodec_find_decoder(AV_CODEC_ID_MJPEGB);
>> +    if (!codec)
>> +        return AVERROR_BUG;
>
> This bug can be triggered, because you did not add a configure
> dependency on the mjpegb decoder.
> Btw: If you used ff_mjpegb_decoder directly, the linker would now about
> this dependency and so this decoder could be fuzzed. (Right now we
> magically add these dependencies for the fuzzer; see lines 188-191 in
> target_dec_fuzzer.c.)
>
>> +    ctx->avctx = avcodec_alloc_context3(codec);
>> +    if (!ctx->avctx)
>> +        return AVERROR(ENOMEM);
>> +    ctx->avctx->thread_count = 1;
>> +    ctx->avctx->flags  = avctx->flags;
>> +    ctx->avctx->flags2 = avctx->flags2;
>> +    ctx->avctx->width  = ctx->avctx->coded_width  = avctx->width;
>> +    ctx->avctx->height = ctx->avctx->coded_height = avctx->height;
>> +
>> +    ret = avcodec_open2(ctx->avctx, codec, NULL);
>> +    if (ret < 0)
>> +        return ret;
>> +
>> +    return 0;
>> +}
>> +
>> +static int media100_decode_frame(AVCodecContext *avctx,
>> +                                 AVFrame *frame, int *got_frame,
>> +                                 AVPacket *avpkt)
>> +{
>> +    Media100Context *ctx = avctx->priv_data;
>> +    unsigned second_field_offset = 0;
>> +    unsigned next_field = 0;
>> +    unsigned dht_offset[2];
>> +    unsigned dqt_offset[2];
>> +    unsigned sod_offset[2];
>> +    unsigned sof_offset[2];
>> +    unsigned sos_offset[2];
>> +    unsigned field = 0;
>> +    GetByteContext gb;
>> +    PutByteContext pb;
>> +    AVPacket *pkt;
>> +    int ret;
>> +
>> +    pkt = av_packet_alloc();
>> +    if (!pkt)
>> +        return AVERROR(ENOMEM);
>
> This packet and all its content leak.

What you propose instead?

>
>> +
>> +    ret = av_new_packet(pkt, avpkt->size + 1024);
>> +    if (ret < 0) {
>> +        av_log(avctx, AV_LOG_ERROR, "Error allocating the output
>> packet\n");
>> +        return ret;
>> +    }
>> +
>> +    bytestream2_init(&gb, avpkt->data, avpkt->size);
>> +    bytestream2_init_writer(&pb, pkt->data, pkt->size);
>> +
>> +second_field:
>> +    bytestream2_put_be32(&pb, 0);
>> +    bytestream2_put_be32(&pb, AV_RB32("mjpg"));
>> +    bytestream2_put_be32(&pb, 0);
>> +    bytestream2_put_be32(&pb, 0);
>> +    for (int i = 0; i < 6; i++)
>> +        bytestream2_put_be32(&pb, 0);
>> +
>> +    sof_offset[field] = bytestream2_tell_p(&pb);
>> +    bytestream2_put_be16(&pb, 17);
>> +    bytestream2_put_byte(&pb, 8);
>> +    bytestream2_put_be16(&pb, avctx->height / 2);
>> +    bytestream2_put_be16(&pb, avctx->width);
>> +    bytestream2_put_byte(&pb, 3);
>> +    bytestream2_put_byte(&pb, 1);
>> +    bytestream2_put_byte(&pb, 0x21);
>> +    bytestream2_put_byte(&pb, 0);
>> +    bytestream2_put_byte(&pb, 2);
>> +    bytestream2_put_byte(&pb, 0x11);
>> +    bytestream2_put_byte(&pb, 1);
>> +    bytestream2_put_byte(&pb, 3);
>> +    bytestream2_put_byte(&pb, 0x11);
>> +    bytestream2_put_byte(&pb, 1);
>> +
>> +    sos_offset[field] = bytestream2_tell_p(&pb);
>> +    bytestream2_put_be16(&pb, 12);
>> +    bytestream2_put_byte(&pb, 3);
>> +    bytestream2_put_byte(&pb, 1);
>> +    bytestream2_put_byte(&pb, 0);
>> +    bytestream2_put_byte(&pb, 2);
>> +    bytestream2_put_byte(&pb, 0x11);
>> +    bytestream2_put_byte(&pb, 3);
>> +    bytestream2_put_byte(&pb, 0x11);
>> +    bytestream2_put_byte(&pb, 0);
>> +    bytestream2_put_byte(&pb, 0);
>> +    bytestream2_put_byte(&pb, 0);
>> +
>> +    dqt_offset[field] = bytestream2_tell_p(&pb);
>> +    bytestream2_put_be16(&pb, 132);
>> +    bytestream2_put_byte(&pb, 0);
>> +    bytestream2_skip(&gb, 4);
>> +    for (int i = 0; i < 64; i++)
>> +        bytestream2_put_byte(&pb, bytestream2_get_be32(&gb));
>> +    bytestream2_put_byte(&pb, 1);
>> +    for (int i = 0; i < 64; i++)
>> +        bytestream2_put_byte(&pb, bytestream2_get_be32(&gb));
>> +
>> +    dht_offset[field] = 0;
>> +    sod_offset[field] = bytestream2_tell_p(&pb);
>> +
>> +    for (int i = bytestream2_tell(&gb) + 8; next_field == 0 && i <
>> avpkt->size - 4; i++) {
>> +        if (AV_RB32(avpkt->data + i) == 0x00000001) {
>> +            next_field = i;
>> +            break;
>> +        }
>> +    }
>> +
>> +    bytestream2_skip(&gb, 8);
>> +    bytestream2_copy_buffer(&pb, &gb, next_field -
>> bytestream2_tell(&gb));
>> +    bytestream2_put_be64(&pb, 0);
>> +
>> +    if (field == 0) {
>> +        field = 1;
>> +        second_field_offset = bytestream2_tell_p(&pb);
>> +        next_field = avpkt->size;
>> +        goto second_field;
>> +    }
>> +
>> +    AV_WB32(pkt->data +  8, second_field_offset);
>> +    AV_WB32(pkt->data + 12, second_field_offset);
>> +    AV_WB32(pkt->data + 16, second_field_offset);
>> +    AV_WB32(pkt->data + 20, dqt_offset[0]);
>> +    AV_WB32(pkt->data + 24, dht_offset[0]);
>> +    AV_WB32(pkt->data + 28, sof_offset[0]);
>> +    AV_WB32(pkt->data + 32, sos_offset[0]);
>> +    AV_WB32(pkt->data + 36, sod_offset[0]);
>> +
>> +    AV_WB32(pkt->data + second_field_offset +  8, bytestream2_tell_p(&pb)
>> - second_field_offset);
>> +    AV_WB32(pkt->data + second_field_offset + 12, bytestream2_tell_p(&pb)
>> - second_field_offset);
>> +    AV_WB32(pkt->data + second_field_offset + 16, 0);
>> +    AV_WB32(pkt->data + second_field_offset + 20, dqt_offset[1] -
>> second_field_offset);
>> +    AV_WB32(pkt->data + second_field_offset + 24, dht_offset[1]);
>> +    AV_WB32(pkt->data + second_field_offset + 28, sof_offset[1] -
>> second_field_offset);
>> +    AV_WB32(pkt->data + second_field_offset + 32, sos_offset[1] -
>> second_field_offset);
>> +    AV_WB32(pkt->data + second_field_offset + 36, sod_offset[1] -
>> second_field_offset);
>> +
>> +    pkt->size = bytestream2_tell_p(&pb);
>> +
>> +    ret = avcodec_send_packet(ctx->avctx, pkt);
>> +    if (ret < 0) {
>> +        av_log(avctx, AV_LOG_ERROR, "Error submitting a packet for
>> decoding\n");
>> +        return ret;
>> +    }
>> +
>> +    ret = avcodec_receive_frame(ctx->avctx, frame);
>> +    if (ret < 0)
>> +        return ret;
>> +
>> +    *got_frame = 1;
>> +
>> +    return avpkt->size;
>> +}
>> +
>> +static av_cold int media100_decode_end(AVCodecContext *avctx)
>> +{
>> +    Media100Context *ctx = avctx->priv_data;
>> +
>> +    avcodec_free_context(&ctx->avctx);
>> +
>> +    return 0;
>> +}
>> +
>> +const FFCodec ff_media100_decoder = {
>> +    .p.name           = "media100",
>> +    CODEC_LONG_NAME("Media 100"),
>> +    .p.type           = AVMEDIA_TYPE_VIDEO,
>> +    .p.id             = AV_CODEC_ID_MEDIA100,
>> +    .priv_data_size   = sizeof(Media100Context),
>> +    .init             = media100_decode_init,
>> +    .close            = media100_decode_end,
>> +    FF_CODEC_DECODE_CB(media100_decode_frame),
>> +    .caps_internal    = FF_CODEC_CAP_INIT_CLEANUP,
>> +};
>> diff --git a/libavformat/isom_tags.c b/libavformat/isom_tags.c
>> index 362cb77e8f..b1f1193234 100644
>> --- a/libavformat/isom_tags.c
>> +++ b/libavformat/isom_tags.c
>> @@ -274,6 +274,10 @@ const AVCodecTag ff_codec_movvideo_tags[] = {
>>
>>      { AV_CODEC_ID_RAWVIDEO, MKTAG('B', 'G', 'G', 'R') }, /* ASC Bayer
>> BGGR */
>>
>> +    { AV_CODEC_ID_MEDIA100, MKTAG('d', 't', 'P', 'A') },
>> +    { AV_CODEC_ID_MEDIA100, MKTAG('d', 't', 'n', 't') },
>> +    { AV_CODEC_ID_MEDIA100, MKTAG('6', '0', '1', 'P') },
>> +
>>      { AV_CODEC_ID_NONE, 0 },
>>  };
>>
>> -- 2.37.2
>
> _______________________________________________
> 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".