[FFmpeg-devel] [PATCH 2/2] MxPEG decoder
Michael Niedermayer
michaelni
Thu Nov 25 05:05:31 CET 2010
On Wed, Nov 17, 2010 at 06:08:44PM +0300, Anatoly Nenashev wrote:
> On 17.11.2010 16:49, Michael Niedermayer wrote:
>> On Mon, Nov 15, 2010 at 03:27:07AM +0300, Anatoly Nenashev wrote:
>>
>>> On 13.11.2010 04:24, Michael Niedermayer wrote:
>>>
>>>> On Thu, Nov 11, 2010 at 02:16:27AM +0300, Anatoly Nenashev wrote:
>>>>
>>>>
>>>>> On 10.11.2010 18:28, Michael Niedermayer wrote:
>>>>>
>>>>>
>>>>>> On Wed, Nov 10, 2010 at 06:08:47PM +0300, Anatoly Nenashev wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>>> [...]
>>>>>>> If to read more than 16 byte at once then it is required additional
>>>>>>> operations of memcpy and memmove.
>>>>>>> For example. If I read buffer of 256 bytes in which audio packet
>>>>>>> available in position 10 and size 100 then I need
>>>>>>> to copy data of size 100-10=90 in new audio packet and move data of size
>>>>>>> 256-100=156 in internal buffer. I think this may reduce the performance.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>> you dont need to move it.
>>>>>>
>>>>>> and the overhead of all your code executed once every 16 bytes will reduce
>>>>>> performance more than an occasional memcpy(). That said it can be done
>>>>>> without memcpy by accessing the internal buffer but iam not suggesting this
>>>>>> to be done.
>>>>>>
>>>>>>
>>>>>>
>>>> There are many ways to implement this, one simple one is
>>>> you have a internal buffer
>>>> you read into it
>>>> you return AVPackets that point into this buffer and have destruct=NULL
>>>>
>>>>
>>>>
>>> Implemented this way.
>>>
>>>
>> [...]
>>
>>> +static int mxg_read_header(AVFormatContext *s, AVFormatParameters *ap)
>>> +{
>>> + AVStream *video_st = 0, *audio_st = 0;
>>>
>> the =0 is useless
>>
>>
>>
> Ok. Removed.
>
>> [...]
>>
>>
>>> + ret = mxg_reallocate_buffer(mxg, size + FF_INPUT_BUFFER_PADDING_SIZE);
>>> + if (ret< 0)
>>> + return ret;
>>> + startmarker_ptr = mxg->buffer_ptr - 2;
>>> +
>>> + ret = get_buffer(s->pb, mxg->buffer_ptr + mxg->cache_size,
>>> + size - mxg->cache_size);
>>> + if (ret< 0)
>>> + return ret;
>>>
>> i think the get_buffer() could also be moved into mxg_reallocate_buffer() and
>> it be renamed accordingly. That should simplify the code
>>
>>
>
> Reimplemented.
> Makefile | 1
> allformats.c | 1
> mxg.c | 251 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 253 insertions(+)
> 93f3152efcd761ee465f3d3eab8fcc9e4ea7af04 mxg_v17.patch
> diff --git a/libavformat/Makefile b/libavformat/Makefile
> index e62a5ea..d3e3cd0 100644
> --- a/libavformat/Makefile
> +++ b/libavformat/Makefile
> @@ -147,6 +147,7 @@ OBJS-$(CONFIG_MTV_DEMUXER) += mtv.o
> OBJS-$(CONFIG_MVI_DEMUXER) += mvi.o
> OBJS-$(CONFIG_MXF_DEMUXER) += mxfdec.o mxf.o
> OBJS-$(CONFIG_MXF_MUXER) += mxfenc.o mxf.o audiointerleave.o
> +OBJS-$(CONFIG_MXG_DEMUXER) += mxg.o
> OBJS-$(CONFIG_NC_DEMUXER) += ncdec.o
> OBJS-$(CONFIG_NSV_DEMUXER) += nsvdec.o
> OBJS-$(CONFIG_NULL_MUXER) += nullenc.o
> diff --git a/libavformat/allformats.c b/libavformat/allformats.c
> index e35f4f8..4afbef5 100644
> --- a/libavformat/allformats.c
> +++ b/libavformat/allformats.c
> @@ -142,6 +142,7 @@ void av_register_all(void)
> REGISTER_DEMUXER (MVI, mvi);
> REGISTER_MUXDEMUX (MXF, mxf);
> REGISTER_MUXER (MXF_D10, mxf_d10);
> + REGISTER_DEMUXER (MXG, mxg);
> REGISTER_DEMUXER (NC, nc);
> REGISTER_DEMUXER (NSV, nsv);
> REGISTER_MUXER (NULL, null);
> diff --git a/libavformat/mxg.c b/libavformat/mxg.c
> new file mode 100644
> index 0000000..778ef77
> --- /dev/null
> +++ b/libavformat/mxg.c
> @@ -0,0 +1,251 @@
> +/*
> + * MxPEG clip file demuxer
> + * Copyright (c) 2010 Anatoly Nenashev
> + *
> + * 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/intreadwrite.h"
> +#include "libavcodec/mjpeg.h"
> +#include "avformat.h"
> +#include "avio.h"
> +
> +#define VIDEO_STREAM_INDEX 0
> +#define AUDIO_STREAM_INDEX 1
> +#define DEFAULT_PACKET_SIZE 1024
> +#define OVERREAD_SIZE 3
> +
> +typedef struct MXGContext {
> + uint8_t *buffer;
> + uint8_t *buffer_ptr;
> + uint8_t *soi_ptr;
> + unsigned int buffer_size;
> + int64_t dts;
> + unsigned int cache_size;
> +} MXGContext;
> +
> +static int mxg_read_header(AVFormatContext *s, AVFormatParameters *ap)
> +{
> + AVStream *video_st, *audio_st;
> + MXGContext *mxg = s->priv_data;
> +
> + /* video parameters will be extracted from the compressed bitstream */
> + video_st = av_new_stream(s, VIDEO_STREAM_INDEX);
> + if (!video_st)
> + return AVERROR(ENOMEM);
> + video_st->codec->codec_type = AVMEDIA_TYPE_VIDEO;
> + video_st->codec->codec_id = CODEC_ID_MXPEG;
> + av_set_pts_info(video_st, 64, 1, 1000000);
> +
> + audio_st = av_new_stream(s, AUDIO_STREAM_INDEX);
> + if (!audio_st)
> + return AVERROR(ENOMEM);
> + audio_st->codec->codec_type = AVMEDIA_TYPE_AUDIO;
> + audio_st->codec->codec_id = CODEC_ID_PCM_ALAW;
> + audio_st->codec->channels = 1;
> + audio_st->codec->sample_rate = 8000;
> + audio_st->codec->bits_per_coded_sample = 8;
> + audio_st->codec->block_align = 1;
> + av_set_pts_info(audio_st, 64, 1, 1000000);
> +
> + mxg->soi_ptr = mxg->buffer_ptr = mxg->buffer = 0;
> + mxg->buffer_size = 0;
> + mxg->dts = AV_NOPTS_VALUE;
> + mxg->cache_size = 0;
> +
> + return 0;
> +}
> +
> +static uint8_t* mxg_find_startmarker(uint8_t *p, uint8_t *end)
> +{
> + for (; p < end - 3; p += 4) {
> + uint32_t x = *(uint32_t*)p;
> +
> + if (x & (~(x+0x01010101)) & 0x80808080) {
> + if (p[0] == 0xff) {
> + return p;
> + } else if (p[1] == 0xff) {
> + return p+1;
> + } else if (p[2] == 0xff) {
> + return p+2;
> + } else if (p[3] == 0xff) {
> + return p+3;
> + }
> + }
> + }
> +
> + for (; p < end; ++p) {
> + if (*p == 0xff) return p;
> + }
> +
> + return end;
> +}
> +
> +static int mxg_update_cache(AVFormatContext *s, unsigned int cache_size)
> +{
> + MXGContext *mxg = s->priv_data;
> + unsigned int current_pos = mxg->buffer_ptr - mxg->buffer;
> + unsigned int soi_pos;
> + int ret;
> +
> + /* reallocate internal buffer */
> + if (current_pos > current_pos + cache_size)
> + return AVERROR(ENOMEM);
> + if (mxg->soi_ptr) soi_pos = mxg->soi_ptr - mxg->buffer;
> + mxg->buffer = av_fast_realloc(mxg->buffer, &mxg->buffer_size,
> + current_pos + cache_size +
> + FF_INPUT_BUFFER_PADDING_SIZE);
> + if (!mxg->buffer)
> + return AVERROR(ENOMEM);
> + mxg->buffer_ptr = mxg->buffer + current_pos;
> + if (mxg->soi_ptr) mxg->soi_ptr = mxg->buffer + soi_pos;
> +
> + /* get data */
> + ret = get_buffer(s->pb, mxg->buffer_ptr + mxg->cache_size,
> + cache_size - mxg->cache_size);
> + if (ret < 0)
> + return ret;
> +
> + mxg->cache_size += ret;
> +
> + return ret;
> +}
> +
> +static int mxg_read_packet(AVFormatContext *s, AVPacket *pkt)
> +{
> + int ret;
> + unsigned int size;
> + uint8_t *startmarker_ptr, *end, *search_end, marker;
> + MXGContext *mxg = s->priv_data;
> +
> + while (!url_feof(s->pb) && !url_ferror(s->pb)){
> + if (mxg->cache_size <= OVERREAD_SIZE) {
> + /* update internal buffer */
> + ret = mxg_update_cache(s, DEFAULT_PACKET_SIZE + OVERREAD_SIZE);
> + if (ret < 0)
> + return ret;
> + }
> + end = mxg->buffer_ptr + mxg->cache_size;
> +
> + /* find start marker - 0xff */
> + if (mxg->buffer_ptr < end - OVERREAD_SIZE) {
mxg->cache_size > OVERREAD_SIZE
also has the demuxer been tested with corrupted input (see tools/trasher) ?
it should not crash or end in an infinite loop at least. If it can recover
after damage that is a plus
except these i guess the patch should be close to be ok
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
DNS cache poisoning attacks, popular search engine, Google internet authority
dont be evil, please
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101125/60116936/attachment.pgp>
More information about the ffmpeg-devel
mailing list