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

Michael Niedermayer michaelni
Mon Nov 1 20:10:24 CET 2010


On Mon, Nov 01, 2010 at 09:42:06PM +0300, Anatoly Nenashev wrote:
> On 01.11.2010 15:55, Michael Niedermayer wrote:
>> On Mon, Nov 01, 2010 at 11:52:54AM +0300, Anatoly Nenashev wrote:
>>    
>>> On 01.11.2010 03:31, Michael Niedermayer wrote:
>>>      
>>>> On Mon, Nov 01, 2010 at 02:56:50AM +0300, Anatoly Nenashev wrote:
>>>>
>>>>        
>>>>> On 01.11.2010 01:17, Michael Niedermayer wrote:
>>>>>
>>>>>          
>>>>>> On Sun, Oct 31, 2010 at 09:23:13PM +0300, Anatoly Nenashev wrote:
>>>>>>
>>>>>>
>>>>>>            
>>>>>>> Patch for .mxg demuxer
>>>>>>>
>>>>>>>
>>>>>>>              
>>>>>> [...]
>>>>>>
>>>>>>
>>>>>>            
>>>>>>> +static int mxg_read_header(AVFormatContext *s, AVFormatParameters *ap)
>>>>>>> +{
>>>>>>> +    AVStream *video_st = 0, *audio_st = 0;
>>>>>>> +    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;
>>>>>>> +
>>>>>>>
>>>>>>>
>>>>>>>              
>>>>>>
>>>>>>            
>>>>>>> +    mxg->buffer = (uint8_t*) av_malloc(DEFAULT_PACKET_SIZE);
>>>>>>>
>>>>>>>
>>>>>>>              
>>>>>> unneeded cast
>>>>>>
>>>>>>
>>>>>>            
>>>>> removed
>>>>>
>>>>>
>>>>>          
>>>>>> [...]
>>>>>>
>>>>>>
>>>>>>            
>>>>>>> +        if (mxg->found_video_packet) {
>>>>>>> +            mxg->buffer[mxg->current_pos++] = data;
>>>>>>> +            if (mxg->state == 0xfffe) {
>>>>>>> +                int size = get_be16(s->pb);
>>>>>>> +                if (url_feof(s->pb) || url_ferror(s->pb))
>>>>>>> +                    return AVERROR_EOF;
>>>>>>> +                mxg->buffer = av_fast_realloc(mxg->buffer,&mxg->buffer_size,
>>>>>>> +                                              mxg->current_pos + size);
>>>>>>>
>>>>>>>
>>>>>>>              
>>>>>> here the + and similar at other points can overflow and lead to realloc to a
>>>>>> too small buffer
>>>>>>
>>>>>>
>>>>>>            
>>>>> fixed
>>>>>
>>>>>
>>>>>          
>>>>>> [...]
>>>>>>
>>>>>>
>>>>>>            
>>>>>>> +            if (mxg->current_pos>= mxg->buffer_size) {
>>>>>>> +                mxg->buffer = (uint8_t*) av_fast_realloc(mxg->buffer,&mxg->buffer_size,
>>>>>>>
>>>>>>>
>>>>>>>              
>>>>>> unneeded cast
>>>>>>
>>>>>>
>>>>>>            
>>>>> removed
>>>>>
>>>>>
>>>>>          
>>>> [...]
>>>>
>>>>        
>>>>> +static int mxg_read_packet(AVFormatContext *s, AVPacket *pkt)
>>>>> +{
>>>>> +    int ret, size;
>>>>> +    MXGContext *mxg = s->priv_data;
>>>>> +
>>>>> +    while(!url_feof(s->pb)&&   !url_ferror(s->pb)) {
>>>>> +        uint8_t data;
>>>>> +        int found_frame_end = 0;
>>>>> +
>>>>> +        if (mxg->state == 0xffed) {
>>>>> +            size = get_be16(s->pb);
>>>>> +            if (url_feof(s->pb) || url_ferror(s->pb))
>>>>> +                return AVERROR_EOF;
>>>>> +            if (size<= 14)
>>>>> +                return AVERROR(EINVAL);
>>>>> +            url_fskip(s->pb, 12);
>>>>> +            if (url_feof(s->pb))
>>>>> +                return AVERROR_EOF;
>>>>> +            size -= 14;
>>>>> +            ret = av_get_packet(s->pb, pkt, size);
>>>>> +            if (ret<   0)
>>>>> +                return ret;
>>>>> +
>>>>> +            pkt->stream_index = AUDIO_STREAM_INDEX;
>>>>> +            mxg->state = 0;
>>>>> +
>>>>> +            return pkt->size;
>>>>> +        }
>>>>> +
>>>>> +        data = get_byte(s->pb);
>>>>> +        mxg->state = (mxg->state<<   8) | data;
>>>>> +
>>>>> +        if (mxg->found_video_packet) {
>>>>> +            mxg->buffer[mxg->current_pos++] = data;
>>>>> +            if (mxg->state == 0xfffe) {
>>>>> +                int size = get_be16(s->pb);
>>>>> +                if (url_feof(s->pb) || url_ferror(s->pb))
>>>>> +                    return AVERROR_EOF;
>>>>> +
>>>>> +                if (mxg->current_pos>   mxg->current_pos + size)
>>>>> +                {
>>>>> +                    av_log(s, AV_LOG_ERROR, "buffer overflow\n");
>>>>> +                    return AVERROR(ENOMEM);
>>>>> +                }
>>>>> +                mxg->buffer = av_fast_realloc(mxg->buffer,&mxg->buffer_size,
>>>>> +                                              mxg->current_pos + size);
>>>>> +                if (!mxg->buffer) {
>>>>> +                    av_log(s, AV_LOG_ERROR, "mxg demuxer error in av_fast_realloc()\n");
>>>>> +                    return AVERROR(ENOMEM);
>>>>> +                }
>>>>> +                if (size<   2) {
>>>>> +                    av_log(s, AV_LOG_ERROR, "wrong comment buffer size\n");
>>>>> +                    return AVERROR(EINVAL);
>>>>> +                }
>>>>> +                mxg->buffer[mxg->current_pos++] = size>>   8;
>>>>> +                mxg->buffer[mxg->current_pos++] = size&   0xff;
>>>>> +
>>>>> +                ret = get_buffer(s->pb, mxg->buffer + mxg->current_pos, size - 2);
>>>>> +                if (ret<   0)
>>>>> +                    return ret;
>>>>> +                if (ret>= 16&&   !strncmp(mxg->buffer + mxg->current_pos, "MXF", 3)) {
>>>>> +                    mxg->dts = AV_RL64(mxg->buffer + mxg->current_pos + 8);
>>>>> +                }
>>>>> +                mxg->current_pos += ret;
>>>>> +                mxg->state = 0;
>>>>> +            } else if (mxg->state == 0xffd9) {
>>>>> +                found_frame_end = mxg->current_pos;
>>>>> +                mxg->current_pos = 0;
>>>>> +                mxg->found_video_packet = 0;
>>>>> +            } else if (mxg->state == 0xffd8) {
>>>>> +                //emulating frame end
>>>>> +                found_frame_end = mxg->current_pos - 2;
>>>>> +                mxg->current_pos = 2;
>>>>> +                mxg->found_video_packet = 1;
>>>>> +            } else if (mxg->state == 0xffed) {
>>>>> +                //emulating frame end
>>>>> +                found_frame_end = mxg->current_pos - 2;
>>>>> +                mxg->current_pos = 0;
>>>>> +                mxg->found_video_packet = 0;
>>>>> +            }
>>>>> +
>>>>> +            if (found_frame_end) {
>>>>> +                ret = av_new_packet(pkt, found_frame_end);
>>>>> +                if (ret<   0)
>>>>> +                    return ret;
>>>>> +                memcpy(pkt->data, mxg->buffer, found_frame_end);
>>>>> +                pkt->stream_index = VIDEO_STREAM_INDEX;
>>>>> +                pkt->dts = mxg->dts;
>>>>> +                return pkt->size;
>>>>> +            }
>>>>> +
>>>>> +            if (mxg->current_pos>= mxg->buffer_size) {
>>>>> +                if (mxg->current_pos>   mxg->current_pos + DEFAULT_PACKET_SIZE)
>>>>> +                {
>>>>> +                    av_log(s, AV_LOG_ERROR, "buffer overflow\n");
>>>>> +                    return AVERROR(ENOMEM);
>>>>> +                }
>>>>> +                mxg->buffer = av_fast_realloc(mxg->buffer,&mxg->buffer_size,
>>>>> +                                              mxg->current_pos + DEFAULT_PACKET_SIZE);
>>>>> +                if (!mxg->buffer) {
>>>>> +                    av_log(s, AV_LOG_ERROR, "mxg demuxer error in av_fast_realloc()\n");
>>>>> +                    return AVERROR(ENOMEM);
>>>>> +                }
>>>>> +            }
>>>>> +        } else if (mxg->state == 0xffd8){
>>>>> +            mxg->found_video_packet = 1;
>>>>> +        }
>>>>>
>>>>>          
>>>> This loop is a bit oddly structured, is there a reason for this?
>>>> id expect it to be more of the form
>>>> while {
>>>> state= (state<<8) + ...
>>>>
>>>> if(state ==)
>>>> else if
>>>> else if
>>>> else
>>>> }
>>>>
>>>> but its kind of convoluted with the audio code being before the state update
>>>> then found_video_packet=1 and checks for that
>>>>
>>>>
>>>>        
>>> Reimplemented. I hope that new version is more canonical.:-)
>>>      
>> no its the same
>> id like to understand why it is so convolutedly written if there is a reason
>> why this is needed
>> If its really needed this has to be documented so someone doesnt waste time
>> rewriting it to half the code.
>> And if there is no reason it should be simplified
>>    
>>
>
> The principal reason consists in the following. If demuxer yet hasn't  
> found a starting marker for package video it should check much less  
> conditions, than in other case. Moreover in the second case it should  
> copy data from incoming stream into the internal buffer because we don't  
> know the size of video packet. That's why I have divided these two cases  
> in a program code in hope that so will be faster.

I thought there is just audio and video
What you say makes one belive that there are significant amounts of other
things, besides

last= byte;
byte= get_byte(s->pb);

if(last == 0xFF){
    if(byte==
        ...
    else if
        ...
}

could be used to reduce the checks if they really matter speedwise

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The real ebay dictionary, page 1
"Used only once"    - "Some unspecified defect prevented a second use"
"In good condition" - "Can be repaird by experienced expert"
"As is" - "You wouldnt want it even if you were payed for it, if you knew ..."
-------------- 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/20101101/bba6c070/attachment.pgp>



More information about the ffmpeg-devel mailing list