diff mbox

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

Message ID 3f2077be-182c-e7da-6850-09fab3022fdd@iitk.ac.in
State New
Headers show

Commit Message

Shivam Goyal Aug. 20, 2019, 3:23 p.m. UTC
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

Comments

Michael Niedermayer Aug. 20, 2019, 8:30 p.m. UTC | #1
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@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


> +    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


> +    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


> +    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 


> +    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

[...]

> +
> +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()


[...]
> +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

[...]
Moritz Barsnick Aug. 21, 2019, 1:57 p.m. UTC | #2
On Tue, Aug 20, 2019 at 20:53:43 +0530, Shivam wrote:
> Please review,

(Untested, just visual code inspection.)

> +- DICOM decoder and demxer

Typo -> demuxer. Also, when splitting the commits, also split the
changes to the Changelog. (Can still be one line eventually.)

> +        .props     = AV_CODEC_PROP_LOSSY | AV_CODEC_PROP_LOSSLESS,

Possibly also AV_CODEC_PROP_INTRA_ONLY? (PNG doesn't have that either,
but other still image formats do.) Is DICOM a still image format, or
does it have multiple images and a sense of I-frames?

> +static int extract_ed(AVCodecContext *avctx)

The return value is never used anywhere.

> +    int ed_s = avctx->extradata_size;

Feel free to name the variable with something containing "size", makes
the code somewhat easier to understand.

> +static uint8_t apply_transfm(int64_t val, int64_t bitmask, int pix_pad,
                             ^ I see no reason to save two letters in this name. ;-)

> +static int decode_mono( AVCodecContext *avctx,
> +                        const uint8_t *buf,
> +                        AVFrame *p)
                          ^ spurious space

> +    switch (bitsalloc) {

> +        case 8:

ffmpeg uses the same indentation level for "case" as for "switch".

> +            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;
> +            }

What is this doing? Is the cast to uint8_t an implicit "abs()"?
Could it just be:
               pix = pixrep ? bytestream_get_byte(&buf) : FFABS(bytestream_get_byte(&buf));
               out[i] = ...

Or in a somewhat different style:
               uintXY_t val = bytestream_get_byte(&buf);
               pix = pixrep ? byte : FFABS(byte);
                out[i] = ...

> +        default:
> +            av_log(avctx, AV_LOG_ERROR, "Bits allocated %d not supported\n", bitsalloc);
> +            return AVERROR_INVALIDDATA;

avctx->bits_per_coded_sample is a constant per file, right?
This "default" could be avoided if avctx->bits_per_coded_sample were
checked in init(), not in decode_frame().

> +        av_log(avctx, AV_LOG_ERROR, "Required buffer size is %d but recieved only %d\n", req_buf_size, buf_size);
typo: received

> +    void* bytes;

void *bytes

> +    char* desc; // description (from dicom dictionary)

char *desc;

> +    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;

Would:
  if (!strncmp(p->buf, "DICM", 4)
also work? Seems much simpler to me. (Also skipping the intermediate
"d" variable.)

> +    if (de->bytes)
> +        av_free(de->bytes);
> +    if (de->desc)
> +        av_free(de->desc);

As Michael said, av_free() includes the NULL checks already.
Additionally, I believe the use of av_freep() is preferred for these
pointers. (Provokes a segfault on use after free, instead of "silently"
writing into / reading from that memory.)

> +// detects transfer syntax
> +static TransferSyntax get_transfer_sytax (const char *ts) {
                                      ^ typo: syntax

Could you also please not name the variable "ts", as that already has
too many other meanings. ;-) (Use e.g. "syntax".)

> +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;
> +}

I'm not sure you're doing anything with edp here. Did you mean to use:
    bytestream_put_le32(&edp, dicom->interpret);
?

> +    sprintf(key, "(%04x,%04x) %s", de->GroupNumber, de->ElementNumber, de->desc);

As Michael said, this can overflow "key". *Always* use snprintf.

> +    switch(de->VR) {
> +        case AT:

Indentation.

> +static int set_multiframe_data(AVFormatContext *s, AVStream* st, DataElement *de)
> +{
> +    DICOMContext *dicom = s->priv_data;
> +
> +    if (de->GroupNumber != MF_GR_NB)
> +        return 0;
> +
> +    switch (de->ElementNumber) {
> +        case 0x1063: // frame time
> +            dicom->delay = conv_DS(de);
> +            dicom->duration = dicom->delay * dicom->nb_frames;
> +            break;
> +    }
> +    return 0;
> +}

Always returns 0.

Is this a switch/case, so that it can be expanded in the future?

> +        av_log(s, AV_LOG_WARNING,"Data Element Value length: %ld can't be odd\n", de->VL);

de->VL is int64_t, so the correct printf format is "%" PRIi64.

> +    if (de->VL < 0)
> +        return AVERROR_INVALIDDATA;

You could put this check before the odd check.

> +static int read_implicit_seq_item(AVFormatContext *s, DataElement *de)
> +{
> +    int ret, f = -2, i = 0;
> +
> +    de->bytes = av_malloc(MAX_UNDEF_LENGTH);
> +    for(; i < MAX_UNDEF_LENGTH; ++i) {

Unusual to initialize the "int i" above and not here, but okay.

> +        if (ret == SEQ_GR_NB)
> +            f = i;
> +        if (ret == SEQ_ITEM_DEL_EL_NB && f == i - 1) {

else if

> +static int read_seq(AVFormatContext *s, DataElement *de) {
> +    int i = 0, ret;
> +    DICOMContext *dicom = s->priv_data;
> +    DataElement *seq_data = (DataElement *)av_malloc(sizeof(DataElement) * MAX_SEQ_LENGTH);
> +    de->bytes = seq_data;
> +    dicom->inseq = 1;
> +    for (;i < MAX_SEQ_LENGTH; ++i) {

Unusual to initialize the "int i" above and not here, but okay.

> +        seq_data[i].bytes = NULL;
> +        seq_data[i].desc = NULL;
> +        seq_data[i].is_found = 0;
> +        read_de_metainfo(s, seq_data + i);
> +        if (seq_data[i].GroupNumber == SEQ_GR_NB
> +            && seq_data[i].ElementNumber == SEQ_DEL_EL_NB){

Nit: Missing space before curly bracket.

> +            ret = i;
> +            goto exit;
> +        }
> +        if (seq_data[i].VL == UNDEFINED_VL)
> +            ret = read_implicit_seq_item(s, seq_data + i);
> +        else
> +            ret = read_de(s, seq_data + i);
> +        if (ret < 0)
> +            goto exit;
> +    }
> +
> +exit:

How about just "break" instead of "goto exit", and drop the label? If
this is the only use of the label, goto is too confusing.

> +static int read_de_valuefield(AVFormatContext *s, DataElement *de) {
> +    if (de->VL == UNDEFINED_VL)
> +        return read_seq(s, de);
> +    else return read_de(s, de);

Line break after else, please.

> +    ret = read_de_metainfo(s, de);
> +    ret = read_de_valuefield(s, de);
> +    if (ret < 0) {
> +        free_de(de);
> +        return ret;
> +    }

The first assignment of ret ("ret = read_de_metainfo(s, de);") is
ignored. It can even return an error.

> +        av_log(s, AV_LOG_WARNING,"First data element is not \'File MetaInfo Group Length\', may fail to demux");

Missing space after ",". I believe the "'" shouldn't be escaped. And
missing a "\n" at the end.

> +        bytes_read += ret;
> +        value = get_val_str(de);

This allocates a pointer via get_val_str()...

> +        if (de->GroupNumber == TS_GR_NB && de->ElementNumber == TS_EL_NB)
> +            dicom->transfer_syntax = get_transfer_sytax(value);
> +
> +        av_dict_set(m, key, value, AV_DICT_DONT_STRDUP_KEY | AV_DICT_DONT_STRDUP_VAL);
> +        free_de(de);

... but doesn't free it.


> +    if (dicom->frame_nb > 1 && dicom->frame_nb <= dicom->nb_frames)
> +        goto inside_pix_data;

What's the effect (and readability) of jumping into the inside of a
for() loop's block? Can't this be made more readable and logical?
(Sorry, don't have a suggestion, just very confused by this code.)

> +        ret = read_de_metainfo(s,de);
> +        if (ret < 0)
> +            goto exit;

Again, since this is the body of a for() loop, wouldn't it be correct
to use "break" instread of "goto", achieving the same jump? "goto" is
used to avoid code duplication, not to avoid standard C code style. ;-)

> +                av_packet_unref(pkt);
> +                goto exit;

Same for allthe other "exit"s, presumably.

> +            if (ret < 0)
> +                goto end_de;

This on the other hand may be legitimate.

> +    end_de:
> +        free_de(de);
> +    }
> +exit:
> +    free_de(de);
> +    if (ret < 0)
> +        return ret;
> +    return AVERROR_EOF;
> +}
> +
> +static const AVOption options[] = {
> +    { "window", "override default window found in file", offsetof(DICOMContext, window), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 99999, AV_OPT_FLAG_DECODING_PARAM },
> +    { "level", "override default level found in file", offsetof(DICOMContext, level), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 99999, AV_OPT_FLAG_DECODING_PARAM },
> +    { "metadata", "skip or decode metadata", offsetof(DICOMContext, metadata)  , AV_OPT_TYPE_BOOL,{.i64 = 0}, 0, 1, AV_OPT_FLAG_DECODING_PARAM },

"true" is skip, "false" is decode? Or vice versa? Better just say what
happens when true "whether to decode metadata". (The default is
automatically documented here.)

Apropros: You should add documentation, also separately for demuxer and
decoder (but in their respective commits).

> +static const AVClass dicom_class = {
> +    .class_name = "dicomdec",

Looking at the other demuxers, this should probably be "DICOM demuxer".

> +static int dcm_dict_comp(const void *vde, const void *vDd) {
> +    DataElement *de = (DataElement *) vde;
> +    DICOMDictionary *Dd = (DICOMDictionary *) vDd;
> +
> +    if (de->GroupNumber > Dd->GroupNumber)
> +        return 1;
> +    else if (de->GroupNumber < Dd->GroupNumber)
> +        return -1;
> +    else {
> +        if (de->ElementNumber > Dd->ElementNumber)
> +            return 1;
> +        else if (de->ElementNumber < Dd->ElementNumber)
> +            return -1;
> +        else
> +            return 0;

We have the FFDIFFSIGN() macro for this.
:-)

This should work:

int ret;
ret = FFDIFFSIGN(de->GroupNumber, Dd->GroupNumber);
if (!ret)
    ret = FFDIFFSIGN(de->ElementNumber, Dd->ElementNumber);
return ret;

> +    if (!de->GroupNumber || !de->ElementNumber)
> +        return -2;

This is eventually returned by dicom_read_packet(). What is this "-2"
interpreted as by the caller of this .read_packet callback?

> +}
> \ No newline at end of file

Please add a newline to the last line of this file. ;-)

Cheers,
Moritz
Moritz Barsnick Aug. 21, 2019, 3:42 p.m. UTC | #3
On Tue, Aug 20, 2019 at 20:53:43 +0530, Shivam wrote:
> I have also uploaded DICOM samples here.
> https://1drv.ms/u/s!AosJ9_SrP4Tsh2WoPfQAI8cSsqUs?e=ZMd5fR

I found something else, with valgrind:

> +static int read_implicit_seq_item(AVFormatContext *s, DataElement *de)
> +{
> +    int ret, f = -2, i = 0;
> +
> +    de->bytes = av_malloc(MAX_UNDEF_LENGTH);

This return value needs to be checked!

> +    for(; i < MAX_UNDEF_LENGTH; ++i) {
> +        ret = avio_rl16(s->pb);
> +        if (ret < 0)
> +            return ret;
> +
> +        if (ret == SEQ_GR_NB)
> +            f = i;
> +        if (ret == SEQ_ITEM_DEL_EL_NB && f == i - 1) {
> +            avio_skip(s->pb, 4);
> +            break;
> +        }
> +        bytestream_put_le16((uint8_t **)&de->bytes, ret);
> +    }
> +    de->VL = (i - 1) * 2;
> +    return 0;
> +}
> +
> +static int read_seq(AVFormatContext *s, DataElement *de) {
> +    int i = 0, ret;
> +    DICOMContext *dicom = s->priv_data;
> +    DataElement *seq_data = (DataElement *)av_malloc(sizeof(DataElement) * MAX_SEQ_LENGTH);

This is an array, right? There's a special av_malloc*() function for
allocating an array. (And the typecast is not required, IIUC.)

Furthermore, it is never freed.

Fixing free_de() as suggested alone is not sufficient either, the array
elements remain unfreed - those allocated here as seq_data[i]:

> +    de->bytes = seq_data;
> +    dicom->inseq = 1;
> +    for (;i < MAX_SEQ_LENGTH; ++i) {
> +        seq_data[i].bytes = NULL;
> +        seq_data[i].desc = NULL;
> +        seq_data[i].is_found = 0;
> +        read_de_metainfo(s, seq_data + i);
> +        if (seq_data[i].GroupNumber == SEQ_GR_NB
> +            && seq_data[i].ElementNumber == SEQ_DEL_EL_NB){
> +            ret = i;
> +            goto exit;
> +        }
> +        if (seq_data[i].VL == UNDEFINED_VL)
> +            ret = read_implicit_seq_item(s, seq_data + i);

^ Here.

> +        else
> +            ret = read_de(s, seq_data + i);
> +        if (ret < 0)
> +            goto exit;
> +    }

valgrind extract:

==20223== 10,000 bytes in 1 blocks are definitely lost in loss record 16 of 16
==20223==    at 0x483AE4C: memalign (vg_replace_malloc.c:908)
==20223==    by 0x483AF59: posix_memalign (vg_replace_malloc.c:1072)
==20223==    by 0x5146F9: av_malloc (mem.c:87)
==20223==    by 0x454C80: read_implicit_seq_item (dicomdec.c:371)
==20223==    by 0x454C80: read_seq (dicomdec.c:410)
==20223==    by 0x454C80: read_de_valuefield (dicomdec.c:424)
==20223==    by 0x455312: dicom_read_packet (dicomdec.c:549)
==20223==    by 0x4649C3: ff_read_packet (utils.c:856)
==20223==    by 0x46594B: read_frame_internal (utils.c:1582)
==20223==    by 0x46A001: avformat_find_stream_info (utils.c:3781)
==20223==    by 0x4170BE: open_input_file (ffmpeg_opt.c:1126)
==20223==    by 0x418A95: open_files (ffmpeg_opt.c:3275)
==20223==    by 0x418A95: ffmpeg_parse_options (ffmpeg_opt.c:3315)
==20223==    by 0x411875: main (ffmpeg.c:4872)




Furthermore, all your samples 000*.dcm give me the following error:

[dicom @ 0xfc26c0] Could not find codec parameters for stream 0 (Video: dicom, none): unspecified size
Consider increasing the value for the 'analyzeduration' and 'probesize' options

Example:

[barsnick@paradise ffmpeg]$ ./ffmpeg_g -i 000028.dcm -f null -
ffmpeg version N-94609-g406b3e902f Copyright (c) 2000-2019 the FFmpeg developers
  built with gcc 8 (GCC)
  configuration: --disable-doc --disable-everything --disable-network --disable-vdpau --enable-indev=lavfi --enable-demuxer=dicom --enable-muxer='null,hash,framehash,md5,framemd5' --enable-encoder='wrapped_avframe,rawvideo,pcm_s16le' --enable-decoder='rawvideo,pcm_f64le,dicom' --enable-filter='color,testsrc,anoisesrc,null,aresample' --enable-protocol='pipe,file'
  libavutil      56. 33.100 / 56. 33.100
  libavcodec     58. 56.100 / 58. 56.100
  libavformat    58. 32.101 / 58. 32.101
  libavdevice    58.  9.100 / 58.  9.100
  libavfilter     7. 58.100 /  7. 58.100
  libswscale      5.  6.100 /  5.  6.100
  libswresample   3.  6.100 /  3.  6.100
[dicom @ 0xfc76c0] Could not find codec parameters for stream 0 (Video: dicom, none): unspecified size
Consider increasing the value for the 'analyzeduration' and 'probesize' options
Input #0, dicom, from '000028.dcm':
  Metadata:
    (0002,0001) File Meta Information Version: [Binary data]
    (0002,0002) Media Storage SOP Class UID: 1.2.840.10008.5.1.4.1.1.2
    (0002,0003) Media Storage SOP Inst UID: 1.3.6.1.4.1.14519.5.2.1.7014.4598.325035o92754234010375598120031
    (0002,0010) Transfer Syntax UID: 1.2.840.10008.1.2.1
    (0002,0012) Implementation Class UID: 1.2.40.0.13.1.1.1
    (0002,0013) Implementation Version Name: dcm4che-1.4.35
  Duration: N/A, start: 0.000000, bitrate: N/A
    Stream #0:0: Video: dicom, none, 1k tbr, 1k tbn
Output #0, null, to 'pipe:':
Output file #0 does not contain any stream


Having compiled the demuxer and decoder, I do now understand that DICOM
images can have multiple "frames". I'd still like to understand if
they're "I-frames" only, as remarked in my previous email.

Cheers,
Moritz
Moritz Barsnick Aug. 21, 2019, 4:09 p.m. UTC | #4
On Tue, Aug 20, 2019 at 20:53:43 +0530, Shivam wrote:

One more. Some more nitpicking around this. Please use valgrind with
all your samples and some demuxing and decoding command lines.

> +static int dicom_read_packet(AVFormatContext *s, AVPacket *pkt)
> +{
> +    DICOMContext *dicom = s->priv_data;
> +    int metadata = dicom->metadata, ret;
> +    AVDictionary **st_md;
> +    char *key, *value;
> +    AVStream  *st;
> +    DataElement *de;
> +
> +    if (s->nb_streams < 1) {
> +        st = avformat_new_stream(s, NULL);
> +        if (!st)
> +            return AVERROR(ENOMEM);
> +        st->codecpar->codec_id   = AV_CODEC_ID_DICOM;
> +        st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
> +        st->start_time = 0;
> +        avpriv_set_pts_info(st, 64, 1, 1000);
> +        if (dicom->window != -1)
> +            st->codecpar->profile = dicom->window;
> +        if (dicom->level != -1)
> +            st->codecpar->level = dicom->level;
> +    } else
> +        st = s->streams[0];
> +
> +    st_md = &st->metadata;
> +    dicom->inheader = 0;
> +    if (dicom->frame_nb > 1 && dicom->frame_nb <= dicom->nb_frames)
> +        goto inside_pix_data;
> +
> +    for (;;) {
> +        ret = avio_feof(s->pb);
> +        if (ret)
> +            return AVERROR_EOF;
> +
> +        de = alloc_de();

Allocation failure needs to be checked:
        if (!de)
            return AVERROR(ENOMEM);

> +        ret = read_de_metainfo(s,de);
> +        if (ret < 0)
> +            goto exit;
> +
> +        if (de->GroupNumber == PIXEL_GR_NB && de->ElementNumber == PIXELDATA_EL_NB) {
> +            dicom->frame_size = de->VL / dicom->nb_frames;
> +            set_dec_extradata(s, st);
> +        inside_pix_data:
> +            if (av_new_packet(pkt, dicom->frame_size) < 0)
> +                return AVERROR(ENOMEM);

This leaks de. Instead:
            if (av_new_packet(pkt, dicom->frame_size) < 0) {
                ret = AVERROR(ENOMEM);
                goto exit;
            }

> +            pkt->pos = avio_tell(s->pb);
> +            pkt->stream_index = 0;
> +            pkt->size = dicom->frame_size;
> +            pkt->duration = dicom->delay;
> +            ret = avio_read(s->pb, pkt->data, dicom->frame_size);
> +            if (ret < 0) {
> +                av_packet_unref(pkt);
> +                goto exit;
> +            }
> +            dicom->frame_nb ++;
> +            return ret;

This leaks everything (i.e. de and pkt). Instead:
            dicom->frame_nb ++;
            av_packet_unref(pkt);
            goto exit;


> +        } else if (de->GroupNumber == IMAGE_GR_NB || de->GroupNumber == MF_GR_NB) {
> +            ret = read_de_valuefield(s, de);
> +            if (ret < 0)
> +                goto exit;
> +            set_imagegroup_data(s, st, de);
> +            set_multiframe_data(s, st, de);
> +        } else {
> +            if (metadata || de->VL == UNDEFINED_VL)
> +                ret = read_de_valuefield(s, de);
> +            else
> +                ret = avio_skip(s->pb, de->VL); // skip other elements
> +            if (ret < 0)
> +                goto exit;
> +        }
> +        if (metadata) {
> +            ret = dicom_dict_find_elem_info(de);
> +            if (ret < 0)
> +                goto end_de;
> +            key = get_key_str(de);
> +            value = get_val_str(de);
> +            av_dict_set(st_md, key, value, AV_DICT_DONT_STRDUP_KEY | AV_DICT_DONT_STRDUP_VAL);
> +        }
> +    end_de:
> +        free_de(de);
> +    }
> +exit:
> +    free_de(de);
> +    if (ret < 0)
> +        return ret;
> +    return AVERROR_EOF;
> +}

And, as mentioned, "goto exit" can probably be a simple "break", but
that's up to you to verify that it still does the right thing.

Cheers,
Moritz
Shivam Goyal Aug. 22, 2019, 5:44 a.m. UTC | #5
On 8/21/19 7:27 PM, Moritz Barsnick wrote:
> On Tue, Aug 20, 2019 at 20:53:43 +0530, Shivam wrote:
>> Please review,
> (Untested, just visual code inspection.)
>
>> +- DICOM decoder and demxer
> Typo -> demuxer. Also, when splitting the commits, also split the
> changes to the Changelog. (Can still be one line eventually.)
>
>> +        .props     = AV_CODEC_PROP_LOSSY | AV_CODEC_PROP_LOSSLESS,
> Possibly also AV_CODEC_PROP_INTRA_ONLY? (PNG doesn't have that either,
> but other still image formats do.) Is DICOM a still image format, or
> does it have multiple images and a sense of I-frames?
>
>> +static int extract_ed(AVCodecContext *avctx)
> The return value is never used anywhere.
>
>> +    int ed_s = avctx->extradata_size;
> Feel free to name the variable with something containing "size", makes
> the code somewhat easier to understand.
>
>> +static uint8_t apply_transfm(int64_t val, int64_t bitmask, int pix_pad,
>                               ^ I see no reason to save two letters in this name. ;-)
>
>> +static int decode_mono( AVCodecContext *avctx,
>> +                        const uint8_t *buf,
>> +                        AVFrame *p)
>                            ^ spurious space
>
>> +    switch (bitsalloc) {
>> +        case 8:
> ffmpeg uses the same indentation level for "case" as for "switch".
>
>> +            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;
>> +            }
> What is this doing? Is the cast to uint8_t an implicit "abs()"?
> Could it just be:
>                 pix = pixrep ? bytestream_get_byte(&buf) : FFABS(bytestream_get_byte(&buf));
>                 out[i] = ...
>
> Or in a somewhat different style:
>                 uintXY_t val = bytestream_get_byte(&buf);
>                 pix = pixrep ? byte : FFABS(byte);
>                  out[i] = ...
>
>> +        default:
>> +            av_log(avctx, AV_LOG_ERROR, "Bits allocated %d not supported\n", bitsalloc);
>> +            return AVERROR_INVALIDDATA;
> avctx->bits_per_coded_sample is a constant per file, right?
> This "default" could be avoided if avctx->bits_per_coded_sample were
> checked in init(), not in decode_frame().
>
>> +        av_log(avctx, AV_LOG_ERROR, "Required buffer size is %d but recieved only %d\n", req_buf_size, buf_size);
> typo: received
>
>> +    void* bytes;
> void *bytes
>
>> +    char* desc; // description (from dicom dictionary)
> char *desc;
>
>> +    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;
> Would:
>    if (!strncmp(p->buf, "DICM", 4)
> also work? Seems much simpler to me. (Also skipping the intermediate
> "d" variable.)
>
>> +    if (de->bytes)
>> +        av_free(de->bytes);
>> +    if (de->desc)
>> +        av_free(de->desc);
> As Michael said, av_free() includes the NULL checks already.
> Additionally, I believe the use of av_freep() is preferred for these
> pointers. (Provokes a segfault on use after free, instead of "silently"
> writing into / reading from that memory.)
>
>> +// detects transfer syntax
>> +static TransferSyntax get_transfer_sytax (const char *ts) {
>                                        ^ typo: syntax
>
> Could you also please not name the variable "ts", as that already has
> too many other meanings. ;-) (Use e.g. "syntax".)
>
>> +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;
>> +}
> I'm not sure you're doing anything with edp here. Did you mean to use:
>      bytestream_put_le32(&edp, dicom->interpret);
> ?
>
>> +    sprintf(key, "(%04x,%04x) %s", de->GroupNumber, de->ElementNumber, de->desc);
> As Michael said, this can overflow "key". *Always* use snprintf.
>
>> +    switch(de->VR) {
>> +        case AT:
> Indentation.
>
>> +static int set_multiframe_data(AVFormatContext *s, AVStream* st, DataElement *de)
>> +{
>> +    DICOMContext *dicom = s->priv_data;
>> +
>> +    if (de->GroupNumber != MF_GR_NB)
>> +        return 0;
>> +
>> +    switch (de->ElementNumber) {
>> +        case 0x1063: // frame time
>> +            dicom->delay = conv_DS(de);
>> +            dicom->duration = dicom->delay * dicom->nb_frames;
>> +            break;
>> +    }
>> +    return 0;
>> +}
> Always returns 0.
>
> Is this a switch/case, so that it can be expanded in the future?
>
>> +        av_log(s, AV_LOG_WARNING,"Data Element Value length: %ld can't be odd\n", de->VL);
> de->VL is int64_t, so the correct printf format is "%" PRIi64.
>
>> +    if (de->VL < 0)
>> +        return AVERROR_INVALIDDATA;
> You could put this check before the odd check.
>
>> +static int read_implicit_seq_item(AVFormatContext *s, DataElement *de)
>> +{
>> +    int ret, f = -2, i = 0;
>> +
>> +    de->bytes = av_malloc(MAX_UNDEF_LENGTH);
>> +    for(; i < MAX_UNDEF_LENGTH; ++i) {
> Unusual to initialize the "int i" above and not here, but okay.
>
>> +        if (ret == SEQ_GR_NB)
>> +            f = i;
>> +        if (ret == SEQ_ITEM_DEL_EL_NB && f == i - 1) {
> else if
>
>> +static int read_seq(AVFormatContext *s, DataElement *de) {
>> +    int i = 0, ret;
>> +    DICOMContext *dicom = s->priv_data;
>> +    DataElement *seq_data = (DataElement *)av_malloc(sizeof(DataElement) * MAX_SEQ_LENGTH);
>> +    de->bytes = seq_data;
>> +    dicom->inseq = 1;
>> +    for (;i < MAX_SEQ_LENGTH; ++i) {
> Unusual to initialize the "int i" above and not here, but okay.
>
>> +        seq_data[i].bytes = NULL;
>> +        seq_data[i].desc = NULL;
>> +        seq_data[i].is_found = 0;
>> +        read_de_metainfo(s, seq_data + i);
>> +        if (seq_data[i].GroupNumber == SEQ_GR_NB
>> +            && seq_data[i].ElementNumber == SEQ_DEL_EL_NB){
> Nit: Missing space before curly bracket.
>
>> +            ret = i;
>> +            goto exit;
>> +        }
>> +        if (seq_data[i].VL == UNDEFINED_VL)
>> +            ret = read_implicit_seq_item(s, seq_data + i);
>> +        else
>> +            ret = read_de(s, seq_data + i);
>> +        if (ret < 0)
>> +            goto exit;
>> +    }
>> +
>> +exit:
> How about just "break" instead of "goto exit", and drop the label? If
> this is the only use of the label, goto is too confusing.
>
>> +static int read_de_valuefield(AVFormatContext *s, DataElement *de) {
>> +    if (de->VL == UNDEFINED_VL)
>> +        return read_seq(s, de);
>> +    else return read_de(s, de);
> Line break after else, please.
>
>> +    ret = read_de_metainfo(s, de);
>> +    ret = read_de_valuefield(s, de);
>> +    if (ret < 0) {
>> +        free_de(de);
>> +        return ret;
>> +    }
> The first assignment of ret ("ret = read_de_metainfo(s, de);") is
> ignored. It can even return an error.
>
>> +        av_log(s, AV_LOG_WARNING,"First data element is not \'File MetaInfo Group Length\', may fail to demux");
> Missing space after ",". I believe the "'" shouldn't be escaped. And
> missing a "\n" at the end.
>
>> +        bytes_read += ret;
>> +        value = get_val_str(de);
> This allocates a pointer via get_val_str()...
>
>> +        if (de->GroupNumber == TS_GR_NB && de->ElementNumber == TS_EL_NB)
>> +            dicom->transfer_syntax = get_transfer_sytax(value);
>> +
>> +        av_dict_set(m, key, value, AV_DICT_DONT_STRDUP_KEY | AV_DICT_DONT_STRDUP_VAL);
>> +        free_de(de);
> ... but doesn't free it.
>
>
>> +    if (dicom->frame_nb > 1 && dicom->frame_nb <= dicom->nb_frames)
>> +        goto inside_pix_data;
> What's the effect (and readability) of jumping into the inside of a
> for() loop's block? Can't this be made more readable and logical?
> (Sorry, don't have a suggestion, just very confused by this code.)
>
>> +        ret = read_de_metainfo(s,de);
>> +        if (ret < 0)
>> +            goto exit;
> Again, since this is the body of a for() loop, wouldn't it be correct
> to use "break" instread of "goto", achieving the same jump? "goto" is
> used to avoid code duplication, not to avoid standard C code style. ;-)
>
>> +                av_packet_unref(pkt);
>> +                goto exit;
> Same for allthe other "exit"s, presumably.
>
>> +            if (ret < 0)
>> +                goto end_de;
> This on the other hand may be legitimate.
>
>> +    end_de:
>> +        free_de(de);
>> +    }
>> +exit:
>> +    free_de(de);
>> +    if (ret < 0)
>> +        return ret;
>> +    return AVERROR_EOF;
>> +}
>> +
>> +static const AVOption options[] = {
>> +    { "window", "override default window found in file", offsetof(DICOMContext, window), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 99999, AV_OPT_FLAG_DECODING_PARAM },
>> +    { "level", "override default level found in file", offsetof(DICOMContext, level), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 99999, AV_OPT_FLAG_DECODING_PARAM },
>> +    { "metadata", "skip or decode metadata", offsetof(DICOMContext, metadata)  , AV_OPT_TYPE_BOOL,{.i64 = 0}, 0, 1, AV_OPT_FLAG_DECODING_PARAM },
> "true" is skip, "false" is decode? Or vice versa? Better just say what
> happens when true "whether to decode metadata". (The default is
> automatically documented here.)
>
> Apropros: You should add documentation, also separately for demuxer and
> decoder (but in their respective commits).
>
>> +static const AVClass dicom_class = {
>> +    .class_name = "dicomdec",
> Looking at the other demuxers, this should probably be "DICOM demuxer".
>
>> +static int dcm_dict_comp(const void *vde, const void *vDd) {
>> +    DataElement *de = (DataElement *) vde;
>> +    DICOMDictionary *Dd = (DICOMDictionary *) vDd;
>> +
>> +    if (de->GroupNumber > Dd->GroupNumber)
>> +        return 1;
>> +    else if (de->GroupNumber < Dd->GroupNumber)
>> +        return -1;
>> +    else {
>> +        if (de->ElementNumber > Dd->ElementNumber)
>> +            return 1;
>> +        else if (de->ElementNumber < Dd->ElementNumber)
>> +            return -1;
>> +        else
>> +            return 0;
> We have the FFDIFFSIGN() macro for this.
> :-)
>
> This should work:
>
> int ret;
> ret = FFDIFFSIGN(de->GroupNumber, Dd->GroupNumber);
> if (!ret)
>      ret = FFDIFFSIGN(de->ElementNumber, Dd->ElementNumber);
> return ret;
>
>> +    if (!de->GroupNumber || !de->ElementNumber)
>> +        return -2;
> This is eventually returned by dicom_read_packet(). What is this "-2"
> interpreted as by the caller of this .read_packet callback?
>
>> +}
>> \ No newline at end of file
> Please add a newline to the last line of this file. ;-)

Ok, i will apply all the improvements.

Thanks for the review

Shivam Goyal
diff mbox

Patch

From cd7ebe834278b29b0429b3f5b377154490ee159f Mon Sep 17 00:00:00 2001
From: Shivam Goyal <1998.goyal.shivam@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 +-
 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;
+    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;
+    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(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);
+    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);
+    return key;
+}
+
+static char *get_val_str(DataElement *de) {
+    char *val;
+
+    switch(de->VR) {
+        case AT:
+        case OB:
+        case OD:
+        case OF:
+        case OL:
+        case OV:
+        case OW:
+            val = av_strdup("[Binary data]");
+            break;
+        case UN:
+        case SQ:
+            val = av_strdup("[Sequence of items]");
+            break;
+        case FL:
+            val = (char *) av_malloc(15);
+            sprintf(val, "%.3f", ((float *)de->bytes)[0]);
+            break;
+        case FD:
+            val = (char *) av_malloc(20);
+            sprintf(val, "%.3lf", ((double *)de->bytes)[0]);
+            break;
+        case SL:
+            val = (char *) av_malloc(15);
+            sprintf(val, "%d", ((int32_t *)de->bytes)[0]);
+            break;
+        case SS:
+            val = (char *) av_malloc(10);
+            sprintf(val, "%d", ((int16_t *)de->bytes)[0]);
+            break;
+        case SV:
+            val = (char *) av_malloc(25);
+            sprintf(val, "%ld", ((int64_t *)de->bytes)[0]);
+            break;
+        case UL:
+            val = (char *) av_malloc(15);
+            sprintf(val, "%u", ((uint32_t *)de->bytes)[0]);
+            break;
+        case US:
+            val = (char *) av_malloc(10);
+            sprintf(val, "%u", ((uint16_t *)de->bytes)[0]);
+            break;
+        case UV:
+            val = (char *) av_malloc(25);
+            sprintf(val, "%lu", ((uint64_t *)de->bytes)[0]);
+            break;
+        default:
+            val = (char *) av_malloc(de->VL + 1);
+            memcpy(val, de->bytes, de->VL);
+            val[de->VL] = 0;
+            break;
+    }
+    return val;
+}
+
+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;
+            st->codecpar->height = dicom->height;
+            break;
+        case 0x0011: // columns
+            dicom->width = *(uint16_t *)bytes;
+            st->codecpar->width = dicom->width;
+            break;
+        case 0x0100: // bits allocated
+            st->codecpar->bits_per_raw_sample = *(uint16_t *)bytes;
+            break;
+        case 0x0101: // bits stored
+            st->codecpar->bits_per_coded_sample = *(uint16_t *)bytes;
+            break;
+        case 0x0008: // number of frames
+            dicom->nb_frames = conv_IS(de);
+            st->nb_frames = dicom->nb_frames;
+            st->duration = dicom->delay * dicom->nb_frames;
+            break;
+        case 0x1050: // window center/level
+            if (dicom->level == -1) {
+                st->codecpar->level = conv_IS(de);
+                dicom->level = st->codecpar->level;
+            }
+            break;
+        case 0x1051: // window width/window
+            if (dicom->window == -1) {
+                st->codecpar->profile = conv_IS(de);
+                dicom->window = st->codecpar->profile;
+            }
+            break;
+        case 0x0120: // pixel padding
+            dicom->pixpad = *(int16_t *)bytes;
+            break;
+        case 0x0004: // photometric interpret
+            cbytes = av_malloc(len + 1);
+            memcpy(cbytes, bytes, len);
+            cbytes[len] = 0;
+            dicom->interpret = find_PI(cbytes);
+            av_free(cbytes);
+            break;
+        case 0x0103: // pix rep
+            dicom->pixrep = *(uint16_t *)bytes;
+            break;
+        case 0x1053: // rescale slope
+            dicom->slope = conv_IS(de);
+            break;
+        case 0x1052: // rescale intercept
+            dicom->intcpt = conv_IS(de);
+            break;
+    }
+    return 0;
+}
+
+static int set_multiframe_data(AVFormatContext *s, AVStream* st, DataElement *de)
+{
+    DICOMContext *dicom = s->priv_data;
+
+    if (de->GroupNumber != MF_GR_NB)
+        return 0;
+
+    switch (de->ElementNumber) {
+        case 0x1063: // frame time
+            dicom->delay = conv_DS(de);
+            dicom->duration = dicom->delay * dicom->nb_frames;
+            break;
+    }
+    return 0;
+}
+
+
+static int read_de_metainfo(AVFormatContext *s, DataElement *de)
+{
+    DICOMContext *dicom = s->priv_data;
+    ValueRepresentation vr;
+    int ret;
+    int bytes_read = 0;
+
+    ret = avio_rl16(s->pb);
+    if (ret < 0)
+        return ret;
+    de->GroupNumber = ret;
+    de->ElementNumber =  avio_rl16(s->pb);
+    if (dicom->inseq || (dicom->transfer_syntax == IMPLICIT_VR && !dicom->inheader)) {
+        de->VL = avio_rl32(s->pb);
+        bytes_read += 8;
+        return bytes_read;
+    }
+    vr = avio_rb16(s->pb); // Stored in Big Endian in dicom.h
+    de->VR = vr;
+    bytes_read += 6;
+    switch (vr) {
+        case OB:
+        case OD:
+        case OF:
+        case OL:
+        case OV:
+        case OW:
+        case SQ:
+        case SV:
+        case UC:
+        case UR:
+        case UT:
+        case UN:
+        case UV:
+            avio_skip(s->pb, 2); // Padding always 0x0000
+            de->VL = avio_rl32(s->pb);
+            bytes_read += 6;
+            break;
+        default:
+            de->VL = avio_rl16(s->pb);
+            bytes_read += 2;
+            break;
+    }
+    if (de->VL != UNDEFINED_VL && de->VL % 2)
+        av_log(s, AV_LOG_WARNING,"Data Element Value length: %ld can't be odd\n", de->VL);
+    if (de->VL < 0)
+        return AVERROR_INVALIDDATA;
+    return bytes_read;
+}
+
+static int read_de(AVFormatContext *s, DataElement *de)
+{
+    int ret;
+    uint32_t len = de->VL;
+    de->bytes = av_malloc(len);
+    ret = avio_read(s->pb, de->bytes, len);
+    return ret;
+}
+
+static int read_implicit_seq_item(AVFormatContext *s, DataElement *de)
+{
+    int ret, f = -2, i = 0;
+
+    de->bytes = av_malloc(MAX_UNDEF_LENGTH);
+    for(; i < MAX_UNDEF_LENGTH; ++i) {
+        ret = avio_rl16(s->pb);
+        if (ret < 0)
+            return ret;
+
+        if (ret == SEQ_GR_NB)
+            f = i;
+        if (ret == SEQ_ITEM_DEL_EL_NB && f == i - 1) {
+            avio_skip(s->pb, 4);
+            break;
+        }
+        bytestream_put_le16((uint8_t **)&de->bytes, ret);
+    }
+    de->VL = (i - 1) * 2;
+    return 0;
+}
+
+static int read_seq(AVFormatContext *s, DataElement *de) {
+    int i = 0, ret;
+    DICOMContext *dicom = s->priv_data;
+    DataElement *seq_data = (DataElement *)av_malloc(sizeof(DataElement) * MAX_SEQ_LENGTH);
+    de->bytes = seq_data;
+    dicom->inseq = 1;
+    for (;i < MAX_SEQ_LENGTH; ++i) {
+        seq_data[i].bytes = NULL;
+        seq_data[i].desc = NULL;
+        seq_data[i].is_found = 0;
+        read_de_metainfo(s, seq_data + i);
+        if (seq_data[i].GroupNumber == SEQ_GR_NB
+            && seq_data[i].ElementNumber == SEQ_DEL_EL_NB){
+            ret = i;
+            goto exit;
+        }
+        if (seq_data[i].VL == UNDEFINED_VL)
+            ret = read_implicit_seq_item(s, seq_data + i);
+        else
+            ret = read_de(s, seq_data + i);
+        if (ret < 0)
+            goto exit;
+    }
+
+exit:
+    dicom->inseq = 0;
+    return ret;
+}
+
+static int read_de_valuefield(AVFormatContext *s, DataElement *de) {
+    if (de->VL == UNDEFINED_VL)
+        return read_seq(s, de);
+    else return read_de(s, de);
+}
+
+static int dicom_read_header(AVFormatContext *s)
+{
+    AVIOContext  *pb = s->pb;
+    AVDictionary **m = &s->metadata;
+    DICOMContext *dicom = s->priv_data;
+    DataElement *de;
+    char *key, *value;
+    uint32_t header_size, bytes_read = 0;
+    int ret;
+
+    ret = avio_skip(pb, DICOM_PREAMBLE_SIZE + DICOM_PREFIX_SIZE);
+    if (ret < 0)
+        return ret;
+    dicom->inheader = 1;
+    de = alloc_de();
+    ret = read_de_metainfo(s, de);
+    ret = read_de_valuefield(s, de);
+    if (ret < 0) {
+        free_de(de);
+        return ret;
+    }
+    if (de->GroupNumber != 0x02 || de->ElementNumber != 0x00) {
+        av_log(s, AV_LOG_WARNING,"First data element is not \'File MetaInfo Group Length\', may fail to demux");
+        header_size = 200; // Fallback to default length
+    } else
+        header_size = *(uint32_t *)de->bytes;
+
+    free_de(de);
+    while (bytes_read < header_size) {
+        de = alloc_de();
+        ret = read_de_metainfo(s, de);
+        if (ret < 0) {
+            free_de(de);
+            return ret;
+        }
+        bytes_read += ret;
+        dicom_dict_find_elem_info(de);
+        key = get_key_str(de);
+        ret = read_de_valuefield(s, de);
+        if (ret < 0) {
+            free_de(de);
+            return ret;
+        }
+        bytes_read += ret;
+        value = get_val_str(de);
+        if (de->GroupNumber == TS_GR_NB && de->ElementNumber == TS_EL_NB)
+            dicom->transfer_syntax = get_transfer_sytax(value);
+
+        av_dict_set(m, key, value, AV_DICT_DONT_STRDUP_KEY | AV_DICT_DONT_STRDUP_VAL);
+        free_de(de);
+    }
+    set_context_defaults(dicom);
+    s->ctx_flags |= AVFMTCTX_NOHEADER;
+    s->start_time = 0;
+    return 0;
+}
+
+static int dicom_read_packet(AVFormatContext *s, AVPacket *pkt)
+{
+    DICOMContext *dicom = s->priv_data;
+    int metadata = dicom->metadata, ret;
+    AVDictionary **st_md;
+    char *key, *value;
+    AVStream  *st;
+    DataElement *de;
+
+    if (s->nb_streams < 1) {
+        st = avformat_new_stream(s, NULL);
+        if (!st)
+            return AVERROR(ENOMEM);
+        st->codecpar->codec_id   = AV_CODEC_ID_DICOM;
+        st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
+        st->start_time = 0;
+        avpriv_set_pts_info(st, 64, 1, 1000);
+        if (dicom->window != -1)
+            st->codecpar->profile = dicom->window;
+        if (dicom->level != -1)
+            st->codecpar->level = dicom->level;
+    } else
+        st = s->streams[0];
+
+    st_md = &st->metadata;
+    dicom->inheader = 0;
+    if (dicom->frame_nb > 1 && dicom->frame_nb <= dicom->nb_frames)
+        goto inside_pix_data;
+
+    for (;;) {
+        ret = avio_feof(s->pb);
+        if (ret)
+            return AVERROR_EOF;
+
+        de = alloc_de();
+        ret = read_de_metainfo(s,de);
+        if (ret < 0)
+            goto exit;
+
+        if (de->GroupNumber == PIXEL_GR_NB && de->ElementNumber == PIXELDATA_EL_NB) {
+            dicom->frame_size = de->VL / dicom->nb_frames;
+            set_dec_extradata(s, st);
+        inside_pix_data:
+            if (av_new_packet(pkt, dicom->frame_size) < 0)
+                return AVERROR(ENOMEM);
+            pkt->pos = avio_tell(s->pb);
+            pkt->stream_index = 0;
+            pkt->size = dicom->frame_size;
+            pkt->duration = dicom->delay;
+            ret = avio_read(s->pb, pkt->data, dicom->frame_size);
+            if (ret < 0) {
+                av_packet_unref(pkt);
+                goto exit;
+            }
+            dicom->frame_nb ++;
+            return ret;
+        } else if (de->GroupNumber == IMAGE_GR_NB || de->GroupNumber == MF_GR_NB) {
+            ret = read_de_valuefield(s, de);
+            if (ret < 0)
+                goto exit;
+            set_imagegroup_data(s, st, de);
+            set_multiframe_data(s, st, de);
+        } else {
+            if (metadata || de->VL == UNDEFINED_VL)
+                ret = read_de_valuefield(s, de);
+            else
+                ret = avio_skip(s->pb, de->VL); // skip other elements
+            if (ret < 0)
+                goto exit;
+        }
+        if (metadata) {
+            ret = dicom_dict_find_elem_info(de);
+            if (ret < 0)
+                goto end_de;
+            key = get_key_str(de);
+            value = get_val_str(de);
+            av_dict_set(st_md, key, value, AV_DICT_DONT_STRDUP_KEY | AV_DICT_DONT_STRDUP_VAL);
+        }
+    end_de:
+        free_de(de);
+    }
+exit:
+    free_de(de);
+    if (ret < 0)
+        return ret;
+    return AVERROR_EOF;
+}
+
+static const AVOption options[] = {
+    { "window", "override default window found in file", offsetof(DICOMContext, window), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 99999, AV_OPT_FLAG_DECODING_PARAM },
+    { "level", "override default level found in file", offsetof(DICOMContext, level), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 99999, AV_OPT_FLAG_DECODING_PARAM },
+    { "metadata", "skip or decode metadata", offsetof(DICOMContext, metadata)  , AV_OPT_TYPE_BOOL,{.i64 = 0}, 0, 1, AV_OPT_FLAG_DECODING_PARAM },
+    { NULL },
+};
+
+static const AVClass dicom_class = {
+    .class_name = "dicomdec",
+    .item_name  = av_default_item_name,
+    .option     = options,
+    .version    = LIBAVUTIL_VERSION_INT,
+};
+
+AVInputFormat ff_dicom_demuxer = {
+    .name           = "dicom",
+    .long_name      = NULL_IF_CONFIG_SMALL("DICOM (Digital Imaging and Communications in Medicine)"),
+    .priv_data_size = sizeof(DICOMContext),
+    .read_probe     = dicom_probe,
+    .read_header    = dicom_read_header,
+    .read_packet    = dicom_read_packet,
+    .extensions     = "dcm",
+    .priv_class     = &dicom_class,
+};
diff --git a/libavformat/dicomdict.c b/libavformat/dicomdict.c
new file mode 100644
index 0000000000..f1ea8d128d
--- /dev/null
+++ b/libavformat/dicomdict.c
@@ -0,0 +1,716 @@ 
+/*
+ * DICOM Dictionary
+ *
+ * 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/mem.h"
+#include "dicom.h"
+
+typedef struct DICOMDictionary {
+    uint16_t GroupNumber;
+    uint16_t ElementNumber;
+    ValueRepresentation vr;
+    const char *desc;
+} DICOMDictionary;
+
+/**
+ * dicom_dictionary[] to store dicom tags value rep
+ * and desc.
+ *
+ * On adding new data elements to dicom_dictionary[]
+ * always keep them in ascending order of GroupNumber
+ * and ElementNumber
+*/
+DICOMDictionary dicom_dictionary[] = {
+    {0x0002, 0x0000, UL, "File Meta Elements Group Len"},
+    {0x0002, 0x0001, OB, "File Meta Information Version"},
+    {0x0002, 0x0002, UI, "Media Storage SOP Class UID"},
+    {0x0002, 0x0003, UI, "Media Storage SOP Inst UID"},
+    {0x0002, 0x0010, UI, "Transfer Syntax UID"},
+    {0x0002, 0x0012, UI, "Implementation Class UID"},
+    {0x0002, 0x0013, SH, "Implementation Version Name"},
+    {0x0002, 0x0016, AE, "Source Application Entity Title"},
+    {0x0002, 0x0017, AE, "Sending Application Entity Title"},
+    {0x0002, 0x0018, AE, "Receiving Application Entity Title"},
+    {0x0002, 0x0100, UI, "Private Information Creator UID"},
+    {0x0002, 0x0102, OB, "Private Information"},
+    {0x0004, 0x1130, CS, "File-set ID"},
+    {0x0004, 0x1141, CS, "File-set Descriptor File ID"},
+    {0x0004, 0x1142, CS, "Specific Character Set of File-set Descriptor File"},
+    {0x0004, 0x1200, UL, "Offset of the First Directory Record of the Root Directory Entity"},
+    {0x0004, 0x1202, UL, "Offset of the Last Directory Record of the Root Directory Entity"},
+    {0x0004, 0x1212, US, "File-set Consistency Flag"},
+    {0x0004, 0x1220, SQ, "Directory Record Sequence"},
+    {0x0004, 0x1400, UL, "Offset of the Next Directory Record"},
+    {0x0004, 0x1410, US, "Record In-use Flag"},
+    {0x0004, 0x1420, UL, "Offset of Referenced Lower-Level Directory Entity"},
+    {0x0004, 0x1430, CS, "Directory Record Type"},
+    {0x0004, 0x1432, UI, "Private Record UID"},
+    {0x0004, 0x1500, CS, "Referenced File ID"},
+    {0x0004, 0x1504, UL, "MRDR Directory Record Offset"},
+    {0x0004, 0x1510, UI, "Referenced SOP Class UID in File"},
+    {0x0004, 0x1511, UI, "Referenced SOP Instance UID in File"},
+    {0x0004, 0x1512, UI, "Referenced Transfer Syntax UID in File"},
+    {0x0004, 0x151A, UI, "Referenced Related General SOP Class UID in File"},
+    {0x0004, 0x1600, UL, "Number of References"},
+    {0x0008, 0x0001, UL, "Length to End"},
+    {0x0008, 0x0005, CS, "Specific Character Set"},
+    {0x0008, 0x0006, SQ, "Language Code Sequence"},
+    {0x0008, 0x0008, CS, "Image Type"},
+    {0x0008, 0x0010, SH, "Recognition Code"},
+    {0x0008, 0x0012, DA, "Instance Creation Date"},
+    {0x0008, 0x0013, TM, "Instance Creation Time"},
+    {0x0008, 0x0014, UI, "Instance Creator UID"},
+    {0x0008, 0x0015, DT, "Instance Create UID"},
+    {0x0008, 0x0016, UI, "SOP Class UID"},
+    {0x0008, 0x0018, UI, "SOP Instance UID"},
+    {0x0008, 0x001A, UI, "Related General SOP Class UID"},
+    {0x0008, 0x001B, UI, "Original Specialized SOP Class UID"},
+    {0x0008, 0x0020, DA, "Study Date"},
+    {0x0008, 0x0021, DA, "Series Date"},
+    {0x0008, 0x0022, DA, "Acquisition Date"},
+    {0x0008, 0x0023, DA, "Content Date"},
+    {0x0008, 0x0024, DA, "Overlay Date"},
+    {0x0008, 0x0025, DA, "Curve Date"},
+    {0x0008, 0x002A, DT, "Acquisition DateTime"},
+    {0x0008, 0x0030, TM, "Study Time"},
+    {0x0008, 0x0031, TM, "Series Time"},
+    {0x0008, 0x0032, TM, "Acquisition Time"},
+    {0x0008, 0x0033, TM, "Content Time"},
+    {0x0008, 0x0034, TM, "Overlay Time"},
+    {0x0008, 0x0035, TM, "Curve Time"},
+    {0x0008, 0x0040, US, "Data Set Type"},
+    {0x0008, 0x0041, LO, "Data Set Subtype"},
+    {0x0008, 0x0042, CS, "Nuclear Medicine Series Type"},
+    {0x0008, 0x0050, SH, "Accession Number"},
+    {0x0008, 0x0051, SQ, "Issuer of Accession Number Sequence"},
+    {0x0008, 0x0052, CS, "Query/Retrieve Level"},
+    {0x0008, 0x0053, CS, "Query/Retrieve View"},
+    {0x0008, 0x0054, AE, "Retrieve AE Title"},
+    {0x0008, 0x0055, AE, "Station AE Title"},
+    {0x0008, 0x0056, CS, "Instance Availability"},
+    {0x0008, 0x0058, UI, "Failed SOP Instance UID List"},
+    {0x0008, 0x0060, CS, "Modality"},
+    {0x0008, 0x0061, CS, "Modalities in Study"},
+    {0x0008, 0x0062, UI, "SOP Classes in Study"},
+    {0x0008, 0x0064, CS, "Conversion Type"},
+    {0x0008, 0x0068, CS, "Presentation Intent Type"},
+    {0x0008, 0x0070, LO, "Manufacturer"},
+    {0x0008, 0x0080, LO, "Institution Name"},
+    {0x0008, 0x0081, ST, "Institution Address"},
+    {0x0008, 0x0082, SQ, "Institution Code Sequence"},
+    {0x0008, 0x0090, PN, "Referring Physician's Name"},
+    {0x0008, 0x0092, ST, "Referring Physician's Address"},
+    {0x0008, 0x0094, SH, "Referring Physician's Telephone Numbers"},
+    {0x0008, 0x0096, SQ, "Referring Physician Identification Sequence"},
+    {0x0008, 0x009C, PN, "Consulting Physician's Name"},
+    {0x0008, 0x009D, SQ, "Consulting Physician Identification Sequence"},
+    {0x0008, 0x0100, SH, "Code Value"},
+    {0x0008, 0x0101, LO, "Extended Code Value"},
+    {0x0008, 0x0102, SH, "Coding Scheme Designator"},
+    {0x0008, 0x0104, LO, "Code Meaning"},
+    {0x0008, 0x0105, CS, "Mapping Resource"},
+    {0x0008, 0x0106, DT, "Context Group Version"},
+    {0x0008, 0x0107, DT, "Context Group Local Version"},
+    {0x0008, 0x0108, LT, "Extended Code Meaning"},
+    {0x0008, 0x010C, UI, "Coding Scheme UID"},
+    {0x0008, 0x010D, UI, "Context Group Extension Creator UID"},
+    {0x0008, 0x010F, CS, "Context Identifier"},
+    {0x0008, 0x0110, SQ, "Coding Scheme Identification Sequence"},
+    {0x0008, 0x0112, LO, "Coding Scheme Registry"},
+    {0x0008, 0x0114, ST, "Coding Scheme External ID"},
+    {0x0008, 0x0115, ST, "Coding Scheme Name"},
+    {0x0008, 0x0116, ST, "Coding Scheme Responsible Organization"},
+    {0x0008, 0x0117, UI, "Context UID"},
+    {0x0008, 0x0118, UI, "Mapping Resource UID"},
+    {0x0008, 0x0119, UC, "Long Code Value"},
+    {0x0008, 0x0120, UR, "URN Code Value"},
+    {0x0008, 0x0121, SQ, "Equivalent Code Sequence"},
+    {0x0008, 0x0122, LO, "Mapping Resource Name"},
+    {0x0008, 0x0123, SQ, "Context Group Identification Sequence"},
+    {0x0008, 0x0124, SQ, "Mapping Resource Identification Sequence"},
+    {0x0008, 0x0201, SH, "Timezone Offset From UTC"},
+    {0x0008, 0x0300, SQ, "Private Data Element Characteristics Sequence"},
+    {0x0008, 0x0301, US, "Private Group Reference"},
+    {0x0008, 0x0302, LO, "Private Creator Reference"},
+    {0x0008, 0x0303, CS, "Block Identifying Information Status"},
+    {0x0008, 0x0304, US, "Nonidentifying PrivateElements"},
+    {0x0008, 0x0305, SQ, "Deidentification ActionSequence"},
+    {0x0008, 0x0306, US, "Identifying PrivateElements"},
+    {0x0008, 0x0307, CS, "Deidentification Action"},
+    {0x0008, 0x1000, AE, "Network ID"},
+    {0x0008, 0x1010, SH, "Station Name"},
+    {0x0008, 0x1030, LO, "Study Description"},
+    {0x0008, 0x1032, SQ, "Procedure Code Sequence"},
+    {0x0008, 0x103E, LO, "Series Description"},
+    {0x0008, 0x103F, SQ, "Series Description CodeSequence"},
+    {0x0008, 0x1040, LO, "Institutional Department Name"},
+    {0x0008, 0x1048, PN, "Physician(s) of Record"},
+    {0x0008, 0x1049, SQ, "Physician(s) of Record Identification Sequence"},
+    {0x0008, 0x1050, PN, "Attending Physician's Name"},
+    {0x0008, 0x1052, SQ, "Performing Physician Identification Sequence"},
+    {0x0008, 0x1060, PN, "Name of Physician(s) Reading Study"},
+    {0x0008, 0x1062, SQ, "Physician(s) ReadingStudy Identification Sequenc"},
+    {0x0008, 0x1070, PN, "Operator's Name"},
+    {0x0008, 0x1072, SQ, "Operator Identification Sequence"},
+    {0x0008, 0x1080, LO, "Admitting Diagnosis Description"},
+    {0x0008, 0x1084, SQ, "Admitting Diagnosis Code Sequence"},
+    {0x0008, 0x1090, LO, "Manufacturer's Model Name"},
+    {0x0008, 0x1100, SQ, "Referenced Results Sequence"},
+    {0x0008, 0x1110, SQ, "Referenced Study Sequence"},
+    {0x0008, 0x1111, SQ, "Referenced Study Component Sequence"},
+    {0x0008, 0x1115, SQ, "Referenced Series Sequence"},
+    {0x0008, 0x1120, SQ, "Referenced Patient Sequence"},
+    {0x0008, 0x1125, SQ, "Referenced Visit Sequence"},
+    {0x0008, 0x1130, SQ, "Referenced Overlay Sequence"},
+    {0x0008, 0x1134, SQ, "Referenced Stereometric Instance Sequence"},
+    {0x0008, 0x113A, SQ, "Referenced Waveform Sequence"},
+    {0x0008, 0x1140, SQ, "Referenced Image Sequence"},
+    {0x0008, 0x1145, SQ, "Referenced Curve Sequence"},
+    {0x0008, 0x114A, SQ, "Referenced InstanceSequence"},
+    {0x0008, 0x114B, SQ, "Referenced Real World Value Mapping InstanceSequence"},
+    {0x0008, 0x1150, UI, "Referenced SOP Class UID"},
+    {0x0008, 0x1155, UI, "Referenced SOP Instance UID"},
+    {0x0008, 0x115A, UI, "SOP Classes Supported"},
+    {0x0008, 0x1160, IS, "Referenced Frame Number"},
+    {0x0008, 0x1161, UL, "Simple Frame List"},
+    {0x0008, 0x1162, UL, "Calculated Frame List"},
+    {0x0008, 0x1163, FD, "Time Range"},
+    {0x0008, 0x1164, SQ, "Frame Extraction Sequence"},
+    {0x0008, 0x1167, UI, "Multi-frame Source SOP Instance UID"},
+    {0x0008, 0x1190, UR, "Retrieve URL"},
+    {0x0008, 0x1195, UI, "Transaction UID"},
+    {0x0008, 0x1196, US, "Warning Reason"},
+    {0x0008, 0x1197, US, "Failure Reason"},
+    {0x0008, 0x1198, SQ, "Failed SOP Sequence"},
+    {0x0008, 0x1199, SQ, "Referenced SOP Sequence"},
+    {0x0008, 0x119A, SQ, "Other Failures Sequence"},
+    {0x0008, 0x1200, SQ, "Studies Containing OtherReferenced InstancesSequence"},
+    {0x0008, 0x1250, SQ, "Related Series Sequence"},
+    {0x0008, 0x2110, CS, "Lossy Image Compression(Retired)"},
+    {0x0008, 0x2111, ST, "Derivation Description"},
+    {0x0008, 0x2112, SQ, "Source Image Sequence"},
+    {0x0008, 0x2120, SH, "Stage Name"},
+    {0x0008, 0x2122, IS, "Stage Number"},
+    {0x0008, 0x2124, IS, "Number of Stages"},
+    {0x0008, 0x2127, SH, "View Name"},
+    {0x0008, 0x2128, IS, "View Number"},
+    {0x0008, 0x2129, IS, "Number of Event Timers"},
+    {0x0008, 0x212A, IS, "Number of Views in Stage"},
+    {0x0008, 0x2130, DS, "Event Elapsed Time(s)"},
+    {0x0008, 0x2132, LO, "Event Timer Name(s)"},
+    {0x0008, 0x2133, SQ, "Event Timer Sequence"},
+    {0x0008, 0x2134, FD, "Event Time Offset"},
+    {0x0008, 0x2135, SQ, "Event Code Sequence"},
+    {0x0008, 0x2142, IS, "Start Trim"},
+    {0x0008, 0x2143, IS, "Stop Trim"},
+    {0x0008, 0x2144, IS, "Recommended Display Frame Rate"},
+    {0x0008, 0x2200, CS, "Transducer Position"},
+    {0x0008, 0x2204, CS, "Transducer Orientation"},
+    {0x0008, 0x2208, CS, "Anatomic Structure"},
+    {0x0008, 0x2218, SQ, "Anatomic RegionSequence"},
+    {0x0008, 0x2220, SQ, "Anatomic Region ModifierSequence"},
+    {0x0008, 0x2228, SQ, "Primary Anatomic Structure Sequence"},
+    {0x0008, 0x2229, SQ, "Anatomic Structure, Spaceor Region Sequence"},
+    {0x0008, 0x2230, SQ, "Primary Anatomic Structure ModifierSequence"},
+    {0x0008, 0x2240, SQ, "Transducer Position Sequence"},
+    {0x0008, 0x2242, SQ, "Transducer Position Modifier Sequence"},
+    {0x0008, 0x2244, SQ, "Transducer Orientation Sequence"},
+    {0x0008, 0x2246, SQ, "Transducer Orientation Modifier Sequence"},
+    {0x0008, 0x2251, SQ, "Anatomic Structure SpaceOr Region Code Sequence(Trial)"},
+    {0x0008, 0x2253, SQ, "Anatomic Portal Of Entrance Code Sequence(Trial)"},
+    {0x0008, 0x2255, SQ, "Anatomic ApproachDirection Code Sequence(Trial)"},
+    {0x0008, 0x2256, ST, "Anatomic Perspective Description (Trial)"},
+    {0x0008, 0x2257, SQ, "Anatomic Perspective Code Sequence (Trial)"},
+    {0x0008, 0x2258, ST, "Anatomic Location Of Examining InstrumentDescription (Trial)"},
+    {0x0008, 0x2259, SQ, "Anatomic Location Of Examining InstrumentCode Sequence (Trial)"},
+    {0x0008, 0x225A, SQ, "Anatomic Structure SpaceOr Region Modifier CodeSequence (Trial)"},
+    {0x0008, 0x225C, SQ, "On Axis Background Anatomic Structure CodeSequence (Trial)"},
+    {0x0008, 0x3001, SQ, "Alternate Representation Sequence"},
+    {0x0008, 0x3010, UI, "Irradiation Event UID"},
+    {0x0008, 0x3011, SQ, "Source Irradiation Event Sequence"},
+    {0x0008, 0x2012, UI, "Radiopharmaceutical Administration Event UID"},
+    {0x0008, 0x4000, LT, "Identifying Comments"},
+    {0x0008, 0x9007, CS, "Frame Type"},
+    {0x0008, 0x9092, SQ, "Referenced ImageEvidence Sequence"},
+    {0x0008, 0x9121, SQ, "Referenced Raw DataSequence"},
+    {0x0008, 0x9123, UI, "Creator-Version UID"},
+    {0x0008, 0x9124, SQ, "Derivation ImageSequence"},
+    {0x0008, 0x9154, SQ, "Source Image EvidenceSequence"},
+    {0x0008, 0x9205, CS, "Pixel Presentation"},
+    {0x0008, 0x9206, CS, "Volumetric Properties"},
+    {0x0008, 0x9207, CS, "Volume Based Calculation Technique"},
+    {0x0008, 0x9208, CS, "Complex Image Component"},
+    {0x0008, 0x9209, CS, "Acquisition Contrast"},
+    {0x0008, 0x9215, SQ, "Derivation Code Sequence"},
+    {0x0008, 0x9237, SQ, "Referenced Presentation State Sequence"},
+    {0x0008, 0x9410, SQ, "Referenced Other Plane Sequence"},
+    {0x0008, 0x9458, SQ, "Frame Display Sequence"},
+    {0x0008, 0x9459, FL, "Recommended DisplayFrame Rate in Float"},
+    {0x0008, 0x9460, CS, "Skip Frame Range Flag"},
+
+    // Group Number 0x0010, Patient's Info (incomplete / only main elems)
+    {0x0010, 0x0010, PN, "Patient's Name"},
+    {0x0010, 0x0020, LO, "Patient ID"},
+    {0x0010, 0x0021, LO, "Issuer of Patient ID"},
+    {0x0010, 0x0022, CS, "Type of Patient ID"},
+    {0x0010, 0x0024, SQ, "Issuer of Patient ID Qualifiers Sequence"},
+    {0x0010, 0x0026, SQ, "Source Patient Group Identification Sequence"},
+    {0x0010, 0x0027, SQ, "Group of Patients Identification Sequence"},
+    {0x0010, 0x0028, US, "Subject Relative Position in Image"},
+    {0x0010, 0x0038, DA, "Patient's Birth Date"},
+    {0x0010, 0x0032, TM, "Patient's Birth Time"},
+    {0x0010, 0x0033, LO, "Patient's Birth Date in Alternative Calendar"},
+    {0x0010, 0x0034, LO, "Patient's Death Date in Alternative Calendar"},
+    {0x0010, 0x0035, CS, "Patient's Alternative Calendar"},
+    {0x0010, 0x0040, CS, "Patient's Sex"},
+    {0x0010, 0x0050, SQ, "Patient's Insurance PlanCode Sequence"},
+
+    // Group Number 0x0028, Image Group Number
+    {0x0028, 0x0002, US, "Samples per Pixel"},
+    {0x0028, 0x0003, US, "Samples per Pixel Used"},
+    {0x0028, 0x0004, CS, "Photometric Interpretation"},
+    {0x0028, 0x0005, US, "Image Dimensions"},
+    {0x0028, 0x0006, US, "Planar Configuration"},
+    {0x0028, 0x0008, IS, "Number of Frames"},
+    {0x0028, 0x0009, AT, "Frame Increment Pointer"},
+    {0x0028, 0x000A, AT, "Frame Dimension Pointer"},
+    {0x0028, 0x0010, US, "Rows"},
+    {0x0028, 0x0011, US, "Columns"},
+    {0x0028, 0x0012, US, "Planes"},
+    {0x0028, 0x0014, US, "Ultrasound Color DataPresent"},
+    {0x0028, 0x0030, DS, "Pixel Spacing"},
+    {0x0028, 0x0031, DS, "Zoom Factor"},
+    {0x0028, 0x0032, DS, "Zoom Center"},
+    {0x0028, 0x0034, IS, "Pixel Aspect Ratio"},
+    {0x0028, 0x0040, CS, "Image Format"},
+    {0x0028, 0x0050, LO, "Manipulated Image"},
+    {0x0028, 0x0051, CS, "Corrected Image"},
+    {0x0028, 0x005F, LO, "Compression Recognition Code"},
+    {0x0028, 0x0060, CS, "Compression Code"},
+    {0x0028, 0x0061, SH, "Compression Originator"},
+    {0x0028, 0x0062, LO, "Compression Label"},
+    {0x0028, 0x0063, SH, "Compression Description"},
+    {0x0028, 0x0065, CS, "Compression Sequence"},
+    {0x0028, 0x0066, AT, "Compression Step Pointers"},
+    {0x0028, 0x0068, US, "Repeat Interval"},
+    {0x0028, 0x0069, US, "Bits Grouped"},
+    {0x0028, 0x0070, US, "Perimeter Table"},
+    {0x0028, 0x0071, US, "Perimeter Value"},
+    {0x0028, 0x0071, SS, "Perimeter Value"},
+    {0x0028, 0x0080, US, "Predictor Rows"},
+    {0x0028, 0x0081, US, "Predictor Columns"},
+    {0x0028, 0x0082, US, "Predictor Constants"},
+    {0x0028, 0x0090, CS, "Blocked Pixels"},
+    {0x0028, 0x0091, US, "Block Rows"},
+    {0x0028, 0x0092, US, "Block Columns"},
+    {0x0028, 0x0093, US, "Row Overlap"},
+    {0x0028, 0x0094, US, "Column Overlap"},
+    {0x0028, 0x0100, US, "Bits Allocated"},
+    {0x0028, 0x0101, US, "Bits Stored"},
+    {0x0028, 0x0102, US, "High Bit"},
+    {0x0028, 0x0103, US, "Pixel Representation"},
+    {0x0028, 0x0104, US, "Smallest Valid Pixel Value"},
+    {0x0028, 0x0104, SS, "Smallest Valid Pixel Value"},
+    {0x0028, 0x0105, US, "Largest Valid Pixel Value"},
+    {0x0028, 0x0105, SS, "Largest Valid Pixel Value"},
+    {0x0028, 0x0106, US, "Smallest Image Pixel Value"},
+    {0x0028, 0x0106, SS, "Smallest Image Pixel Value"},
+    {0x0028, 0x0107, US, "Largest Image Pixel Value"},
+    {0x0028, 0x0107, SS, "Largest Image Pixel Value"},
+    {0x0028, 0x0108, US, "Smallest Pixel Value in Series"},
+    {0x0028, 0x0108, SS, "Smallest Pixel Value in Series"},
+    {0x0028, 0x0109, US, "Largest Pixel Value in Series"},
+    {0x0028, 0x0109, SS, "Largest Pixel Value in Series"},
+    {0x0028, 0x0120, US, "Pixel Padding Value"},
+    {0x0028, 0x0120, SS, "Pixel Padding Value"},
+    {0x0028, 0x0121, US, "Pixel Padding Range Limit"},
+    {0x0028, 0x0121, SS, "Pixel Padding Range Limit"},
+    {0x0028, 0x0122, FL, "Float Pixel Padding Value"},
+    {0x0028, 0x0123, FD, "Double Float Pixel PaddingValue"},
+    {0x0028, 0x0124, FL, "Float Pixel Padding RangeLimit"},
+    {0x0028, 0x0125, FD, "Double Float Pixel PaddingRange Limit"},
+    {0x0028, 0x0200, US, "Image Location"},
+    {0x0028, 0x0300, CS, "Quality Control Image"},
+    {0x0028, 0x0301, CS, "Burned In Annotation"},
+    {0x0028, 0x0302, CS, "Recognizable VisualFeatures"},
+    {0x0028, 0x0303, CS, "Longitudinal Temporal Information Modified"},
+    {0x0028, 0x0304, UI, "Referenced Color Palette Instance UID"},
+    {0x0028, 0x0400, LO, "Transform Label"},
+    {0x0028, 0x0401, LO, "Transform Version Number"},
+    {0x0028, 0x0402, US, "Number of Transform Steps"},
+    {0x0028, 0x0403, LO, "Sequence of CompressedData"},
+    {0x0028, 0x0404, AT, "Details of Coefficients"},
+    {0x0028, 0x0410, US, "Rows For Nth OrderCoefficients"},
+    {0x0028, 0x0420, US, "Rows For Nth OrderCoefficients"},
+    {0x0028, 0x0430, US, "Rows For Nth OrderCoefficients"},
+    {0x0028, 0x0440, US, "Rows For Nth OrderCoefficients"},
+    {0x0028, 0x0450, US, "Rows For Nth OrderCoefficients"},
+    {0x0028, 0x0460, US, "Rows For Nth OrderCoefficients"},
+    {0x0028, 0x0470, US, "Rows For Nth OrderCoefficients"},
+    {0x0028, 0x0480, US, "Rows For Nth OrderCoefficients"},
+    {0x0028, 0x0490, US, "Rows For Nth OrderCoefficients"},
+    {0x0028, 0x04A0, US, "Rows For Nth OrderCoefficients"},
+    {0x0028, 0x04B0, US, "Rows For Nth OrderCoefficients"},
+    {0x0028, 0x04C0, US, "Rows For Nth OrderCoefficients"},
+    {0x0028, 0x04D0, US, "Rows For Nth OrderCoefficients"},
+    {0x0028, 0x04E0, US, "Rows For Nth OrderCoefficients"},
+    {0x0028, 0x04F0, US, "Rows For Nth OrderCoefficients"},
+    {0x0028, 0x0411, US, "Columns For Nth OrderCoefficients"},
+    {0x0028, 0x0421, US, "Columns For Nth OrderCoefficients"},
+    {0x0028, 0x0431, US, "Columns For Nth OrderCoefficients"},
+    {0x0028, 0x0441, US, "Columns For Nth OrderCoefficients"},
+    {0x0028, 0x0451, US, "Columns For Nth OrderCoefficients"},
+    {0x0028, 0x0461, US, "Columns For Nth OrderCoefficients"},
+    {0x0028, 0x0471, US, "Columns For Nth OrderCoefficients"},
+    {0x0028, 0x0481, US, "Columns For Nth OrderCoefficients"},
+    {0x0028, 0x0491, US, "Columns For Nth OrderCoefficients"},
+    {0x0028, 0x04A1, US, "Columns For Nth OrderCoefficients"},
+    {0x0028, 0x04B1, US, "Columns For Nth OrderCoefficients"},
+    {0x0028, 0x04C1, US, "Columns For Nth OrderCoefficients"},
+    {0x0028, 0x04D1, US, "Columns For Nth OrderCoefficients"},
+    {0x0028, 0x04E1, US, "Columns For Nth OrderCoefficients"},
+    {0x0028, 0x04F1, US, "Columns For Nth OrderCoefficients"},
+    {0x0028, 0x0412, US, "Coefficient Coding"},
+    {0x0028, 0x0422, US, "Coefficient Coding"},
+    {0x0028, 0x0432, US, "Coefficient Coding"},
+    {0x0028, 0x0442, US, "Coefficient Coding"},
+    {0x0028, 0x0452, US, "Coefficient Coding"},
+    {0x0028, 0x0462, US, "Coefficient Coding"},
+    {0x0028, 0x0472, US, "Coefficient Coding"},
+    {0x0028, 0x0482, US, "Coefficient Coding"},
+    {0x0028, 0x0492, US, "Coefficient Coding"},
+    {0x0028, 0x04A2, US, "Coefficient Coding"},
+    {0x0028, 0x04B2, US, "Coefficient Coding"},
+    {0x0028, 0x04C2, US, "Coefficient Coding"},
+    {0x0028, 0x04D2, US, "Coefficient Coding"},
+    {0x0028, 0x04E2, US, "Coefficient Coding"},
+    {0x0028, 0x04F2, US, "Coefficient Coding"},
+    {0x0028, 0x0413, AT, "Coefficient Coding Pointers"},
+    {0x0028, 0x0423, AT, "Coefficient Coding Pointers"},
+    {0x0028, 0x0433, AT, "Coefficient Coding Pointers"},
+    {0x0028, 0x0443, AT, "Coefficient Coding Pointers"},
+    {0x0028, 0x0453, AT, "Coefficient Coding Pointers"},
+    {0x0028, 0x0463, AT, "Coefficient Coding Pointers"},
+    {0x0028, 0x0473, AT, "Coefficient Coding Pointers"},
+    {0x0028, 0x0483, AT, "Coefficient Coding Pointers"},
+    {0x0028, 0x0493, AT, "Coefficient Coding Pointers"},
+    {0x0028, 0x04A3, AT, "Coefficient Coding Pointers"},
+    {0x0028, 0x04B3, AT, "Coefficient Coding Pointers"},
+    {0x0028, 0x04C3, AT, "Coefficient Coding Pointers"},
+    {0x0028, 0x04D3, AT, "Coefficient Coding Pointers"},
+    {0x0028, 0x04E3, AT, "Coefficient Coding Pointers"},
+    {0x0028, 0x04F3, AT, "Coefficient Coding Pointers"},
+    {0x0028, 0x0700, LO, "DCT Label"},
+    {0x0028, 0x0701, CS, "Data Block Description"},
+    {0x0028, 0x0702, AT, "Data Block"},
+    {0x0028, 0x0710, US, "Normalization FactorFormat"},
+    {0x0028, 0x0720, US, "Zonal Map Number Format"},
+    {0x0028, 0x0721, AT, "Zonal Map Location"},
+    {0x0028, 0x0722, US, "Zonal Map Format"},
+    {0x0028, 0x0730, US, "Adaptive Map Format"},
+    {0x0028, 0x0740, US, "Code Number Format"},
+    {0x0028, 0x0800, CS, "Code Label"},
+    {0x0028, 0x0810, CS, "Code Label"},
+    {0x0028, 0x0820, CS, "Code Label"},
+    {0x0028, 0x0830, CS, "Code Label"},
+    {0x0028, 0x0840, CS, "Code Label"},
+    {0x0028, 0x0850, CS, "Code Label"},
+    {0x0028, 0x0860, CS, "Code Label"},
+    {0x0028, 0x0870, CS, "Code Label"},
+    {0x0028, 0x0880, CS, "Code Label"},
+    {0x0028, 0x0890, CS, "Code Label"},
+    {0x0028, 0x08A0, CS, "Code Label"},
+    {0x0028, 0x08B0, CS, "Code Label"},
+    {0x0028, 0x08C0, CS, "Code Label"},
+    {0x0028, 0x08D0, CS, "Code Label"},
+    {0x0028, 0x08E0, CS, "Code Label"},
+    {0x0028, 0x08F0, CS, "Code Label"},
+    {0x0028, 0x0802, US, "Number of Tables"},
+    {0x0028, 0x0812, US, "Number of Tables"},
+    {0x0028, 0x0822, US, "Number of Tables"},
+    {0x0028, 0x0832, US, "Number of Tables"},
+    {0x0028, 0x0842, US, "Number of Tables"},
+    {0x0028, 0x0852, US, "Number of Tables"},
+    {0x0028, 0x0862, US, "Number of Tables"},
+    {0x0028, 0x0872, US, "Number of Tables"},
+    {0x0028, 0x0882, US, "Number of Tables"},
+    {0x0028, 0x0892, US, "Number of Tables"},
+    {0x0028, 0x08A2, US, "Number of Tables"},
+    {0x0028, 0x08B2, US, "Number of Tables"},
+    {0x0028, 0x08C2, US, "Number of Tables"},
+    {0x0028, 0x08D2, US, "Number of Tables"},
+    {0x0028, 0x08E2, US, "Number of Tables"},
+    {0x0028, 0x08F2, US, "Number of Tables"},
+    {0x0028, 0x0803, AT, "Code Table Location"},
+    {0x0028, 0x0813, AT, "Code Table Location"},
+    {0x0028, 0x0823, AT, "Code Table Location"},
+    {0x0028, 0x0833, AT, "Code Table Location"},
+    {0x0028, 0x0843, AT, "Code Table Location"},
+    {0x0028, 0x0853, AT, "Code Table Location"},
+    {0x0028, 0x0863, AT, "Code Table Location"},
+    {0x0028, 0x0873, AT, "Code Table Location"},
+    {0x0028, 0x0883, AT, "Code Table Location"},
+    {0x0028, 0x0893, AT, "Code Table Location"},
+    {0x0028, 0x08A3, AT, "Code Table Location"},
+    {0x0028, 0x08B3, AT, "Code Table Location"},
+    {0x0028, 0x08C3, AT, "Code Table Location"},
+    {0x0028, 0x08D3, AT, "Code Table Location"},
+    {0x0028, 0x08E3, AT, "Code Table Location"},
+    {0x0028, 0x08F3, AT, "Code Table Location"},
+    {0x0028, 0x0804, US, "Bits For Code Word"},
+    {0x0028, 0x0814, US, "Bits For Code Word"},
+    {0x0028, 0x0824, US, "Bits For Code Word"},
+    {0x0028, 0x0834, US, "Bits For Code Word"},
+    {0x0028, 0x0844, US, "Bits For Code Word"},
+    {0x0028, 0x0854, US, "Bits For Code Word"},
+    {0x0028, 0x0864, US, "Bits For Code Word"},
+    {0x0028, 0x0874, US, "Bits For Code Word"},
+    {0x0028, 0x0884, US, "Bits For Code Word"},
+    {0x0028, 0x0894, US, "Bits For Code Word"},
+    {0x0028, 0x08A4, US, "Bits For Code Word"},
+    {0x0028, 0x08B4, US, "Bits For Code Word"},
+    {0x0028, 0x08C4, US, "Bits For Code Word"},
+    {0x0028, 0x08D4, US, "Bits For Code Word"},
+    {0x0028, 0x08E4, US, "Bits For Code Word"},
+    {0x0028, 0x08F4, US, "Bits For Code Word"},
+    {0x0028, 0x0808, AT, "Image Data Location"},
+    {0x0028, 0x0818, AT, "Image Data Location"},
+    {0x0028, 0x0828, AT, "Image Data Location"},
+    {0x0028, 0x0838, AT, "Image Data Location"},
+    {0x0028, 0x0848, AT, "Image Data Location"},
+    {0x0028, 0x0858, AT, "Image Data Location"},
+    {0x0028, 0x0868, AT, "Image Data Location"},
+    {0x0028, 0x0878, AT, "Image Data Location"},
+    {0x0028, 0x0888, AT, "Image Data Location"},
+    {0x0028, 0x0898, AT, "Image Data Location"},
+    {0x0028, 0x08A8, AT, "Image Data Location"},
+    {0x0028, 0x08B8, AT, "Image Data Location"},
+    {0x0028, 0x08C8, AT, "Image Data Location"},
+    {0x0028, 0x08D8, AT, "Image Data Location"},
+    {0x0028, 0x08E8, AT, "Image Data Location"},
+    {0x0028, 0x08F8, AT, "Image Data Location"},
+    {0x0028, 0x0A02, CS, "Pixel Spacing Calibration Type"},
+    {0x0028, 0x0A04, LO, "Pixel Spacing Calibration Description"},
+    {0x0028, 0x1040, CS, "Pixel Intensity Relationship"},
+    {0x0028, 0x1041, SS, "Pixel Intensity Relationship Sign"},
+    {0x0028, 0x1050, DS, "Window Center"},
+    {0x0028, 0x1051, DS, "Window Width"},
+    {0x0028, 0x1052, DS, "Rescale Intercept"},
+    {0x0028, 0x1053, DS, "Rescale Slope"},
+    {0x0028, 0x1054, LO, "Rescale Type"},
+    {0x0028, 0x1055, LO, "Window Center & Width Explanation"},
+    {0x0028, 0x1056, CS, "VOI LUT Function"},
+    {0x0028, 0x1080, CS, "Gray Scale"},
+    {0x0028, 0x1090, CS, "Recommended Viewing Mode"},
+    {0x0028, 0x1100, US, "Gray Lookup Table Descriptor"},
+    {0x0028, 0x1100, SS, "Gray Lookup Table Descriptor"},
+    {0x0028, 0x1101, US, "Red Palette Color Lookup Table Descriptor"},
+    {0x0028, 0x1101, SS, "Red Palette Color Lookup Table Descriptor"},
+    {0x0028, 0x1102, US, "Green Palette Color Lookup Table Descriptor"},
+    {0x0028, 0x1102, SS, "Green Palette Color Lookup Table Descriptor"},
+    {0x0028, 0x1103, US, "Blue Palette Color Lookup Table Descriptor"},
+    {0x0028, 0x1103, SS, "Blue Palette Color Lookup Table Descriptor"},
+    {0x0028, 0x1104, US, "Alpha Palette Color Lookup Table Descriptor"},
+    {0x0028, 0x1111, US, "Large Red Palette ColorLookup Table Descriptor"},
+    {0x0028, 0x1111, SS, "Large Red Palette ColorLookup Table Descriptor"},
+    {0x0028, 0x1112, US, "Large Green Palette ColorLookup Table Descriptor"},
+    {0x0028, 0x1112, SS, "Large Green Palette ColorLookup Table Descriptor"},
+    {0x0028, 0x1113, US, "Large Blue Palette ColorLookup Table Descriptor"},
+    {0x0028, 0x1113, SS, "Large Blue Palette ColorLookup Table Descriptor"},
+    {0x0028, 0x1199, UI, "Palette Color Lookup TableUID"},
+    {0x0028, 0x1200, US, "Gray Lookup Table Data"},
+    {0x0028, 0x1200, SS, "Gray Lookup Table Data"},
+    {0x0028, 0x1200, OW, "Gray Lookup Table Data"},
+    {0x0028, 0x1201, OW, "Red Palette Color Lookup Table Data"},
+    {0x0028, 0x1202, OW, "Green Palette Color Lookup Table Data"},
+    {0x0028, 0x1203, OW, "Blue Palette Color Lookup Table Data"},
+    {0x0028, 0x1204, OW, "Alpha Palette Color LookupTable Data"},
+    {0x0028, 0x1211, OW, "Large Red Palette ColorLookup Table Data"},
+    {0x0028, 0x1212, OW, "Large Green Palette ColorLookup Table Data"},
+    {0x0028, 0x1213, OW, "Large Blue Palette ColorLookup Table Data"},
+    {0x0028, 0x1214, UI, "Large Palette Color LookupTable UID"},
+    {0x0028, 0x1221, OW, "Segmented Red PaletteColor Lookup Table Data"},
+    {0x0028, 0x1222, OW, "Segmented Green PaletteColor Lookup Table Data"},
+    {0x0028, 0x1223, OW, "Segmented Blue PaletteColor Lookup Table Data"},
+    {0x0028, 0x1224, OW, "Segmented Alpha PaletteColor Lookup Table Data"},
+    {0x0028, 0x1300, CS, "Breast Implant Present"},
+    {0x0028, 0x1350, CS, "Partial View"},
+    {0x0028, 0x1351, ST, "Partial View Description"},
+    {0x0028, 0x1352, SQ, "Partial View CodeSequence"},
+    {0x0028, 0x135A, CS, "Spatial LocationsPreserved"},
+    {0x0028, 0x1401, SQ, "Data Frame Assignment Sequence"},
+    {0x0028, 0x1402, CS, "Data Path Assignment"},
+    {0x0028, 0x1403, US, "Bits Mapped to Color Lookup Table"},
+    {0x0028, 0x1404, SQ, "Blending LUT 1 Sequence"},
+    {0x0028, 0x1405, CS, "Blending LUT 1 Transfer Function"},
+    {0x0028, 0x1406, FD, "Blending Weight Constant"},
+    {0x0028, 0x1407, US, "Blending Lookup Table Descriptor"},
+    {0x0028, 0x1408, OW, "Blending Lookup Table Data"},
+    {0x0028, 0x140B, SQ, "Enhanced Palette Color Lookup Table Sequence"},
+    {0x0028, 0x140C, SQ, "Blending LUT 2 Sequence"},
+    {0x0028, 0x140D, CS, "Blending LUT 2 Transfer Function"},
+    {0x0028, 0x140E, CS, "Data Path ID"},
+    {0x0028, 0x140F, CS, "RGB LUT Transfer Function"},
+    {0x0028, 0x1410, CS, "Alpha LUT Transfer Function"},
+    {0x0028, 0x2000, OB, "ICC Profile"},
+    {0x0028, 0x2002, CS, "Color Space"},
+    {0x0028, 0x2110, CS, "Lossy Image Compression"},
+    {0x0028, 0x2112, DS, "Lossy Image Compression Ratio"},
+    {0x0028, 0x2114, CS, "Lossy Image Compression Method"},
+    {0x0028, 0x3000, SQ, "Modality LUT Sequence"},
+    {0x0028, 0x3002, US, "LUT Descriptor"},
+    {0x0028, 0x3002, SS, "LUT Descriptor"},
+    {0x0028, 0x3003, LO, "LUT Explanation"},
+    {0x0028, 0x3004, LO, "Modality LUT Type"},
+    {0x0028, 0x3006, US, "LUT Data"},
+    {0x0028, 0x3006, OW, "LUT Data"},
+    {0x0028, 0x3010, SQ, "VOI LUT Sequence"},
+    {0x0028, 0x4000, LT, "Image Presentation Comments"},
+    {0x0028, 0x5000, SQ, "Bi-Plane Acquisition Sequence"},
+    {0x0028, 0x6010, US, "Representative Frame Number"},
+    {0x0028, 0x6020, US, "Frame Numbers of Interest(FOI)"},
+    {0x0028, 0x6022, LO, "Frame of Interest Description"},
+    {0x0028, 0x6023, CS, "Frame of Interest Type "},
+    {0x0028, 0x6030, US, "Mask Pointer(s)"},
+    {0x0028, 0x6040, US, "R Wave Pointer"},
+    {0x0028, 0x6100, SQ, "Mask Subtraction Sequence"},
+    {0x0028, 0x6101, CS, "Mask Operation"},
+    {0x0028, 0x6102, US, "Applicable Frame Range"},
+    {0x0028, 0x6110, US, "Mask Frame Numbers"},
+    {0x0028, 0x6112, US, "Contrast Frame Averaging"},
+    {0x0028, 0x6114, FL, "Mask Sub-pixel Shift "},
+    {0x0028, 0x6120, SS, "TID Offset"},
+    {0x0028, 0x6190, ST, "Mask Operation Explanation"},
+    {0x0028, 0x7000, SQ, "Equipment Administrator Sequence"},
+    {0x0028, 0x7001, US, "Number Of Display Subsystems"},
+    {0x0028, 0x7002, US, "Current Configuration ID"},
+    {0x0028, 0x7003, US, "Display Subsystem ID"},
+    {0x0028, 0x7004, SH, "Display Subsystem Name"},
+    {0x0028, 0x7005, LO, "Display Subsystem Description"},
+    {0x0028, 0x7006, CS, "System Status"},
+    {0x0028, 0x7007, LO, "System Status Comment"},
+    {0x0028, 0x7008, SQ, "Target Luminance Characteristics Sequence"},
+    {0x0028, 0x7009, US, "Luminance Characteristics ID"},
+    {0x0028, 0x700A, SQ, "Display Subsystem Configuration Sequence"},
+    {0x0028, 0x700B, US, "Configuration ID"},
+    {0x0028, 0x700C, SH, "Configuration Name"},
+    {0x0028, 0x700D, LO, "Configuration Description"},
+    {0x0028, 0x700E, US, "Referenced Target Luminance Characteristics ID"},
+    {0x0028, 0x700F, SQ, "QA Results Sequence"},
+    {0x0028, 0x7010, SQ, "Display Subsystem QA Results Sequence"},
+    {0x0028, 0x7011, SQ, "Configuration QA Results Sequence"},
+    {0x0028, 0x7012, SQ, "Measurement Equipment Sequence"},
+    {0x0028, 0x7013, CS, "Measurement Functions"},
+    {0x0028, 0x7014, CS, "Measurement Equipment Type"},
+    {0x0028, 0x7015, SQ, "Visual Evaluation Result Sequence"},
+    {0x0028, 0x7016, SQ, "Display Calibration Result Sequence"},
+    {0x0028, 0x7017, US, "DDL Value"},
+    {0x0028, 0x7018, FL, "CIExy White Point"},
+    {0x0028, 0x7019, CS, "Display Function Type"},
+    {0x0028, 0x701A, FL, "Gamma Value"},
+    {0x0028, 0x701B, US, "Number Of Luminance Points"},
+    {0x0028, 0x701C, SQ, "Luminance Response Sequence"},
+    {0x0028, 0x701D, FL, "Target Minimum Luminance"},
+    {0x0028, 0x701E, FL, "Target Maximum Luminance"},
+    {0x0028, 0x701F, FL, "Luminance Value"},
+    {0x0028, 0x7020, LO, "Luminance Response Description"},
+    {0x0028, 0x7021, CS, "White Point Flag"},
+    {0x0028, 0x7022, SQ, "Display Device Type Code Sequence"},
+    {0x0028, 0x7023, SQ, "Display Subsystem Sequence"},
+    {0x0028, 0x7024, SQ, "Luminance Result Sequence"},
+    {0x0028, 0x7025, CS, "Ambient Light Value Source"},
+    {0x0028, 0x7026, CS, "Measured Characteristics"},
+    {0x0028, 0x7027, SQ, "Luminance Uniformity Result Sequence"},
+    {0x0028, 0x7028, SQ, "Visual Evaluation Test Sequence"},
+    {0x0028, 0x7029, CS, "Test Result"},
+    {0x0028, 0x702A, LO, "Test Result Comment"},
+    {0x0028, 0x702B, CS, "Test Image Validation"},
+    {0x0028, 0x702C, SQ, "Test Pattern Code Sequence"},
+    {0x0028, 0x702D, SQ, "Measurement Pattern Code Sequence"},
+    {0x0028, 0x702E, SQ, "Visual Evaluation Method Code Sequence"},
+    {0x0028, 0x7FE0, UR, "Pixel Data Provider URL"},
+    {0x0028, 0x9001, UL, "Data Point Rows"},
+    {0x0028, 0x9002, UL, "Data Point Columns"},
+    {0x0028, 0x9003, CS, "Signal Domain Columns"},
+    {0x0028, 0x9099, US, "Largest Monochrome Pixel Value"},
+    {0x0028, 0x9108, CS, "Data Representation"},
+    {0x0028, 0x9110, SQ, "Pixel Measures Sequence"},
+    {0x0028, 0x9132, SQ, "Frame VOILUT Sequence"},
+    {0x0028, 0x9145, SQ, "Pixel Value Transformation Sequence"},
+    {0x0028, 0x9235, CS, "Signal Domain Rows"},
+    {0x0028, 0x9411, FL, "Display Filter Percentage"},
+    {0x0028, 0x9415, SQ, "Frame Pixel Shift Sequence"},
+    {0x0028, 0x9416, US, "Subtraction Item ID"},
+    {0x0028, 0x9422, SQ, "Pixel Intensity Relationship LUT Sequence"},
+    {0x0028, 0x9443, SQ, "Frame Pixel Data Properties Sequence"},
+    {0x0028, 0x9444, CS, "Geometrical Properties"},
+    {0x0028, 0x9445, FL, "Geometric Maximum Distortion"},
+    {0x0028, 0x9446, CS, "Image Processing Applied"},
+    {0x0028, 0x9454, CS, "Mask Selection Mode"},
+    {0x0028, 0x9474, CS, "LUT Function"},
+    {0x0028, 0x9478, FL, "Mask Visibility Percentage"},
+    {0x0028, 0x9501, SQ, "Pixel Shift Sequence"},
+    {0x0028, 0x9502, SQ, "Region Pixel Shift Sequence"},
+    {0x0028, 0x9503, SS, "Vertices Of The Region"},
+    {0x0028, 0x9505, SQ, "Multi Frame Presentation Sequence"},
+    {0x0028, 0x9506, US, "Pixel Shift Frame Range"},
+    {0x0028, 0x9507, US, "LUT Frame Range"},
+    {0x0028, 0x9520, DS, "Image To Equipment Mapping Matrix"},
+    {0x0028, 0x9537, CS, "Equipment Coordinate System Identification"}
+};
+
+static int dcm_dict_comp(const void *vde, const void *vDd) {
+    DataElement *de = (DataElement *) vde;
+    DICOMDictionary *Dd = (DICOMDictionary *) vDd;
+
+    if (de->GroupNumber > Dd->GroupNumber)
+        return 1;
+    else if (de->GroupNumber < Dd->GroupNumber)
+        return -1;
+    else {
+        if (de->ElementNumber > Dd->ElementNumber)
+            return 1;
+        else if (de->ElementNumber < Dd->ElementNumber)
+            return -1;
+        else
+            return 0;
+    }
+}
+
+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);
+    de->is_found = 1;
+    return 0;
+}
\ No newline at end of file
diff --git a/libavformat/version.h b/libavformat/version.h
index 9814db8633..f835adea8d 100644
--- a/libavformat/version.h
+++ b/libavformat/version.h
@@ -32,7 +32,7 @@ 
 // Major bumping may affect Ticket5467, 5421, 5451(compatibility with Chromium)
 // Also please add any ticket numbers that you believe might be affected here
 #define LIBAVFORMAT_VERSION_MAJOR  58
-#define LIBAVFORMAT_VERSION_MINOR  31
+#define LIBAVFORMAT_VERSION_MINOR  32
 #define LIBAVFORMAT_VERSION_MICRO 101
 
 #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
-- 
2.22.0