[FFmpeg-devel] [PATCH 2/2] MxPEG decoder
Michael Niedermayer
michaelni
Mon Nov 1 13:55:03 CET 2010
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
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
The bravest are surely those who have the clearest vision
of what is before them, glory and danger alike, and yet
notwithstanding go out to meet it. -- Thucydides
-------------- 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/fb656343/attachment.pgp>
More information about the ffmpeg-devel
mailing list