[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