[FFmpeg-devel] [PATCH] avcodec: Implement DM PAR Muxer/Demuxer
Andreas Rheinhardt
andreas.rheinhardt at gmail.com
Sat Jul 6 01:54:00 EEST 2019
Tom Needham:
> Samples are Available from
>
> https://transfernow.net/131xk9g4u0jt
>
> Signed-off-by: Thomas Needham <06needhamt at gmail.com>
> ---
> Changelog | 1 +
> configure | 7 +
> libavformat/Makefile | 10 +
> libavformat/adaudio.c | 137 ++++
> libavformat/adbinary.c | 609 ++++++++++++++++
> libavformat/adcommon.c | 1106 +++++++++++++++++++++++++++++
> libavformat/adffmpeg_errors.h | 94 +++
> libavformat/adjfif.c | 551 ++++++++++++++
> libavformat/adjfif.h | 41 ++
> libavformat/admime.c | 822 +++++++++++++++++++++
> libavformat/adpic.h | 116 +++
> libavformat/adraw.c | 131 ++++
> libavformat/allformats.c | 7 +
> libavformat/ds.c | 1262 +++++++++++++++++++++++++++++++++
> libavformat/ds.h | 135 ++++
> libavformat/ds_exports.h | 173 +++++
> libavformat/dsenc.c | 488 +++++++++++++
> libavformat/dsenc.h | 35 +
> libavformat/dspic.c | 317 +++++++++
> libavformat/libpar.c | 1030 +++++++++++++++++++++++++++
> libavformat/libpar.h | 40 ++
> libavformat/netvu.c | 214 ++++++
> libavformat/netvu.h | 21 +
> libavformat/version.h | 4 +-
> 24 files changed, 7349 insertions(+), 2 deletions(-)
> create mode 100644 libavformat/adaudio.c
> create mode 100644 libavformat/adbinary.c
> create mode 100644 libavformat/adcommon.c
> create mode 100644 libavformat/adffmpeg_errors.h
> create mode 100644 libavformat/adjfif.c
> create mode 100644 libavformat/adjfif.h
> create mode 100644 libavformat/admime.c
> create mode 100644 libavformat/adpic.h
> create mode 100644 libavformat/adraw.c
> create mode 100644 libavformat/ds.c
> create mode 100644 libavformat/ds.h
> create mode 100644 libavformat/ds_exports.h
> create mode 100644 libavformat/dsenc.c
> create mode 100644 libavformat/dsenc.h
> create mode 100644 libavformat/dspic.c
> create mode 100644 libavformat/libpar.c
> create mode 100644 libavformat/libpar.h
> create mode 100644 libavformat/netvu.c
> create mode 100644 libavformat/netvu.h
>
> diff --git a/Changelog b/Changelog
> index 86167b76a1..41d12c092e 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -35,6 +35,7 @@ version <next>:
> - IFV demuxer
> - derain filter
> - deesser filter
> +- AD Holdings PAR Muxer and Demuxer
>
>
> version 4.1:
> diff --git a/configure b/configure
> index 7cea9d4d73..39c4356c00 100755
> --- a/configure
> +++ b/configure
> @@ -317,6 +317,7 @@ External library support:
> --enable-vapoursynth enable VapourSynth demuxer [no]
> --disable-xlib disable xlib [autodetect]
> --disable-zlib disable zlib [autodetect]
> + --enable-libparreader enable PAR (de)muxing via libparreader [no]
>
> The following libraries provide various hardware acceleration features:
> --disable-amf disable AMF video encoding code [autodetect]
> @@ -1720,6 +1721,7 @@ EXTERNAL_LIBRARY_NONFREE_LIST="
> libfdk_aac
> openssl
> libtls
> + libparreader
> "
>
> EXTERNAL_LIBRARY_VERSION3_LIST="
> @@ -2768,6 +2770,8 @@ on2avc_decoder_select="mdct"
> opus_decoder_deps="swresample"
> opus_decoder_select="mdct15"
> opus_encoder_select="audio_frame_queue mdct15"
> +libparreader_demuxer_deps="libparreader"
> +libparreader_muxer_deps="libparreader"
> png_decoder_deps="zlib"
> png_encoder_deps="zlib"
> png_encoder_select="llvidencdsp"
> @@ -6136,7 +6140,10 @@ for func in $COMPLEX_FUNCS; do
> done
>
> # these are off by default, so fail if requested and not available
> +
> enabled cuda_nvcc && { check_nvcc || die "ERROR: failed checking
> for nvcc."; }
> +enabled libparreader && require libparreader "parreader.h
> parreader_types.h" parReader_getPicStructSize -lparreader
> +enabled cuda_sdk && require cuda_sdk cuda.h cuCtxCreate -lcuda
> enabled chromaprint && require chromaprint chromaprint.h
> chromaprint_get_version -lchromaprint
> enabled decklink && { require_headers DeckLinkAPI.h &&
> { test_cpp_condition DeckLinkAPIVersion.h
> "BLACKMAGIC_DECKLINK_API_VERSION >= 0x0a090500" || die "ERROR: Decklink API
> version must be >= 10.9.5."; } }
> diff --git a/libavformat/Makefile b/libavformat/Makefile
> index a434b005a4..363159009d 100644
> --- a/libavformat/Makefile
> +++ b/libavformat/Makefile
> @@ -569,11 +569,21 @@ OBJS-$(CONFIG_YUV4MPEGPIPE_DEMUXER) +=
> yuv4mpegdec.o
> OBJS-$(CONFIG_YUV4MPEGPIPE_MUXER) += yuv4mpegenc.o
>
> # external library muxers/demuxers
> +OBJS-$(CONFIG_ADAUDIO_DEMUXER) += adaudio.o
> +OBJS-$(CONFIG_ADBINARY_DEMUXER) += adbinary.o adcommon.o adjfif.o
> +OBJS-$(CONFIG_ADMIME_DEMUXER) += admime.o adcommon.o adjfif.o
> +OBJS-$(CONFIG_ADRAW_DEMUXER) += adraw.o adcommon.o adjfif.o
> OBJS-$(CONFIG_AVISYNTH_DEMUXER) += avisynth.o
> OBJS-$(CONFIG_CHROMAPRINT_MUXER) += chromaprint.o
> +OBJS-$(CONFIG_DM_PROTOCOL) += dsenc.o ds.o
> +OBJS-$(CONFIG_DSPIC_DEMUXER) += ds.o dspic.o adcommon.o
> OBJS-$(CONFIG_LIBGME_DEMUXER) += libgme.o
> OBJS-$(CONFIG_LIBMODPLUG_DEMUXER) += libmodplug.o
> OBJS-$(CONFIG_LIBOPENMPT_DEMUXER) += libopenmpt.o
> +OBJS-$(CONFIG_LIBPARREADER_DEMUXER) += libpar.o adcommon.o
> +OBJS-$(CONFIG_LIBPARREADER_MUXER) += libpar.o adcommon.o
> +OBJS-$(CONFIG_LIBMODPLUG_DEMUXER) += libmodplug.o
> +OBJS-$(CONFIG_NETVU_PROTOCOL) += netvu.o
> OBJS-$(CONFIG_VAPOURSYNTH_DEMUXER) += vapoursynth.o
>
> # protocols I/O
> diff --git a/libavformat/adaudio.c b/libavformat/adaudio.c
> new file mode 100644
> index 0000000000..0c28497953
> --- /dev/null
> +++ b/libavformat/adaudio.c
> @@ -0,0 +1,137 @@
> +/*
> + * AD-Holdings demuxer for AD audio stream format
> + * Copyright (c) 2006-2010 AD-Holdings plc
> + * Modified for FFmpeg by Tom Needham <06needhamt at gmail.com> (2018)
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> 02110-1301 USA
> + */
> +
> +/**
> + * @file
> + * AD-Holdings demuxer for AD audio stream format
> + */
> +
> +#include "avformat.h"
> +#include "ds_exports.h"
> +#include "adpic.h"
> +
> +
> +#define SIZEOF_RTP_HEADER 12
> +
> +
> +static int adaudio_probe(AVProbeData *p);
> +static int adaudio_read_header(AVFormatContext *s);
> +static int adaudio_read_packet(struct AVFormatContext *s, AVPacket *pkt);
> +
Unnecessary forward declarations
> +
> +static int adaudio_probe(AVProbeData *p)
> +{
> + if( p->buf_size < 4 )
> + return 0;
> +
> + /* Check the value of the first byte and the payload byte */
> + if(
> + p->buf[0] == 0x80 &&
> + (
> + p->buf[1] == RTP_PAYLOAD_TYPE_8000HZ_ADPCM ||
> + p->buf[1] == RTP_PAYLOAD_TYPE_11025HZ_ADPCM ||
> + p->buf[1] == RTP_PAYLOAD_TYPE_16000HZ_ADPCM ||
> + p->buf[1] == RTP_PAYLOAD_TYPE_22050HZ_ADPCM
> + )
> + ) {
This is not K&R.
> + return AVPROBE_SCORE_MAX;
> + }
> +
> + return 0;
> +}
> +
> +static int adaudio_read_header(AVFormatContext *s)
> +{
> + s->ctx_flags |= AVFMTCTX_NOHEADER;
> +
> + return 0;
> +}
> +
> +static int adaudio_read_packet(struct AVFormatContext *s, AVPacket *pkt)
"struct" is unnecessary.
> +{
> + AVIOContext * ioContext = s->pb;
CamelCase is illegal in FFmpeg except for type names. Use underscores.
> + int retVal = AVERROR(EIO);
> + int packetSize = 0;
> + int sampleSize = 0;
> + AVStream * st = NULL;
> + int isPacketAlloced = 0;
> +#ifdef AD_SIDEDATA_IN_PRIV
> + struct ADFrameData * frameData = NULL;
> +#endif
If you are adding new elements to AVPacket
> +
> + /* Get the next packet */
> + if( (packetSize = ioContext->read_packet( ioContext->opaque,
> ioContext->buf_ptr, ioContext->buffer_size )) > 0 ) {
Who says that ioContext has a read_packet function at all? Why are you
relying on its buffer_size having the right value?
Furthermore, the style with a space after opening parentheses and
before closing parentheses is uncommon here.
> + /* Validate the 12 byte RTP header as best we can */
> + if( ioContext->buf_ptr[1] == RTP_PAYLOAD_TYPE_8000HZ_ADPCM ||
> + ioContext->buf_ptr[1] == RTP_PAYLOAD_TYPE_11025HZ_ADPCM ||
> + ioContext->buf_ptr[1] == RTP_PAYLOAD_TYPE_16000HZ_ADPCM ||
> + ioContext->buf_ptr[1] == RTP_PAYLOAD_TYPE_22050HZ_ADPCM
> + ) {
> + /* Calculate the size of the sample data */
> + sampleSize = packetSize - SIZEOF_RTP_HEADER;
> +
> + /* Create a new AVPacket */
> + if( av_new_packet( pkt, sampleSize ) >= 0 ) {
Instead of checking for this to be >= 0, you should rather save the
return value and check it for errors; if an error is indicated, you
return with the right errors (you may use gotos in order to integrate
freeing data for multiple errors). Initializing retVal to AVERROR(EIO)
is unnecessary.
> + isPacketAlloced = 1;If you do the above, this variable becomes unnecessary, too.
> +
> + /* Copy data into packet */
> + audiodata_network2host(pkt->data,
> &ioContext->buf_ptr[SIZEOF_RTP_HEADER], sampleSize);
> +
> + /* Configure stream info */
> + if( (st = ad_get_audio_stream(s, NULL)) != NULL ) {
If I see this correctly, then ad_get_audio_stream will always
dereference its second argument unless there are no audio streams
already in s and unless a call to avformat_new_stream fails. So you
have a NULL pointer dereference. Did you really test this patch?
> + pkt->stream_index = st->index;
> + pkt->duration = ((int)(AV_TIME_BASE * 1.0));
> +
> +#ifdef AD_SIDEDATA_IN_PRIV
It seems that you are basing this patch on others that modify core
components and structures like AVPacket; but you have not released
said modifications, so that you effectively propose that your private
branch (that is useful only to you as the others don't have it) should
be merged into FFmpeg. Good luck with that.
> + if( (frameData = av_malloc(sizeof(*frameData))) !=
> NULL ) {
> + /* Set the frame info up */
> + frameData->frameType = RTPAudio;
> + frameData->frameData =
> (void*)(&ioContext->buf_ptr[1]);
Sample rate changes can already be communicated via sidedata of the
AV_PKT_DATA_PARAM_CHANGE type. It is of course unnecessary to add this
if the parameters don't really change.
> + frameData->additionalData = NULL;
> +
> + pkt->priv = (void*)frameData;
> + retVal = 0;
> + }
> + else
> + retVal = AVERROR(ENOMEM);
> +#endif
> + }
> + }
> + }
> + }
> +
> + /* Check whether we need to release the packet data we allocated */
> + if( retVal < 0 && isPacketAlloced != 0 ) {
> + av_free_packet( pkt );Why not av_packet_unref?
> + }
> +
> + return retVal;
> +}
> +
> +
I'll stop here.
- Andreas
More information about the ffmpeg-devel
mailing list