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

Michael Niedermayer michaelni
Mon Nov 1 23:32:00 CET 2010


On Tue, Nov 02, 2010 at 12:12:16AM +0300, Anatoly Nenashev wrote:
> On 01.11.2010 23:56, Anatoly Nenashev wrote:
>> On 01.11.2010 22:10, Michael Niedermayer wrote:
>>> 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
>> Yes, this is that case. If the data is received from http they can  
>> contain some additional information in the beginning of each frame  
>> like this
>> 0019fa0: 5454 d5d7 d6d6 d50d 0a2d 2d4d 4f42 4f54  TT.......--MOBOT
>> 0019fb0: 4958 5f46 6173 745f 5365 7276 6572 7075  IX_Fast_Serverpu
>> 0019fc0: 7368 0d0a 436f 6e74 656e 742d 7479 7065  sh..Content-type
>> 0019fd0: 3a20 696d 6167 652f 6a70 6567 0d0a 0d0a  : image/jpeg....
>> 0019fe0: ffd8 ffe0 0010 4a46 4946 0001 0100 0001  ......JFIF......
>> .........
>>
>>

the video frames must be pretty small if that makes significant amounts of
data

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

Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope
-------------- 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/1d3c2880/attachment.pgp>



More information about the ffmpeg-devel mailing list