[FFmpeg-devel] [PATCH] lavf: Add DICOM demuxer, lavc: Add DICOM decoder

Wed Aug 21 21:58:28 EEST 2019

-------- Forwarded Message --------
Subject: 	Re: [FFmpeg-devel] [PATCH] lavf: Add DICOM demuxer, lavc: Add 
DICOM decoder
Date: 	Thu, 22 Aug 2019 00:27:55 +0530
From: 	Shivam <shivgo at iitk.ac.in>
To: 	Michael Niedermayer <michael at niedermayer.cc>

On 8/21/19 2:00 AM, Michael Niedermayer wrote:
> On Tue, Aug 20, 2019 at 08:53:43PM +0530, Shivam wrote:
>> Sorry, for my previous mail, i forgot to attach the patch.
>>
>> The patch contains support for Dicom files. The below features are 
>> supported
>> yet:-
>> Uncompressed DICOM files using any of the Implicit and Explicit VR 
>> formats.
>> Multiframe files are also supported.
>> To extract the metadata or info about the procedure, I have added an 
>> option
>> "-metadata." (The tags present in Dicom dictionary would be demuxed).
>>
>> I have also uploaded DICOM samples here.
>> https://1drv.ms/u/s!AosJ9_SrP4Tsh2WoPfQAI8cSsqUs?e=ZMd5fR
>>
>> Please review,
>>
>> Thank you,
>> Shivam Goyal
>>
>> Changelog | 1
>> libavcodec/Makefile | 1
>> libavcodec/allcodecs.c | 1
>> libavcodec/avcodec.h | 1
>> libavcodec/codec_desc.c | 7
>> libavcodec/dicom.c | 189 ++++++++++++
>> libavcodec/version.h | 2
>> libavformat/Makefile | 1
>> libavformat/allformats.c | 1
>> libavformat/dicom.h | 108 +++++++
>> libavformat/dicomdec.c | 594 ++++++++++++++++++++++++++++++++++++++
>> libavformat/dicomdict.c | 716 
>> +++++++++++++++++++++++++++++++++++++++++++++++
>> libavformat/version.h | 2
>> 13 files changed, 1622 insertions(+), 2 deletions(-)
>> d47b7ad6a9f16ce4e29a67a99800183f9056062d add_dicom.patch
>> From cd7ebe834278b29b0429b3f5b377154490ee159f Mon Sep 17 00:00:00 2001
>> From: Shivam Goyal <1998.goyal.shivam at gmail.com>
>> Date: Tue, 20 Aug 2019 20:03:02 +0530
>> Subject: [PATCH] lavc: add DICOM decoder, lavf: add DICOM demuxer
>>
>> ---
>> Changelog | 1 +
>> libavcodec/Makefile | 1 +
>> libavcodec/allcodecs.c | 1 +
>> libavcodec/avcodec.h | 1 +
>> libavcodec/codec_desc.c | 7 +
>> libavcodec/dicom.c | 189 +++++++++++
>> libavcodec/version.h | 2 +-
>> libavformat/Makefile | 1 +
>> libavformat/allformats.c | 1 +
>> libavformat/dicom.h | 108 ++++++
>> libavformat/dicomdec.c | 594 ++++++++++++++++++++++++++++++++
>> libavformat/dicomdict.c | 716 +++++++++++++++++++++++++++++++++++++++
>> libavformat/version.h | 2 +-
> libavformat and libavcodec changes should be in seperate patches
>
>
>> 13 files changed, 1622 insertions(+), 2 deletions(-)
>> create mode 100644 libavcodec/dicom.c
>> create mode 100644 libavformat/dicom.h
>> create mode 100644 libavformat/dicomdec.c
>> create mode 100644 libavformat/dicomdict.c
>>
>> diff --git a/Changelog b/Changelog
>> index c1f1237770..e04c3aa5f6 100644
>> --- a/Changelog
>> +++ b/Changelog
>> @@ -4,6 +4,7 @@ releases are sorted from youngest to oldest.
>> version <next>:
>> - Intel QSV-accelerated MJPEG decoding
>> - Intel QSV-accelerated VP9 decoding
>> +- DICOM decoder and demxer
>> version 4.2:
>> - tpad filter
>> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
>> index e49188357b..799da8aef7 100644
>> --- a/libavcodec/Makefile
>> +++ b/libavcodec/Makefile
>> @@ -263,6 +263,7 @@ OBJS-$(CONFIG_DCA_DECODER) += dcadec.o dca.o 
>> dcadata.o dcahuff.o \
>> OBJS-$(CONFIG_DCA_ENCODER) += dcaenc.o dca.o dcadata.o dcahuff.o \
>> dcaadpcm.o
>> OBJS-$(CONFIG_DDS_DECODER) += dds.o
>> +OBJS-$(CONFIG_DICOM_DECODER) += dicom.o
>> OBJS-$(CONFIG_DIRAC_DECODER) += diracdec.o dirac.o diracdsp.o 
>> diractab.o \
>> dirac_arith.o dirac_dwt.o dirac_vlc.o
>> OBJS-$(CONFIG_DFA_DECODER) += dfa.o
>> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
>> index 22985325e0..02a1afa7e8 100644
>> --- a/libavcodec/allcodecs.c
>> +++ b/libavcodec/allcodecs.c
>> @@ -83,6 +83,7 @@ extern AVCodec ff_cscd_decoder;
>> extern AVCodec ff_cyuv_decoder;
>> extern AVCodec ff_dds_decoder;
>> extern AVCodec ff_dfa_decoder;
>> +extern AVCodec ff_dicom_decoder;
>> extern AVCodec ff_dirac_decoder;
>> extern AVCodec ff_dnxhd_encoder;
>> extern AVCodec ff_dnxhd_decoder;
>> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
>> index d234271c5b..756e168c75 100644
>> --- a/libavcodec/avcodec.h
>> +++ b/libavcodec/avcodec.h
>> @@ -410,6 +410,7 @@ enum AVCodecID {
>> AV_CODEC_ID_SCREENPRESSO,
>> AV_CODEC_ID_RSCC,
>> AV_CODEC_ID_AVS2,
>> + AV_CODEC_ID_DICOM,
>> AV_CODEC_ID_Y41P = 0x8000,
>> AV_CODEC_ID_AVRP,
>> diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
>> index 4d033c20ff..ae9abdb2ba 100644
>> --- a/libavcodec/codec_desc.c
>> +++ b/libavcodec/codec_desc.c
>> @@ -1403,6 +1403,13 @@ static const AVCodecDescriptor 
>> codec_descriptors[] = {
>> .long_name = NULL_IF_CONFIG_SMALL("AVS2-P2/IEEE1857.4"),
>> .props = AV_CODEC_PROP_LOSSY,
>> },
>> + {
>> + .id = AV_CODEC_ID_DICOM,
>> + .type = AVMEDIA_TYPE_VIDEO,
>> + .name = "dicom",
>> + .long_name = NULL_IF_CONFIG_SMALL("DICOM (Digital Imaging and 
>> Communications in Medicine)"),
>> + .props = AV_CODEC_PROP_LOSSY | AV_CODEC_PROP_LOSSLESS,
>> + },
>> {
>> .id = AV_CODEC_ID_Y41P,
>> .type = AVMEDIA_TYPE_VIDEO,
>> diff --git a/libavcodec/dicom.c b/libavcodec/dicom.c
>> new file mode 100644
>> index 0000000000..eaa378d944
>> --- /dev/null
>> +++ b/libavcodec/dicom.c
>> @@ -0,0 +1,189 @@
>> +/*
>> + * DICOM decoder
>> + * Copyright (c) 2019 Shivam Goyal
>> + *
>> + * 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
>> + */
>> +
>> +#include <inttypes.h>
>> +#include "libavutil/dict.h"
>> +#include "avcodec.h"
>> +#include "bytestream.h"
>> +#include "internal.h"
>> +
>> +typedef struct DICOMopts {
>> + const AVClass *class;
>> + int interpret; // streams extradata
>> + int pixrep;
>> + int pixpad;
>> + int slope;
>> + int intcpt;
>> +} DICOMopts;
>> +
>> +
>> +static int extract_ed(AVCodecContext *avctx)
>> +{
>> + DICOMopts *dcmopts = avctx->priv_data;
>> + uint8_t *ed = avctx->extradata;
>> + int ed_s = avctx->extradata_size;
>> + const uint8_t **b = &avctx->extradata;
>> +
>> + dcmopts->interpret = 0x02; // Set defaults
>> + dcmopts->slope = 1;
>> + dcmopts->intcpt = 0;
>> + dcmopts->pixpad = 1 << 31;
>> + dcmopts->pixrep = 0;
>> +
>> + if (ed_s <= AV_INPUT_BUFFER_PADDING_SIZE)
>> + return -1;
> This check looks odd, the input padding size would be after the specified
> size of extradata
Ok, i will change this.
>
>
>> + dcmopts->interpret = bytestream_get_le32(b);
>> + dcmopts->pixrep = bytestream_get_le32(b);
>> + dcmopts->pixpad = bytestream_get_le32(b);
>> + dcmopts->slope = bytestream_get_le32(b);
>> + dcmopts->intcpt = bytestream_get_le32(b);
>> + avctx->extradata = ed;
> I think its less confusing if you dont modify avctx->extradata

change it too;

>
>
>> + return 0;
>> +}
>> +
>> +static uint8_t apply_transfm(int64_t val, int64_t bitmask, int pix_pad,
>> + int slope, int intercept, int w, int l, int interpret)
>> +{
>> + int max, min;
>> + uint8_t ret;
>> +
>> + if (val == pix_pad)
>> + return 0;
>> + if (val > 0)
>> + val = (val & bitmask);
>> + val = slope * val + intercept; // Linear Transformation
>> +
>> + max = l + w / 2 - 1;
>> + min = l - w / 2;
>> + if (val > max)
>> + ret = 255;
>> + else if (val <= min)
>> + ret = 0;
>> + else
>> + ret = (val - min) * 255 / (max - min);
>> + if (interpret == 0x01) // Monochrome1
>> + ret = 255 - ret;
>> + return ret;
>> +}
>> +
>> +// DICOM MONOCHROME1 and MONOCHROME2 decoder
>> +static int decode_mono( AVCodecContext *avctx,
>> + const uint8_t *buf,
>> + AVFrame *p)
>> +{
>> + int w, l, bitsalloc, pixrep, pixpad, slope, intcpt, interpret, ret;
>> + DICOMopts *dcmopts = avctx->priv_data;
>> + uint8_t *out;
>> + int64_t bitmask, pix;
>> + uint64_t i, size;
>> +
>> + avctx->pix_fmt = AV_PIX_FMT_GRAY8;
>> + if ((ret = ff_get_buffer(avctx, p, 0)) < 0)
>> + return ret;
>> + p->pict_type = AV_PICTURE_TYPE_I;
>> + p->key_frame = 1;
>> + out = p->data[0];
>> +
>> + w = avctx->profile;
>> + l = avctx->level;
>> + size = avctx->width * avctx->height;
>> + bitsalloc = avctx->bits_per_raw_sample;
>> + bitmask = (1 << avctx->bits_per_coded_sample) - 1;
>> + interpret = dcmopts->interpret;
>> + pixrep = dcmopts->pixrep;
>> + pixpad = dcmopts->pixpad;
>> + slope = dcmopts->slope;
>> + intcpt = dcmopts->intcpt;
>> +
>> + switch (bitsalloc) {
>> + case 8:
>> + for (i = 0; i < size; i++) {
>> + if (pixrep)
>> + pix = (int8_t)bytestream_get_byte(&buf);
>> + else
>> + pix = (uint8_t)bytestream_get_byte(&buf);
>> + out[i] = pix;
>> + }
>> + break;
>> + case 16:
>> + for (i = 0; i < size; i++) {
>> + if (pixrep)
>> + pix = (int16_t)bytestream_get_le16(&buf);
>> + else
>> + pix = (uint16_t)bytestream_get_le16(&buf);
>> + out[i] = apply_transfm(pix, bitmask, pixpad, slope, intcpt, w, l, 
>> interpret);
>> + }
>> + break;
>> + case 32:
>> + for (i = 0; i < size; i++) {
>> + if (pixrep)
>> + pix = (int32_t)bytestream_get_le32(&buf);
>> + else
>> + pix = (uint32_t)bytestream_get_le32(&buf);
>> + out[i] = apply_transfm(pix, bitmask, pixpad, slope, intcpt, w, l, 
>> interpret);
>> + }
>> + break;
>> + default:
>> + av_log(avctx, AV_LOG_ERROR, "Bits allocated %d not supported\n", 
>> bitsalloc);
>> + return AVERROR_INVALIDDATA;
>> + }
>> + return 0;
>> +}
>> +
>> +static int dicom_decode_frame(AVCodecContext *avctx,
>> + void *data, int *got_frame,
>> + AVPacket *avpkt)
>> +{
>> + DICOMopts *dcmopts = avctx->priv_data;
>> + const uint8_t *buf = avpkt->data;
>> + int buf_size = avpkt->size;
>> + AVFrame *p = data;
>> + int ret, req_buf_size;
>> +
>> + req_buf_size = avctx->width * avctx->height * 
>> avctx->bits_per_raw_sample / 8;
>> + if (buf_size < req_buf_size) {
>> + av_log(avctx, AV_LOG_ERROR, "Required buffer size is %d but 
>> recieved only %d\n", req_buf_size, buf_size);
>> + return AVERROR_INVALIDDATA;
>> + }
>> + extract_ed(avctx);
>> + switch (dcmopts->interpret) {
>> + case 0x01: // MONOCHROME1
>> + case 0x02: // MONOCHROME2
>> + ret = decode_mono(avctx, buf, p);
>> + if (ret < 0)
>> + return ret;
>> + break;
>> + default:
>> + av_log(avctx, AV_LOG_ERROR, "Provided photometric interpretation 
>> not supported\n");
>> + return AVERROR_INVALIDDATA;
>> + }
>> + *got_frame = 1;
>> + return buf_size;
>> +}
>> +
>> +AVCodec ff_dicom_decoder = {
>> + .name = "dicom",
>> + .long_name = NULL_IF_CONFIG_SMALL("DICOM (Digital Imaging and 
>> Communications in Medicine)"),
>> + .type = AVMEDIA_TYPE_VIDEO,
>> + .priv_data_size = sizeof(DICOMopts),
>> + .id = AV_CODEC_ID_DICOM,
>> + .decode = dicom_decode_frame,
>> +};
>> diff --git a/libavcodec/version.h b/libavcodec/version.h
>> index 43c8cdb59f..ed767c8867 100644
>> --- a/libavcodec/version.h
>> +++ b/libavcodec/version.h
>> @@ -28,7 +28,7 @@
>> #include "libavutil/version.h"
>> #define LIBAVCODEC_VERSION_MAJOR 58
>> -#define LIBAVCODEC_VERSION_MINOR 55
>> +#define LIBAVCODEC_VERSION_MINOR 56
>> #define LIBAVCODEC_VERSION_MICRO 100
>> #define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
>> diff --git a/libavformat/Makefile b/libavformat/Makefile
>> index a434b005a4..58ba6a4c36 100644
>> --- a/libavformat/Makefile
>> +++ b/libavformat/Makefile
>> @@ -150,6 +150,7 @@ OBJS-$(CONFIG_DAUD_MUXER) += daudenc.o
>> OBJS-$(CONFIG_DCSTR_DEMUXER) += dcstr.o
>> OBJS-$(CONFIG_DFA_DEMUXER) += dfa.o
>> OBJS-$(CONFIG_DHAV_DEMUXER) += dhav.o
>> +OBJS-$(CONFIG_DICOM_DEMUXER) += dicomdec.o dicomdict.o
>> OBJS-$(CONFIG_DIRAC_DEMUXER) += diracdec.o rawdec.o
>> OBJS-$(CONFIG_DIRAC_MUXER) += rawenc.o
>> OBJS-$(CONFIG_DNXHD_DEMUXER) += dnxhddec.o rawdec.o
>> diff --git a/libavformat/allformats.c b/libavformat/allformats.c
>> index cd00834807..c5120ef1df 100644
>> --- a/libavformat/allformats.c
>> +++ b/libavformat/allformats.c
>> @@ -111,6 +111,7 @@ extern AVOutputFormat ff_daud_muxer;
>> extern AVInputFormat ff_dcstr_demuxer;
>> extern AVInputFormat ff_dfa_demuxer;
>> extern AVInputFormat ff_dhav_demuxer;
>> +extern AVInputFormat ff_dicom_demuxer;
>> extern AVInputFormat ff_dirac_demuxer;
>> extern AVOutputFormat ff_dirac_muxer;
>> extern AVInputFormat ff_dnxhd_demuxer;
>> diff --git a/libavformat/dicom.h b/libavformat/dicom.h
>> new file mode 100644
>> index 0000000000..cffe80ada1
>> --- /dev/null
>> +++ b/libavformat/dicom.h
>> @@ -0,0 +1,108 @@
>> +/*
>> + * DICOM demuxer
>> + *
>> + * Copyright (c) 2019 Shivam Goyal
>> + *
>> + * 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
>> + */
>> +
>> +
>> +#define DICOM_PREAMBLE_SIZE 128
>> +#define DICOM_PREFIX_SIZE 4
>> +
>> +#define IMAGE_GR_NB 0x0028
>> +#define MF_GR_NB 0x0018
>> +#define PIXEL_GR_NB 0x7FE0
>> +#define PIXELDATA_EL_NB 0x0010
>> +#define TS_GR_NB 0x0002
>> +#define TS_EL_NB 0x0010
>> +#define UNDEFINED_VL 0xFFFFFFFF
>> +#define DEFAULT_WINDOW 1100
>> +#define DEFAULT_LEVEL 125
>> +
>> +#define SEQ_GR_NB 0xFFFE
>> +#define SEQ_DEL_EL_NB 0xE0DD
>> +#define SEQ_ITEM_BEG_EL_NB 0xE000
>> +#define SEQ_ITEM_DEL_EL_NB 0xE00D
>> +#define MAX_UNDEF_LENGTH 10000 // max undefined length
>> +#define MAX_SEQ_LENGTH 20 // max sequence length (items)
>> +
>> +typedef enum {
>> + UNSUPPORTED_TS = 0,
>> + IMPLICIT_VR = 1,
>> + EXPLICIT_VR = 2,
>> + DEFLATE_EXPLICIT_VR = 3,
>> + JPEG_BASE_8 = 4,
>> + JPEG_EXT_12 = 5,
>> + JPEG_LOSSLESS_NH_P14 = 6,
>> + JPEG_LOSSLESS_NH_P14_S1 = 7,
>> + JPEG_LS_LOSSLESS = 8,
>> + JPEG_LS_LOSSY = 9,
>> + JPEG2000_LOSSLESS = 10,
>> + JPEG2000 = 11,
>> + RLE = 12
>> +} TransferSyntax;
>> +
>> +typedef enum {
>> + AE = 0x4145,
>> + AS = 0x4153,
>> + AT = 0x4154,
>> + CS = 0x4353,
>> + DA = 0x4441,
>> + DS = 0x4453,
>> + DT = 0x4454,
>> + FD = 0x4644,
>> + FL = 0x464c,
>> + IS = 0x4953,
>> + LO = 0x4c4f,
>> + LT = 0x4c54,
>> + OB = 0x4f42,
>> + OD = 0x4f44,
>> + OF = 0x4f46,
>> + OL = 0x4f4c,
>> + OV = 0x4f56,
>> + OW = 0x4f57,
>> + PN = 0x504e,
>> + SH = 0x5348,
>> + SL = 0x534c,
>> + SQ = 0x5351,
>> + SS = 0x5353,
>> + ST = 0x5354,
>> + SV = 0x5356,
>> + TM = 0x544d,
>> + UC = 0x5543,
>> + UI = 0x5549,
>> + UL = 0x554c,
>> + UN = 0x554e,
>> + UR = 0x5552,
>> + US = 0x5553,
>> + UT = 0x5554,
>> + UV = 0x5556
>> +} ValueRepresentation;
>> +
>> +
>> +typedef struct DataElement {
>> + uint16_t GroupNumber;
>> + uint16_t ElementNumber;
>> + ValueRepresentation VR;
>> + int64_t VL;
>> + void* bytes;
>> + int is_found; // is found in DICOM dictionary
>> + char* desc; // description (from dicom dictionary)
>> +} DataElement;
>> +
>> +int dicom_dict_find_elem_info (DataElement *de);
>> diff --git a/libavformat/dicomdec.c b/libavformat/dicomdec.c
>> new file mode 100644
>> index 0000000000..c38d4311f9
>> --- /dev/null
>> +++ b/libavformat/dicomdec.c
>> @@ -0,0 +1,594 @@
>> +/*
>> + * DICOM demuxer
>> + *
>> + * Copyright (c) 2019 Shivam Goyal
>> + *
>> + * 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
>> + */
>> +
>> +#include "libavutil/avstring.h"
>> +#include "libavutil/dict.h"
>> +#include "libavutil/opt.h"
>> +#include "libavutil/bprint.h"
>> +#include "libavcodec/bytestream.h"
>> +#include "avformat.h"
>> +#include "internal.h"
>> +#include "dicom.h"
>> +
>> +
>> +typedef struct DICOMContext {
>> + const AVClass *class;
>> +
>> + int interpret, pixrep, slope, intcpt, samples_ppix;
>> + uint16_t width;
>> + uint16_t height;
>> + uint64_t nb_frames;
>> + uint64_t frame_size;
>> + int frame_nb;
>> + double delay;
>> + int duration;
>> + int inheader;
>> + int inseq;
>> + int32_t pixpad;
>> + TransferSyntax transfer_syntax;
>> +
>> + int window; // Options
>> + int level;
>> + int metadata;
>> +} DICOMContext;
>> +
>> +
>> +static int dicom_probe(const AVProbeData *p)
>> +{
>> + const uint8_t *d = p->buf;
>> +
>> + if (d[DICOM_PREAMBLE_SIZE] == 'D' &&
>> + d[DICOM_PREAMBLE_SIZE + 1] == 'I' &&
>> + d[DICOM_PREAMBLE_SIZE + 2] == 'C' &&
>> + d[DICOM_PREAMBLE_SIZE + 3] == 'M') {
>> + return AVPROBE_SCORE_MAX;
>> + }
>> + return 0;
>> +}
>> +
>> +static DataElement *alloc_de(void) {
>> + DataElement *de = (DataElement *) av_malloc(sizeof(DataElement));
>> + de->is_found = 0;
>> + de->desc = NULL;
>> + de->bytes = NULL;
>> + return de;
>> +}
>> +
>> +static void free_de(DataElement *de) {
>> + if (!de)
>> + return;
>> + if (de->bytes)
>> + av_free(de->bytes);
>> + if (de->desc)
>> + av_free(de->desc);
> av_free(NULL) is safe, no check needed
at first, it was giving error. So, i added a check, i would check it again.
>
>
>> + av_free(de);
>> +}
>> +
>> +static void set_context_defaults(DICOMContext *dicom) {
>> + dicom->interpret = 0x02; // 2 for MONOCHROME2, 1 for MONOCHROME1
>> + dicom->pixrep = 1;
>> + dicom->pixpad = 1 << 31;
>> + dicom->slope = 1;
>> + dicom->intcpt = 0;
>> + dicom->samples_ppix = 1;
>> +
>> + dicom->delay = 100;
>> + dicom->frame_nb = 1;
>> + dicom->nb_frames = 1;
>> + dicom->inseq = 0;
>> +}
>> +
>> +// detects transfer syntax
>> +static TransferSyntax get_transfer_sytax (const char *ts) {
>> + if (!strcmp(ts, "1.2.840.10008.1.2"))
>> + return IMPLICIT_VR;
>> + else if (!strcmp(ts, "1.2.840.10008.1.2.1"))
>> + return EXPLICIT_VR;
>> + else
>> + return UNSUPPORTED_TS;
>> +}
>> +
>> +static int find_PI(const char *pi) {
>> + if (!strcmp(pi, "MONOCHROME1 "))
>> + return 0x01;
>> + else if (!strcmp(pi, "MONOCHROME2 "))
>> + return 0x02;
>> + else if (!strcmp(pi, "PALETTE COLOR "))
>> + return 0x03;
>> + else if (!strcmp(pi, "RGB "))
>> + return 0x04;
>> + else return 0x00;
>> +}
>> +
>> +static void set_dec_extradata(AVFormatContext *s, AVStream *st) {
>> + DICOMContext *dicom = s->priv_data;
>> + uint8_t *edp;
>> + int size = 20 + AV_INPUT_BUFFER_PADDING_SIZE;
>> +
>> + st->codecpar->extradata = (uint8_t *)av_malloc(size);
>> + edp = st->codecpar->extradata;
>> + bytestream_put_le32(&st->codecpar->extradata, dicom->interpret);
>> + bytestream_put_le32(&st->codecpar->extradata, dicom->pixrep);
>> + bytestream_put_le32(&st->codecpar->extradata, dicom->pixpad);
>> + bytestream_put_le32(&st->codecpar->extradata, dicom->slope);
>> + bytestream_put_le32(&st->codecpar->extradata, dicom->intcpt);
>> + st->codecpar->extradata = edp;
>> + st->codecpar->extradata_size = size;
>> +}
>> +
>> +static double conv_DS(DataElement *de) {
>> + double ret;
>> + char *cbytes = av_malloc(de->VL + 1);
>> +
>> + memcpy(cbytes, de->bytes, de->VL);
>> + cbytes[de->VL] = 0;
>> + ret = atof(cbytes);
>> + av_free(cbytes);
>> + return ret;
>> +}
>> +
>> +static int conv_IS(DataElement *de) {
>> + int ret;
>> + char *cbytes = av_malloc(de->VL + 1);
>> +
>> + memcpy(cbytes, de->bytes, de->VL);
>> + cbytes[de->VL] = 0;
>> + ret = atoi(cbytes);
> this doesnt check for any errors
would change it, too
>
>
>> + av_free(cbytes);
>> + return ret;
>> +}
>> +
>> +
>> +static char *get_key_str(DataElement *de) {
>> + char *key;
>> + if (!de->GroupNumber || !de->ElementNumber)
>> + return 0;
>> + key = (char *) av_malloc(15 + strlen(de->desc));
>> + sprintf(key, "(%04x,%04x) %s", de->GroupNumber, de->ElementNumber, 
>> de->desc);
> please use snprintf() or some other function which checks the output space
ok,
>
> [...]
>
>> +
>> +static int set_imagegroup_data(AVFormatContext *s, AVStream* st, 
>> DataElement *de)
>> +{
>> + DICOMContext *dicom = s->priv_data;
>> + void *bytes = de->bytes;
>> + int len = de->VL;
>> + char *cbytes;
>> +
>> + if (de->GroupNumber != IMAGE_GR_NB)
>> + return 0;
>> +
>> + switch (de->ElementNumber) {
>> + case 0x0010: // rows
>> + dicom->height = *(uint16_t *)bytes;
> does this work on big endian ?
> maybe you where looking for AV_RL16()

Big endian DICOM files are retired and no longer supported by the standard.

>
>
> [...]
>> +int dicom_dict_find_elem_info(DataElement *de) {
>> + int len;
>> + DICOMDictionary *out;
>> +
>> + if (!de->GroupNumber || !de->ElementNumber)
>> + return -2;
>> + len = FF_ARRAY_ELEMS(dicom_dictionary);
>> +
>> + out = (DICOMDictionary *) bsearch(de, &dicom_dictionary, len,
>> + sizeof(dicom_dictionary[0]), dcm_dict_comp);
>> +
>> + if (!out)
>> + return -1;
>> + de->VR = out->vr;
>> + de->desc = av_strdup(out->desc);
> missing malloc failure check

would change it.

Thanks for the review,

Shivam Goyal