[FFmpeg-devel] [PATCH 2/2] MxPEG decoder

Anatoly Nenashev anatoly.nenashev
Thu Nov 25 16:30:50 CET 2010


On 25.11.2010 07:05, Michael Niedermayer wrote:
> 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
>    

Ok. I've replaced the condition in "if" by this version. See attachment.

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

I've made some tests with trasher. There is no problem with crashes in 
demuxer but i've found one problem in decoder. I've reported about it in 
mail list.
I think this is a common problem for MJPEG and MxPEG.


> except these i guess the patch should be close to be ok
>    

Cool! Is it true for decoder (PATCH 1/2)?

-------------- next part --------------
A non-text attachment was scrubbed...
Name: mxg_v18.patch
Type: text/x-patch
Size: 9874 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101125/fc6cdbca/attachment.bin>



More information about the ffmpeg-devel mailing list