[FFmpeg-devel] [PATCH V4] avformat: Add Dynacolor MVC Demuxer

Andreas Rheinhardt andreas.rheinhardt at gmail.com
Sat May 16 21:23:25 EEST 2020


Tom Needham:
> Please ignore my previous email as it was sent in error.
> 
> This demuxer adds support for demuxing files in the Dynacolor format
> such as the sample located at:
> 
> http://samples.ffmpeg.org/camera-dvr/dynacolor/dynacolor-camera-sample
> 
> However some decode errors are showing on the resulting MPEG4 stream.
> I don't know whether this is a bug with the demuxer or the file as there is only one sample
> but the output results in a 8 second mp4 file that is playable in VLC media player.
> 
> I believe that i have addressed the comments that I recieved in the previous review.
> Also i have managed to improve the efficiency of the demuxer and also made the probe function more useful.
> 
> Thanks
> Tom
> 
> Signed-off-by: Tom Needham <06needhamt at gmail.com>
> ---
>  Changelog                |   1 +
>  doc/general.texi         |   1 +
>  libavformat/Makefile     |   1 +
>  libavformat/allformats.c |   1 +
>  libavformat/dynacolor.c  | 411 +++++++++++++++++++++++++++++++++++++++
>  libavformat/dynacolor.h  | 209 ++++++++++++++++++++
>  libavformat/version.h    |   2 +-
>  7 files changed, 625 insertions(+), 1 deletion(-)
>  create mode 100644 libavformat/dynacolor.c
>  create mode 100644 libavformat/dynacolor.h
> 
> diff --git a/Changelog b/Changelog
> index 711861bda9..79d39494c9 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -54,6 +54,7 @@ version <next>:
>  - DERF demuxer
>  - CRI HCA decoder
>  - CRI HCA demuxer
> +- Dynacolor MVC Demuxer
>  
>  
>  version 4.2:
> diff --git a/doc/general.texi b/doc/general.texi
> index 752618a00b..4eb4716d87 100644
> --- a/doc/general.texi
> +++ b/doc/general.texi
> @@ -452,6 +452,7 @@ library:
>  @item DXA                       @tab   @tab X
>      @tab This format is used in the non-Windows version of the Feeble Files
>           game and different game cutscenes repacked for use with ScummVM.
> + at item Dynacolor MVC             @tab   @tab X
>  @item Electronic Arts cdata  @tab    @tab X
>  @item Electronic Arts Multimedia  @tab    @tab X
>      @tab Used in various EA games; files have extensions like WVE and UV2.
> diff --git a/libavformat/Makefile b/libavformat/Makefile
> index 8fd0d43721..4d1ca8b7ed 100644
> --- a/libavformat/Makefile
> +++ b/libavformat/Makefile
> @@ -169,6 +169,7 @@ OBJS-$(CONFIG_DV_MUXER)                  += dvenc.o
>  OBJS-$(CONFIG_DVBSUB_DEMUXER)            += dvbsub.o rawdec.o
>  OBJS-$(CONFIG_DVBTXT_DEMUXER)            += dvbtxt.o rawdec.o
>  OBJS-$(CONFIG_DXA_DEMUXER)               += dxa.o
> +OBJS-$(CONFIG_DYNACOLOR_DEMUXER)         += dynacolor.o
>  OBJS-$(CONFIG_EA_CDATA_DEMUXER)          += eacdata.o
>  OBJS-$(CONFIG_EA_DEMUXER)                += electronicarts.o
>  OBJS-$(CONFIG_EAC3_DEMUXER)              += ac3dec.o rawdec.o
> diff --git a/libavformat/allformats.c b/libavformat/allformats.c
> index 39d2c352f5..50f3926b05 100644
> --- a/libavformat/allformats.c
> +++ b/libavformat/allformats.c
> @@ -131,6 +131,7 @@ extern AVOutputFormat ff_dv_muxer;
>  extern AVInputFormat  ff_dvbsub_demuxer;
>  extern AVInputFormat  ff_dvbtxt_demuxer;
>  extern AVInputFormat  ff_dxa_demuxer;
> +extern AVInputFormat  ff_dynacolor_demuxer;
>  extern AVInputFormat  ff_ea_demuxer;
>  extern AVInputFormat  ff_ea_cdata_demuxer;
>  extern AVInputFormat  ff_eac3_demuxer;
> diff --git a/libavformat/dynacolor.c b/libavformat/dynacolor.c
> new file mode 100644
> index 0000000000..05a32b5299
> --- /dev/null
> +++ b/libavformat/dynacolor.c
> @@ -0,0 +1,411 @@
> +/*
> + * Dynacolor MVC Demuxer
> + * Copyright (c) 2020 Tom Needham
> + *
> + * 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 <time.h>
> +#include "avformat.h"
> +#include "internal.h"
> +#include "dynacolor.h"
> +#include "libavutil/channel_layout.h"
> +#include "libavutil/intreadwrite.h"
> +#include "libavutil/mathematics.h"
> +#include "libavutil/timecode.h"
> +#include "libavutil/avassert.h"

There is no assert in your patch.

> +
> +int ff_dyna_read_packet_header(AVFormatContext *ctx, AVIOContext *pb, unsigned char *pes_data, DynacolorPesHeader *pes,
> +                      unsigned int *size, time_t *time, DynacolorHeader *header, unsigned int *basicIdx_H, unsigned int *basicIdx_L,
> +                       unsigned char first)
> +{
> +    int ret = 0;
> +    unsigned int stream_format;
> +
> +    *(basicIdx_H) = avio_rl32(pb);
> +
> +    header->Basic.Header1    = *(basicIdx_H)&0xFF;
> +    header->Basic.Header2    = *(basicIdx_H) >> 8 & 0xFF;
> +    header->Basic.reserved   = *(basicIdx_H) >> 16 & 0x0F;
> +    header->Basic.Source     = *(basicIdx_H) >> 20 & 0x01;
> +    header->Basic.WidthBase  = *(basicIdx_H) >> 21 & 0x03;
> +    header->Basic.Reserved0  = *(basicIdx_H) >> 23 & 0x01;
> +    header->Basic.Channel    = *(basicIdx_H) >> 24 & 0x02;
> +    header->Basic.StreamType = *(basicIdx_H) >> 26 & 0x02;
> +    header->Basic.QI         = *(basicIdx_H) >> 28 & 0x0F;
> +
> +    *size = avio_rl32(pb);
> +    header->Basic.Size = *size;
> +
> +    *time = (time_t)avio_rl32(pb);
> +    header->Basic.Time = ((unsigned int) *time);

None of the callers of this function use this time at all, so exporting
it via a dedicated parameter is pointless. If any caller wanted this
value, it could read it via header->Basic.Time. It also means that you
can remove time.h. libavutil/timecode.h seems to be superfluous as well
(even without this change).

> +
> +    *(basicIdx_L) = avio_rl32(pb);
> +
> +    header->Basic.NTSC_PAL    = *(basicIdx_L)&0x01;
> +    header->Basic.ImgMode     = *(basicIdx_L) >> 1 & 0x01;
> +    header->Basic.Audio       = *(basicIdx_L) >> 2 & 0x01;
> +    header->Basic.DST         = *(basicIdx_L) >> 3 & 0x01;
> +    header->Basic.Covert      = *(basicIdx_L) >> 4 & 0x01;
> +    header->Basic.Vloss       = *(basicIdx_L) >> 5 & 0x01;
> +    header->Basic.AlarmIn     = *(basicIdx_L) >> 6 & 0x01;
> +    header->Basic.Motion      = *(basicIdx_L) >> 7 & 0x01;
> +    header->Basic.ExtraEnable = *(basicIdx_L) >> 8 & 0x01;
> +    header->Basic.EventEnable = *(basicIdx_L) >> 9 & 0x01;
> +    header->Basic.PPS         = *(basicIdx_L) >> 10 & 0x40;
> +    header->Basic.Type        = *(basicIdx_L) >> 16 & 0x08;
> +    header->Basic.Channel     = *(basicIdx_L) >> 19 & 0x20;
> +    header->Basic.Chksum      = *(basicIdx_L) >> 24 & 0xFF;
> +
> +    if (header->Basic.ExtraEnable) {
> +        // File has DynacolorExtraIdx header so parse it
> +        unsigned int start;
> +        unsigned char end;
> +        char checksum;
> +
> +        start = avio_rl32(pb);
> +
> +        header->Extra.IsDST_S_W      = start & 0x01;
> +        header->Extra.DST_Minutes    = start >> 1 & 0xFF;
> +        header->Extra.OverSpeed      = start >> 9 & 0x01;
> +        header->Extra.SkipPicCnt     = start >> 10 & 0x20;
> +        header->Extra.Exception      = start >> 15 & 0x01;
> +        header->Extra.SuperExtraSize = start >> 16 & 0xFFFF;
> +
> +        header->Extra.Serno     = avio_rl32(pb);
> +        header->Extra.AlmIn[0]  = (unsigned char)avio_r8(pb);
> +        header->Extra.AlmIn[1]  = (unsigned char)avio_r8(pb);
> +        header->Extra.Vloss[0]  = (unsigned char)avio_r8(pb);
> +        header->Extra.Vloss[1]  = (unsigned char)avio_r8(pb);
> +        header->Extra.Motion[0] = (unsigned char)avio_r8(pb);
> +        header->Extra.Motion[1] = (unsigned char)avio_r8(pb);
> +
> +        end = (unsigned char)avio_r8(pb);
> +
> +        header->Extra.IsDVRRec = end & 0x03;
> +        header->Extra.TimeZone = (end >> 2) & 0x40;
> +
> +        checksum = (char)avio_r8(pb);
> +
> +        header->Basic.Chksum = checksum & 0xFF;
> +
> +        header->SuperExtra.type        = (int)avio_rl32(pb);
> +        header->SuperExtra.remain_size = (int)avio_rl32(pb);
> +
> +        if (avio_read(pb, header->SuperExtra.title, DYNACOLOR_SUPEREXTRA_TITLE_SIZE) != DYNACOLOR_SUPEREXTRA_TITLE_SIZE)
> +            return AVERROR_INVALIDDATA;
> +
> +        if (avio_read(pb, header->SuperExtra.extra_data, DYNACOLOR_SUPEREXTRA_EXTRA_DATA_SIZE) != DYNACOLOR_SUPEREXTRA_EXTRA_DATA_SIZE)
> +            return AVERROR_INVALIDDATA;
> +
> +    } else {
> +        // File has no DynacolorExtraIdx header so skip it
> +        av_log(ctx, AV_LOG_DEBUG, "Skipping DynacolorExtraIdx and DynacolorSuperExtraIdx Header\n");
> +        avio_skip(pb, DYNACOLOR_EXTRA_SIZE + DYNACOLOR_SUPEREXTRA_SIZE);
> +
> +        memset(&header->Extra, 0xFF, DYNACOLOR_EXTRA_SIZE);
> +        memset(&header->SuperExtra, 0xFF, DYNACOLOR_SUPEREXTRA_SIZE);
> +    }
> +
> +    if (avio_read(pb, pes_data, DYNACOLOR_PES_HEADER_SIZE) != DYNACOLOR_PES_HEADER_SIZE)
> +        return AVERROR_INVALIDDATA;
> +
> +    ret = ff_dyna_build_pes_header(ctx, pes_data);
> +
> +    if (ret) {
> +        av_log(ctx, AV_LOG_ERROR, "Failed to Build PES Header\n");
> +        return AVERROR_INVALIDDATA;
> +    } else {
> +        if(first) {
> +            stream_format = ff_dyna_get_stream_format(ctx, header);
> +            if (!stream_format) {
> +                avpriv_report_missing_feature(ctx, "Format %02X,", stream_format);
> +                return AVERROR_PATCHWELCOME;
> +            } else if (stream_format == AVERROR_INVALIDDATA) {
> +                av_log(ctx, AV_LOG_ERROR, "Could not Determine Stream Format\n");
> +                return AVERROR_INVALIDDATA;
> +            }
> +            av_log(ctx, AV_LOG_DEBUG, "File Has 0x%02X Stream Format\n", stream_format);
> +        }
> +    }
> +
> +    return ret;
> +}
> +
> +int ff_dyna_get_stream_format(AVFormatContext *ctx, DynacolorHeader *header)
> +{
> +    DynacolorContext *priv = ctx->priv_data;
> +    int ret = 0;
> +
> +    // Try And Build Header for H264 Format
> +    av_log(ctx, AV_LOG_DEBUG, "Validating H264 PES Header\n");
> +    ret = ff_dyna_check_pes_packet_header(ctx, DYNACOLOR_H264_FORMAT_PREFIX);
> +
> +    if (!ret) {
> +        // Format was not H264 so try And Build Header for MPEG4 Format
> +        av_log(ctx, AV_LOG_DEBUG, "Validating MPEG4 PES Header\n");
> +        ret = ff_dyna_check_pes_packet_header(ctx, DYNACOLOR_MPEG4_FORMAT_PREFIX);
> +
> +        if (!ret) {
> +            // Format was not MPEG4 or H264 so try And Build Header for JPEG Format
> +            av_log(ctx, AV_LOG_DEBUG, "Validating JPEG PES Header\n");
> +            ret = ff_dyna_check_pes_packet_header(ctx, DYNACOLOR_JPEG_FORMAT_PREFIX);
> +
> +            if (!ret) {
> +                // Format was not MPEG4 or H264 or JPEG so try And Build Header for AUDIO Format
> +                av_log(ctx, AV_LOG_DEBUG, "Validating AUDIO PES Header\n");
> +                ret = ff_dyna_check_pes_packet_header(ctx, DYNACOLOR_AUDIO_FORMAT_PREFIX);
> +
> +                if (!ret) {
> +                    avpriv_report_missing_feature(ctx, "Format %02X,", priv->pes_header.format_id);
> +                    return AVERROR_PATCHWELCOME;
> +                } else {
> +                    av_log(ctx, AV_LOG_DEBUG, "Successfully Validated AUDIO PES Header\n");
> +                    return DYNACOLOR_JPEG_FORMAT_PREFIX;
> +                }
> +            } else {
> +                av_log(ctx, AV_LOG_DEBUG, "Successfully Validated JPEG PES Header\n");
> +                return DYNACOLOR_JPEG_FORMAT_PREFIX;
> +            }
> +        } else {
> +            av_log(ctx, AV_LOG_DEBUG, "Successfully Validated MPEG4 PES Header\n");
> +            return DYNACOLOR_MPEG4_FORMAT_PREFIX;
> +        }
> +    } else {
> +        av_log(ctx, AV_LOG_DEBUG, "Successfully Validated H264 PES Header\n");
> +        return DYNACOLOR_H264_FORMAT_PREFIX;
> +    }
> +
> +    return 0;
> +}
> +
> +int ff_dyna_build_pes_header(AVFormatContext *ctx, uint8_t *pes_data)

Don't use the AVFormatContext as function argument here (this will make
this function only usable if the ctx's priv_data is a DynacolorContext).
Instead use a pointer to the destination DynacolorPesHeader directly.
And make the source pes_data const.

> +{
> +    DynacolorContext *priv = ctx->priv_data;
> +
> +    priv->pes_header.start_code0 = pes_data[0];
> +    priv->pes_header.start_code1 = pes_data[1];
> +    priv->pes_header.start_code2 = pes_data[2];
> +    priv->pes_header.format_id   = pes_data[3] & 0x0F;
> +    priv->pes_header.channel_id  = pes_data[3] >> 8 & 0x0F;
> +
> +    priv->pes_header.unused_0 = MKTAG(pes_data[4], pes_data[5], pes_data[6], pes_data[7]);
> +    priv->pes_header.unused_1 = MKTAG(pes_data[8], pes_data[9], pes_data[10], pes_data[11]);
> +    priv->pes_header.unused_2 = MKTAG(pes_data[12], pes_data[13], pes_data[14], pes_data[15]);
> +    priv->pes_header.unused_3 = MKTAG(pes_data[16], pes_data[17], pes_data[18], pes_data[19]);
> +    priv->pes_header.unused_4 = MKTAG(pes_data[20], pes_data[21], pes_data[22], pes_data[23]);
> +
> +    priv->pes_header.size_bit7to0   = pes_data[24];
> +    priv->pes_header.size_bit10to8  = pes_data[25] & 0x08;
> +    priv->pes_header.size_bit14to11 = pes_data[26] & 0xF;
> +    priv->pes_header.size_bit21to15 = pes_data[27];
> +    priv->pes_header.size_marker0   = pes_data[28] & 0x01;
> +    priv->pes_header.size_marker1   = pes_data[29] & 0x01;
> +    priv->pes_header.picture_type   = pes_data[30];
> +    priv->pes_header.is_interleaved = pes_data[31] & 0x01;
> +    priv->pes_header.field_id       = pes_data[31] >> 1 & 0x03;
> +
> +    return 0;
> +}
> +
> +int ff_dyna_check_pes_packet_header(AVFormatContext *ctx, int format_prefix)
> +{
> +    DynacolorContext *dyna = ctx->priv_data;
> +    DynacolorPesHeader *pes = &dyna->pes_header;
> +
> +    return pes->format_id == format_prefix;
> +}
> +
> +static int dyna_read_probe(const AVProbeData *p)
> +{
> +    int magic = MKTAG(p->buf[0], p->buf[1], p->buf[2], p->buf[3]);
> +
> +    if (magic == 0x20103240)
> +        return AVPROBE_SCORE_MAX;
> +
> +    return 0;
> +}
> +
> +static int dyna_read_header(AVFormatContext *ctx)
> +{
> +    int ret = 0;
> +    DynacolorContext *priv = ctx->priv_data;
> +    AVIOContext *pb = ctx->pb;
> +
> +    AVStream *vstream = NULL;
> +    AVCodec *vcodec = NULL;
> +    AVCodecParameters *vcodec_params = NULL;
> +
> +    AVStream *astream = NULL;
> +    AVCodec *acodec = NULL;
> +    AVCodecParameters *acodec_params = NULL;
> +
> +    time_t time;
> +    unsigned char *pes_data;
> +    DynacolorPesHeader *pes;
> +    unsigned int size;
> +
> +    unsigned int basicIdx_H, basicIdx_L;
> +
> +    pes_data = av_malloc_array(DYNACOLOR_PES_HEADER_SIZE, 1);
> +    pes = av_malloc(sizeof(DynacolorPesHeader));

Why do you allocate these at all and not just use some arrays/structs on
the stack? Both their size as well as their lifetime point to putting
them on the stack. The same goes for your read_packet function.

(And btw: It is project policy to check for allocation failures which
you didn't do here. And using av_malloc_array to allocate an array with
exactly one member is pointless.)

> +
> +    ret = ff_dyna_read_packet_header(ctx, pb, pes_data, pes, &size, &time,
> +                            &priv->header, &basicIdx_H, &basicIdx_L, 1);
> +
> +    if (ret) {
> +        ret = AVERROR_INVALIDDATA;
> +        goto end;
> +    }
> +
> +    if (priv->pes_header.format_id == DYNACOLOR_JPEG_FORMAT_PREFIX) {
> +        av_log(ctx, AV_LOG_DEBUG, "Demuxing JPEG Video Stream\n");
> +
> +        vcodec = avcodec_find_decoder(AV_CODEC_ID_MJPEG);

This is unnecessary; you should instead set the codec id directly.

> +        vstream = avformat_new_stream(ctx, vcodec);
> +        vcodec_params = vstream->codecpar;
> +
> +        if (!vstream || !vcodec_params) {

You do not need to check for the existence of codecpar. It is always
properly allocated if avformat_new_stream() succeeds (returns non-NULL).

> +            ret = AVERROR_INVALIDDATA;

AVERROR(ENOMEM), not AVERROR_INVALIDDATA. AVERROR_INVALIDDATA makes no
sense here at all.

> +            goto end;
> +        }
> +
> +    } else if (priv->pes_header.format_id == DYNACOLOR_MPEG4_FORMAT_PREFIX) {
> +        av_log(ctx, AV_LOG_DEBUG, "Demuxing MPEG4 Video Stream\n");
> +
> +        vcodec = avcodec_find_decoder(AV_CODEC_ID_MPEG4);
> +        vstream = avformat_new_stream(ctx, vcodec);
> +        vcodec_params = vstream->codecpar;
> +
> +        if (!vstream || !vcodec_params) {
> +            ret = AVERROR_INVALIDDATA;
> +            goto end;
> +        }
> +
> +    } else if (priv->pes_header.format_id == DYNACOLOR_H264_FORMAT_PREFIX) {
> +        av_log(ctx, AV_LOG_DEBUG, "Demuxing H264 Video Stream\n");
> +
> +        vcodec = avcodec_find_decoder(AV_CODEC_ID_H264);
> +        vstream = avformat_new_stream(ctx, vcodec);
> +        vcodec_params = vstream->codecpar;
> +
> +        if (!vstream || !vcodec_params) {
> +            ret = AVERROR_INVALIDDATA;
> +            goto end;
> +        }
> +
> +    } else if (priv->pes_header.format_id == DYNACOLOR_AUDIO_FORMAT_PREFIX) {
> +        av_log(ctx, AV_LOG_DEBUG, "Demuxing Audio Stream\n");
> +
> +        acodec = avcodec_find_decoder(AV_CODEC_ID_PCM_F16LE);
> +        astream = avformat_new_stream(ctx, acodec);
> +        acodec_params = astream->codecpar;
> +
> +        if (!astream || !acodec_params) {
> +            ret = AVERROR_INVALIDDATA;
> +            goto end;
> +        }
> +    }
> +
> +end:
> +    av_free(pes_data);
> +    av_free(pes);
> +    
> +    return ret;
> +}
> +
> +static int dyna_read_packet(AVFormatContext *ctx, AVPacket *pkt)
> +{
> +    int ret = 0;
> +    DynacolorContext *priv = ctx->priv_data;
> +    AVIOContext *pb = ctx->pb;
> +    DynacolorPesHeader *pes;
> +
> +    time_t time;
> +    unsigned char *pes_data;
> +    unsigned char *pkt_data;
> +    unsigned int size;
> +
> +    unsigned int basicIdx_H, basicIdx_L;
> +
> +    pes_data = av_malloc_array(DYNACOLOR_PES_HEADER_SIZE, 1);
> +    pes = av_malloc(sizeof(DynacolorPesHeader));
> +
> +    ret = ff_dyna_read_packet_header(ctx, pb, pes_data, pes, &size, &time,
> +                            &priv->header, &basicIdx_H, &basicIdx_L, 0);
> +
> +    size = (priv->pes_header.size_bit7to0 & 0xFF)
> +        | ((priv->pes_header.size_bit10to8 & 0x04) << 15)
> +        | ((priv->pes_header.size_bit14to11 & 0x08) << 11)
> +        | ((priv->pes_header.size_bit21to15 & 0x7F) << 8);

None of the callers of ff_dyna_read_packet_header() use the size that is
returned via the size pointer argument at all. The same applies to
basicIdx_H/L. This is a sure sign that the function is misdesigned.

> +
> +    pkt_data = av_malloc(size);
> +    ret = avio_read(pb, pkt_data, size);
> +
> +    if (ret != size)
> +        goto end;
> +
> +    ret = av_packet_from_data(pkt, pkt_data, size);
> +
> +    if (ret < 0) {
> +        av_log(ctx, AV_LOG_ERROR, "Error Building Packet\n");
> +        goto end;
> +    }

Use av_get_packet(). (You btw don't check the allocation of pkt_data and
if av_packet_from_data() fails, pkt_data leaks.)

> +
> +end:
> +    av_free(pes_data);
> +    av_free(pes);
> +
> +    return ret;
> +}
> +
> +static int dyna_read_close(AVFormatContext *ctx)
> +{
> +    return 0;
> +}

You don't need a read_close() function at all if it does nothing.

> +
> +static int dyna_read_seek(AVFormatContext *ctx, int stream_index,
> +                        int64_t timestamp, int flags)
> +{
> +    DynacolorContext *priv = ctx->priv_data;
> +    unsigned int size = 0;
> +    int64_t ret = 0L;
> +
> +    size = (priv->pes_header.size_bit7to0 & 0xFF)
> +        | ((priv->pes_header.size_bit10to8 & 0x04) << 15)
> +        | ((priv->pes_header.size_bit14to11 & 0x08) << 11)
> +        | ((priv->pes_header.size_bit21to15 & 0x7F) << 8);
> +
> +    ret = avio_seek(ctx->pb, size, SEEK_SET);

Wait, you always seek to the same point regardless of the desired timestamp?

> +
> +    if(ret < 0)
> +        return ret;
> +
> +    return 0;
> +}
> +
> +AVInputFormat ff_dynacolor_demuxer = {
> +    .name = "dynacolor",
> +    .long_name = NULL_IF_CONFIG_SMALL("Dynacolor MVC"),
> +    .priv_data_size = sizeof(DynacolorContext),
> +    .read_probe = dyna_read_probe,
> +    .read_header = dyna_read_header,
> +    .read_packet = dyna_read_packet,
> +    .read_close = dyna_read_close,
> +    .read_seek = dyna_read_seek,
> +    .extensions = "dyna,dyn",
> +};
> diff --git a/libavformat/dynacolor.h b/libavformat/dynacolor.h
> new file mode 100644
> index 0000000000..a620ed7a2e
> --- /dev/null
> +++ b/libavformat/dynacolor.h
> @@ -0,0 +1,209 @@
> +/*
> + * Dynacolor MVC Demuxer
> + * Copyright (c) 2020 Tom Needham
> + *
> + * 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
> + */
> +
> +#ifndef AVFORMAT_DYNACOLOR_H
> +#define AVFORMAT_DYNACOLOR_H
> +
> +#include "avformat.h"
> +
> +// The data structure of the streaming video:
> +// [16-byte DynacolorBasicIdx header] + [16-byte DynacolorExtraIdx header] + ['SuperExtraSize' bytes] + [32-byte PES header] + [video body]
> +// The 'SuperExtraSize' is defined in the 'DynacolorExtraIdx' structure to store the additional information, and usually equals zero.
> +
> +#define DYNACOLOR_PES_HEADER_SIZE 32
> +#define DYNACOLOR_EXTRA_SIZE 16
> +#define DYNACOLOR_SUPEREXTRA_SIZE 108
> +#define DYNACOLOR_SUPEREXTRA_TITLE_SIZE 12
> +#define DYNACOLOR_SUPEREXTRA_EXTRA_DATA_SIZE 96
> +
> +#define DYNACOLOR_AUDIO_FORMAT_PREFIX 0x0C
> +#define DYNACOLOR_MPEG4_FORMAT_PREFIX 0x0D
> +#define DYNACOLOR_JPEG_FORMAT_PREFIX 0x0E
> +#define DYNACOLOR_H264_FORMAT_PREFIX 0x0F
> +
> +// Big Endian
> +// DG600 uses big-endian CPU
> +
> +// Basic Idx: 16 bytes
> +typedef struct
> +{
> +    unsigned int Header1 : 8;   // [2] "@X" -- X:device type, DG600 is "6" @6
> +    unsigned int Header2 : 8;   // [2] "@X" -- X:device type, DG600 is "6" @6
> +    unsigned int reserved : 4;  // 4 bits
> +    unsigned int Source : 1;    // 1 bit  : (0 - DVR, 1 - IP Dev)
> +    unsigned int WidthBase : 2; // 0:720 , 1:704, 2:640
> +    unsigned int Reserved0 : 1; // 1 bit  : (0 - Disable(backward compatibile), 1 - Enable)
> +                                // this is basically for VSS use since VSS needs to support 96 channels, and
> +                                // there is originally 5 bits only for channel number in the header.
> +                                // With these two extra bits, the channel number becomes ((Channel_Ext<<5)|Channel).
> +    unsigned int Channel_Ext : 2;
> +    unsigned int StreamType : 2; // 0-NormalStream, 1-VStream, 2-DualStream
> +    unsigned int QI : 4;         // 4 bits : QI (0~16)
> +
> +    unsigned int Size; // [4]
> +    unsigned int Time; // [4]
> +
> +    unsigned int NTSC_PAL : 1; // 1 bit : 0:NTSC/1:PAL
> +    unsigned int ImgMode : 1;  // 1 bit : 0:Live/1:Playback
> +    unsigned int Audio : 1;    // 1 bit : 0:Video/1:Audio
> +    unsigned int DST : 1;      // 1 bit : DST
> +    unsigned int Covert : 1;   // 1 bit : Covert
> +    unsigned int Vloss : 1;    // 1 bit : Video loss
> +    unsigned int AlarmIn : 1;  // 1 bit : Alarm In
> +    unsigned int Motion : 1;   // 1 bit : Motion
> +
> +    unsigned int ExtraEnable : 1; // 1 bit  : 0: no extra         1: have extra
> +    unsigned int EventEnable : 1; // 1 bit  : 0: Normal status    1: Event duration
> +    unsigned int PPS : 6;         // 6 bits : Per-channel PPS Idx (0~127 level)
> +
> +    unsigned int Type : 3;    // 3 bits : type (0~7) 0: none 1: I-pic, 2: P-Pic, 3: B-Pic
> +    unsigned int Channel : 5; // 5 bits : Channel No (0~31)
> +    unsigned int Chksum : 8;  // [1]
> +
> +} DynacolorBasicIdx;
> +
> +// Extra Idx: 16 bytes
> +
> +typedef struct
> +{
> +    unsigned int DST_Minutes : 8; // 0~7 = 0 ~ 120 minutes
> +    unsigned int IsDST_S_W : 1;   //   0 = summer time , 1 =winter time
> +    unsigned int OverSpeed : 1;
> +    unsigned int SkipPicCnt : 5;
> +
> +    // use one bit to represent exception alarm
> +    // can't get exactually exception no here
> +    unsigned int Exception : 1;
> +    unsigned int SuperExtraSize : 16;
> +
> +    unsigned int Serno; // [4]
> +
> +    // Original tEventInfo16CH was broken 'cause of the byte alignment problem
> +    // - use the following directly
> +    // the first bit can descript first channel's evnet status, so that it can totally script 16 channels' status.
> +    // if IP camera, the first bit also descripts the first IP cmaera's status.
> +    char AlmIn[2];  // 16 channel, if Source of DynacolorBasicIdx = 0, it means analog camera's status. If Source = 1, it means IP camera's.
> +    char Vloss[2];  // 16 channel, if Source of DynacolorBasicIdx = 0, it means analog camera's status. If Source = 1, it means IP camera's.
> +    char Motion[2]; // 16 channel, if Source of DynacolorBasicIdx = 0, it means analog camera's status. If Source = 1, it means IP camera's.
> +
> +    unsigned char IsDVRRec : 2;
> +
> +    // For TimeZone setting
> +    // - 0 means OFF (backward compatibility)
> +    // - Delta : 30 min
> +    // - Start from : -12:00
> +    // - Aka.
> +    //    1 : -12:00 (-720 min)
> +    //    2 : -11:30 (-690 min)
> +    //    3 : -11:00 (-660 min)
> +    //    ....
> +    //   25 : +00:00 (+000 min)
> +    //    ....
> +    //   41 : +08:00 (+480 min)
> +    //   42 : +08:30 (+510 min)
> +    //   43 : +09:00 (+540 min)
> +    //    ....
> +    //   51 : +13:00 (+780 min)
> +
> +    unsigned char TimeZone : 6;
> +    char Chksum;
> +
> +} DynacolorExtraIdx;
> +
> +// Super Extra Index :
> +//  type = 0
> +//  remain_size = 108
> +// structure to store cam title & extra data(GPS/Text)
> +
> +typedef struct
> +{
> +
> +    // the type for DynacolorExtraIdx is fixed to 0
> +    int type;
> +
> +    // the remain_size for DynacolorExtraIdx is fixed to : 12 + 96 = 108
> +    int remain_size;
> +
> +    // CAM_TIT_LEN
> +    char title[12];
> +
> +    //   Merge original account0 ~ account3
> +    // - GPS & Text support will use the same place
> +    // - REC_INDEX_EXTRA_TYPE, REC_INDEX_EXTRA_CH, REC_INDEX_EXTRA_LENGTH are
> +    //   used for some specific info
> +    // - refer to list.h for detail
> +
> +    char extra_data[96];
> +
> +} DynacolorSuperExtraIdx;
> +
> +typedef struct
> +{
> +    DynacolorBasicIdx Basic;
> +    DynacolorExtraIdx Extra;
> +    DynacolorSuperExtraIdx SuperExtra;
> +} DynacolorHeader;
> +
> +typedef struct
> +{
> +    // 4 bytes
> +    unsigned int start_code0 : 8; // must be 0x00
> +    unsigned int start_code1 : 8; // must be 0x00
> +    unsigned int start_code2 : 8; // must be 0x01
> +    unsigned int format_id : 4;   // JPEG:0xd, MPEG4:0xe, H.264:0xf, Audio:0xc
> +    unsigned int channel_id : 4;  // channel id from 0 to 15 for CH1 to CH16
> +
> +    unsigned int unused_0;
> +    unsigned int unused_1;
> +    unsigned int unused_2;
> +    unsigned int unused_3;
> +    unsigned int unused_4;
> +
> +    // size of the video body and this PES header (total 3 bytes including the markers)
> +    unsigned int size_bit7to0 : 8;   // from bit7 to bit0
> +    unsigned int size_bit10to8 : 3;  // from bit10 to bit8
> +    unsigned int size_bit14to11 : 4; // from bit14 to bit11
> +    unsigned int size_bit21to15 : 7; // from bit21 to bit15
> +    unsigned int size_marker0 : 1;   // must be 0x1
> +    unsigned int size_marker1 : 1;   // must be 0x1
> +
> +    // 1 byte for the picture type
> +    unsigned int picture_type : 8;   // 1: I-slice, 2: P-slice, 3: B-slice
> +    unsigned int is_interleaved : 1; // 0: even/odd fields are separated horizontally
> +                                     // 1: even/odd fields are interleaved
> +    unsigned int field_id : 2;       // 0: odd field, 1: even field, 2/3: undefined
> +    unsigned int unused_5 : 29;
> +
> +} DynacolorPesHeader;
> +
> +typedef struct {
> +    DynacolorHeader header;
> +    DynacolorPesHeader pes_header;
> +} DynacolorContext;
> +
> +int ff_dyna_read_packet_header(AVFormatContext *ctx, AVIOContext *pb, unsigned char *pes_data, DynacolorPesHeader *pes, unsigned int *size,
> +                               time_t *time, DynacolorHeader *cdata_B, unsigned int *basicIdx_H, unsigned int *basicIdx_L,
> +                               unsigned char first);
> +int ff_dyna_get_stream_format(AVFormatContext *ctx, DynacolorHeader *header);
> +int ff_dyna_build_pes_header(AVFormatContext *ctx, uint8_t *pes_data);
> +int ff_dyna_check_pes_packet_header(AVFormatContext *ctx, int format_prefix);
> +
> +#endif
> diff --git a/libavformat/version.h b/libavformat/version.h
> index 18c2f5fec2..493a0b337f 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  42
> +#define LIBAVFORMAT_VERSION_MINOR  43
>  #define LIBAVFORMAT_VERSION_MICRO 100
>  
>  #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
> 
Moreover you are using CamelCase for non-types. And I don't see the
point in using external linkage (and an external header) for several of
your functions -- where do you want to reuse them? (Even if you add a
muxer for this format lateron, said muxer won't receive its data in the
form that you read it in the file (i.e. with all these PES headers) and
so you won't be able to reuse e.g. ff_dyna_read_packet_header() at all.)

Furthermore, why are you trying to directly recreate the file layout in
your structs (by using bitfields and by keeping data that is not used
lateron)?

- Andreas


More information about the ffmpeg-devel mailing list