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

[Anatoly Nenashev]

On 17.11.2010 18:08, 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.
>
>    

Ping.