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

[Moritz Barsnick]

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